diff --git a/core/src/main/java/org/apache/james/core/Domain.java b/core/src/main/java/org/apache/james/core/Domain.java index 50ad81c4126..b4372e2b807 100644 --- a/core/src/main/java/org/apache/james/core/Domain.java +++ b/core/src/main/java/org/apache/james/core/Domain.java @@ -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; @@ -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('.'); diff --git a/core/src/main/java/org/apache/james/core/MailAddress.java b/core/src/main/java/org/apache/james/core/MailAddress.java index 41e88b2b90c..3be44aa8426 100644 --- a/core/src/main/java/org/apache/james/core/MailAddress.java +++ b/core/src/main/java/org/apache/james/core/MailAddress.java @@ -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; @@ -418,7 +419,7 @@ public Optional 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(); } } @@ -549,15 +550,15 @@ private int parseUnquotedLocalPart(StringBuilder lpSB, String address, int pos) //End of local-part break; } else { - // ::= any one of the 128 ASCII characters, but not any - // or + // ::= any printable ASCII character, or any non-ASCII + // unicode codepoint, but not or // ::= "<" | ">" | "(" | ")" | "[" | "]" | "\" | "." // | "," | ";" | ":" | "@" """ | the control // characters (ASCII codes 0 through 31 inclusive and // 127) // ::= 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); } @@ -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; @@ -700,6 +702,11 @@ 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; @@ -707,6 +714,19 @@ private int parseDomain(StringBuilder dSB, String address, int pos) throws Addre 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); diff --git a/core/src/test/java/org/apache/james/core/DomainTest.java b/core/src/test/java/org/apache/james/core/DomainTest.java new file mode 100644 index 00000000000..8a9e4a56214 --- /dev/null +++ b/core/src/test/java/org/apache/james/core/DomainTest.java @@ -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 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); + } +} + + diff --git a/core/src/test/java/org/apache/james/core/MailAddressTest.java b/core/src/test/java/org/apache/james/core/MailAddressTest.java index 86e5adece4b..e9874c9800f 100644 --- a/core/src/test/java/org/apache/james/core/MailAddressTest.java +++ b/core/src/test/java/org/apache/james/core/MailAddressTest.java @@ -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; @@ -55,6 +58,13 @@ private static Stream goodAddresses() { "\\.server-dev@james.apache.org", "Abc@10.42.0.1", "Abc.123@example.com", + "Loïc.Accentué@voilà.fr8", + "pelé@exemple.com", + "δοκιμή@παράδειγμα.δοκιμή", + "我買@屋企.香港", + "二ノ宮@黒川.日本", + "медведь@с-балалайкой.рф", + //"संपर्क@डाटामेल.भारत", fails in Jakarta, reason still unknown "user+mailbox/department=shipping@example.com", "user+mailbox@example.com", "\"Abc@def\"@example.com", @@ -96,26 +106,30 @@ private static Stream badAddresses() { "server-dev@[127.0.1.1.1]", "server-dev@[127.0.1.-1]", "test@dom+ain.com", + "test@xn--.example", "\"a..b\"@domain.com", // jakarta.mail is unable to handle this so we better reject it "server-dev\\.@james.apache.org", // jakarta.mail is unable to handle this so we better reject it "a..b@domain.com", - // 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\\,d@james.apache.org") .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(); } @@ -123,6 +137,7 @@ void testGoodMailAddressString(String mailAddress) { @MethodSource("goodAddresses") void toInternetAddressShouldNoop(String mailAddress) throws Exception { assertThat(new MailAddress(mailAddress).toInternetAddress()) + .as("tries to parse " + mailAddress + " using jakarta.mail") .isNotEmpty(); } @@ -130,6 +145,7 @@ void toInternetAddressShouldNoop(String mailAddress) throws Exception { @MethodSource("badAddresses") void testBadMailAddressString(String mailAddress) { Assertions.assertThatThrownBy(() -> new MailAddress(mailAddress)) + .as("fails to parse " + mailAddress) .isInstanceOf(AddressException.class); }