Skip to content

JAMES-4133 Add support for unicode adddresses as defined in RFC6532. #2728

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

JAMES-4133 Add support for unicode adddresses as defined in RFC6532. #2728

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions core/src/main/java/org/apache/james/core/Domain.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.apache.james.core;

import java.io.Serializable;
import java.net.IDN;
import java.util.Locale;
import java.util.Objects;

Expand Down Expand Up @@ -54,9 +55,16 @@ public static Domain of(String domain) {
Preconditions.checkArgument(domain.length() <= MAXIMUM_DOMAIN_LENGTH,
"Domain name length should not exceed %s characters", MAXIMUM_DOMAIN_LENGTH);

String domainWithoutBrackets = removeBrackets(domain);
String domainWithoutBrackets = IDN.toASCII(removeBrackets(domain), IDN.ALLOW_UNASSIGNED);
Preconditions.checkArgument(PART_CHAR_MATCHER.matchesAllOf(domainWithoutBrackets),
"Domain parts ASCII chars must be a-z A-Z 0-9 - or _ in %s", domain);
"Domain parts ASCII chars must be a-z A-Z 0-9 - or _ in %s", domain);

if (domainWithoutBrackets.startsWith("xn--") ||
domainWithoutBrackets.contains(".xn--")) {
domainWithoutBrackets = IDN.toUnicode(domainWithoutBrackets);
Preconditions.checkArgument(!domainWithoutBrackets.startsWith("xn--") &&
!domainWithoutBrackets.contains(".xn--"));
}

int pos = 0;
int nextDot = domainWithoutBrackets.indexOf('.');
Expand Down
28 changes: 24 additions & 4 deletions core/src/main/java/org/apache/james/core/MailAddress.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.apache.james.core;

import java.net.IDN;
import java.util.Locale;
import java.util.Objects;
import java.util.Optional;
Expand Down Expand Up @@ -418,7 +419,7 @@ public Optional<InternetAddress> toInternetAddress() {
try {
return Optional.of(new InternetAddress(toString()));
} catch (AddressException ae) {
LOGGER.warn("A valid address '{}' as per James criterial fails to parse as a jakarta.mail InternetAdrress", asString());
LOGGER.warn("A valid address '{}' as per James criteria fails to parse as a jakarta.mail InternetAdrress", asString());
return Optional.empty();
}
}
Expand Down Expand Up @@ -549,15 +550,15 @@ private int parseUnquotedLocalPart(StringBuilder lpSB, String address, int pos)
//End of local-part
break;
} else {
//<c> ::= any one of the 128 ASCII characters, but not any
// <special> or <SP>
//<c> ::= any printable ASCII character, or any non-ASCII
// unicode codepoint, but not <special> or <SP>
//<special> ::= "<" | ">" | "(" | ")" | "[" | "]" | "\" | "."
// | "," | ";" | ":" | "@" """ | the control
// characters (ASCII codes 0 through 31 inclusive and
// 127)
//<SP> ::= the space character (ASCII code 32)
char c = address.charAt(pos);
if (c <= 31 || c >= 127 || c == ' ') {
if (c <= 31 || c == 127 || c == ' ') {
throw new AddressException("Invalid character in local-part (user account) at position " +
(pos + 1) + " in '" + address + "'", address, pos + 1);
}
Expand Down Expand Up @@ -688,6 +689,7 @@ private int parseDomain(StringBuilder dSB, String address, int pos) throws Addre
// in practice though, we should relax this as domain names can start
// with digits as well as letters. So only check that doesn't start
// or end with hyphen.
boolean unicode = false;
while (true) {
if (pos >= address.length()) {
break;
Expand All @@ -700,13 +702,31 @@ private int parseDomain(StringBuilder dSB, String address, int pos) throws Addre
resultSB.append(ch);
pos++;
continue;
} else if (ch >= 0x0080) {
resultSB.append(ch);
pos++;
unicode = true;
continue;
}
if (ch == '.') {
break;
}
throw new AddressException("Invalid character at " + pos + " in '" + address + "'", address, pos);
}
String result = resultSB.toString();
if (unicode) {
try {
result = IDN.toASCII(result, IDN.ALLOW_UNASSIGNED);
} catch (IllegalArgumentException e) {
throw new AddressException("Domain invalid according to IDNA", address);
}
}
if (result.startsWith("xn--") || result.contains(".xn--")) {
result = IDN.toUnicode(result);
if (result.startsWith("xn--") || result.contains(".xn--")) {
throw new AddressException("Domain invalid according to IDNA", address);
}
}
if (result.startsWith("-") || result.endsWith("-")) {
throw new AddressException("Domain name cannot begin or end with a hyphen \"-\" at position " +
(pos + 1) + " in '" + address + "'", address, pos + 1);
Expand Down
81 changes: 81 additions & 0 deletions core/src/test/java/org/apache/james/core/DomainTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/****************************************************************
* Licensed to the Apache Software Foundation (ASF) under one *
* or more contributor license agreements. See the NOTICE file *
* distributed with this work for additional information *
* regarding copyright ownership. The ASF licenses this file *
* to you under the Apache License, Version 2.0 (the *
* "License"); you may not use this file except in compliance *
* with the License. You may obtain a copy of the License at *
* *
* http://www.apache.org/licenses/LICENSE-2.0 *
* *
* Unless required by applicable law or agreed to in writing, *
* software distributed under the License is distributed on an *
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
* KIND, either express or implied. See the License for the *
* specific language governing permissions and limitations *
* under the License. *
****************************************************************/

package org.apache.james.core;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import java.util.stream.Stream;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;


class DomainTest {
@Test
void testPlainDomain() {
Domain d1 = Domain.of("example.com");
assertThat(d1.name().equals(d1.asString()));
Domain d2 = Domain.of("Example.com");
assertThat(d2.name()).isNotEqualTo(d2.asString());
assertThat(d1.asString()).isEqualTo(d2.asString());
}

@Test
void testIPv4Domain() {
Domain d1 = Domain.of("192.0.4.1");
assertThat(d1.asString()).isEqualTo("192.0.4.1");
}

@Test
void testPunycodeIDN() {
Domain d1 = Domain.of("xn--gr-zia.example");
assertThat(d1.asString()).isEqualTo("grå.example");
}

@Test
void testDevanagariDomain() {
Domain d1 = Domain.of("डाटामेल.भारत");
assertThat(d1.asString()).isEqualTo(d1.name());
}

private static Stream<Arguments> malformedDomains() {
return Stream.of(
"😊☺️.example", // emoji not permitted by IDNA
"#.example", // really and truly not permitted
"\uFEFF.example", // U+FEFF is the byte order mark
"\u200C.example", // U+200C is a zero-width non-joiner
"\u200Eibm.example" // U+200E is left-to-right
)
.map(Arguments::of);
}

@ParameterizedTest
@MethodSource("malformedDomains")
void testMalformedDomains(String malformed) {
assertThatThrownBy(() -> Domain.of(malformed))
.as("rejecting malformed domain " + malformed)
.isInstanceOf(IllegalArgumentException.class);
}
}


34 changes: 25 additions & 9 deletions core/src/test/java/org/apache/james/core/MailAddressTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,15 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;

import java.util.Properties;
import java.util.stream.Stream;

import jakarta.mail.Session;
import jakarta.mail.internet.AddressException;
import jakarta.mail.internet.InternetAddress;

import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
Expand Down Expand Up @@ -55,6 +58,13 @@ private static Stream<Arguments> goodAddresses() {
"\\[email protected]",
"[email protected]",
"[email protected]",
"Loïc.Accentué@voilà.fr8",
"pelé@exemple.com",
"δοκιμή@παράδειγμα.δοκιμή",
"我買@屋企.香港",
"二ノ宮@黒川.日本",
"медведь@с-балалайкой.рф",
//"संपर्क@डाटामेल.भारत", fails in Jakarta, reason still unknown
"user+mailbox/[email protected]",
"[email protected]",
"\"Abc@def\"@example.com",
Expand Down Expand Up @@ -96,40 +106,46 @@ private static Stream<Arguments> badAddresses() {
"server-dev@[127.0.1.1.1]",
"server-dev@[127.0.1.-1]",
"test@dom+ain.com",
"[email protected]",
"\"a..b\"@domain.com", // jakarta.mail is unable to handle this so we better reject it
"server-dev\\[email protected]", // jakarta.mail is unable to handle this so we better reject it
"[email protected]",
// According to wikipedia these addresses are valid but as jakarta.mail is unable
// to work with them we shall rather reject them (note that this is not breaking retro-compatibility)
"Loïc.Accentué@voilà.fr8",
"pelé@exemple.com",
"δοκιμή@παράδειγμα.δοκιμή",
"我買@屋企.香港",
"二ノ宮@黒川.日本",
"медведь@с-балалайкой.рф",
"संपर्क@डाटामेल.भारत",
"sales@\u200Eibm.example", // U+200E is left-to-right
// According to wikipedia this address is valid but as jakarta.mail is unable
// to work with it we shall rather reject them (note that this is not breaking retro-compatibility)
"mail.allow\\,[email protected]")
.map(Arguments::of);
}

@BeforeEach
void setup() {
Properties props = new Properties();
props.setProperty("mail.mime.allowutf8", "true");
Session s = Session.getDefaultInstance(props);
assertThat(Boolean.parseBoolean(s.getProperties().getProperty("mail.mime.allowutf8", "false")));
}

@ParameterizedTest
@MethodSource("goodAddresses")
void testGoodMailAddressString(String mailAddress) {
assertThatCode(() -> new MailAddress(mailAddress))
.as("parses " + mailAddress)
.doesNotThrowAnyException();
}

@ParameterizedTest
@MethodSource("goodAddresses")
void toInternetAddressShouldNoop(String mailAddress) throws Exception {
assertThat(new MailAddress(mailAddress).toInternetAddress())
.as("tries to parse " + mailAddress + " using jakarta.mail")
.isNotEmpty();
}

@ParameterizedTest
@MethodSource("badAddresses")
void testBadMailAddressString(String mailAddress) {
Assertions.assertThatThrownBy(() -> new MailAddress(mailAddress))
.as("fails to parse " + mailAddress)
.isInstanceOf(AddressException.class);
}

Expand Down