From cbfd4e80921496060f5dd9d80145e84ed6a49150 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 6 Jun 2025 15:39:09 -0700 Subject: [PATCH 1/5] Do not use commons-validator for domain validation --- .../com/maxmind/minfraud/request/Email.java | 49 ++++++++++++++++++- .../maxmind/minfraud/request/EmailTest.java | 29 +++++++++-- 2 files changed, 73 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/maxmind/minfraud/request/Email.java b/src/main/java/com/maxmind/minfraud/request/Email.java index f0b5f1dc..f6dab401 100644 --- a/src/main/java/com/maxmind/minfraud/request/Email.java +++ b/src/main/java/com/maxmind/minfraud/request/Email.java @@ -13,7 +13,6 @@ import java.util.HashMap; import java.util.Map; import java.util.regex.Pattern; -import org.apache.commons.validator.routines.DomainValidator; import org.apache.commons.validator.routines.EmailValidator; /** @@ -28,6 +27,9 @@ public final class Email extends AbstractModel { private static final Map equivalentDomains; private static final Map fastmailDomains; private static final Map yahooDomains; + private static final Pattern DOMAIN_LABEL_PATTERN = Pattern.compile( + "^[a-zA-Z0-9]$|^[a-zA-Z0-9][a-zA-Z0-9\\-]*[a-zA-Z0-9]$" + ); private static final Pattern DOT_PATTERN = Pattern.compile("\\."); private static final Pattern TRAILING_DOT_PATTERN = Pattern.compile("\\.+$"); private static final Pattern REPEAT_COM_PATTERN = Pattern.compile("(?:\\.com){2,}$"); @@ -373,7 +375,7 @@ public Email.Builder hashAddress() { * @throws IllegalArgumentException when domain is not a valid domain. */ public Email.Builder domain(String domain) { - if (enableValidation && !DomainValidator.getInstance().isValid(domain)) { + if (enableValidation && !isValidDomain(domain)) { throw new IllegalArgumentException("The email domain " + domain + " is not valid."); } this.domain = domain; @@ -491,6 +493,49 @@ private String cleanDomain(String domain) { return domain; } + private static boolean isValidDomain(String domain) { + if (domain == null || domain.isEmpty()) { + return false; + } + + String asciiDomain; + try { + asciiDomain = IDN.toASCII(domain); + } catch (IllegalArgumentException e) { + return false; + } + + if (domain.endsWith(".")) { + domain = domain.substring(0, domain.length() - 1); + } + + if (asciiDomain.length() > 255) { + return false; + } + + String[] labels = asciiDomain.split("\\."); + + if (labels.length < 2) { + return false; + } + + for (String label : labels) { + if (!isValidDomainLabel(label)) { + return false; + } + } + + return true; + } + + private static boolean isValidDomainLabel(String label) { + if (label == null || label.isEmpty() || label.length() > 63) { + return false; + } + + return DOMAIN_LABEL_PATTERN.matcher(label).matches(); + } + /** * @return The domain of the email address used in the transaction. */ diff --git a/src/test/java/com/maxmind/minfraud/request/EmailTest.java b/src/test/java/com/maxmind/minfraud/request/EmailTest.java index 4025f1a3..f7cf83b3 100644 --- a/src/test/java/com/maxmind/minfraud/request/EmailTest.java +++ b/src/test/java/com/maxmind/minfraud/request/EmailTest.java @@ -12,6 +12,8 @@ import java.util.HashMap; import java.util.Map; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; public class EmailTest { @@ -264,11 +266,32 @@ public void testDomainWithoutValidation() { assertEquals(domain, email.getDomain()); } - @Test - public void testInvalidDomain() { + @ParameterizedTest(name = "Run #{index}: domain = \"{0}\"") + @ValueSource(strings = { + "example", + "", + " ", + " domain.com", + "domain.com ", + "domain com.com", + "domain_name.com", + "domain$.com", + "-domain.com", + "domain-.com", + "domain..com", + ".domain.com", + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.com", + "a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a" + + ".a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a" + + ".a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a" + + ".a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a" + + ".com", + "xn--.com" + }) + void testInvalidDomains(String invalidDomain) { assertThrows( IllegalArgumentException.class, - () -> new Builder().domain(" domain.com").build() + () -> new Builder().domain(invalidDomain).build() ); } } From ab5105c14a94e4b86d9bd1b59a49577e1ea907fe Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 9 Jun 2025 08:12:50 -0700 Subject: [PATCH 2/5] Replace commons-validator email validation This isn't quite as strict, but it is a good enough "looks like an email" check for this purpose. --- .../com/maxmind/minfraud/request/Email.java | 31 +++++++++++++++++-- .../maxmind/minfraud/request/EmailTest.java | 22 +++++++++++-- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/maxmind/minfraud/request/Email.java b/src/main/java/com/maxmind/minfraud/request/Email.java index f6dab401..0ffe1a0d 100644 --- a/src/main/java/com/maxmind/minfraud/request/Email.java +++ b/src/main/java/com/maxmind/minfraud/request/Email.java @@ -13,7 +13,6 @@ import java.util.HashMap; import java.util.Map; import java.util.regex.Pattern; -import org.apache.commons.validator.routines.EmailValidator; /** * The email information for the transaction. @@ -340,7 +339,7 @@ public Builder(boolean enableValidation) { * @throws IllegalArgumentException when address is not a valid email address. */ public Email.Builder address(String address) { - if (enableValidation && !EmailValidator.getInstance().isValid(address)) { + if (enableValidation && !isValidEmail(address)) { throw new IllegalArgumentException( "The email address " + address + " is not valid."); } @@ -460,6 +459,34 @@ private String cleanAddress(String address) { return localPart + "@" + domain; } + private static boolean isValidEmail(String email) { + if (email == null || email.isEmpty()) { + return false; + } + + // RFC 5321 the forward path limits the mailbox to 254 characters + // even though a domain can be 255 and the local part 64 + if (email.length() > 254) { + return false; + } + + int atIndex = email.lastIndexOf('@'); + if (atIndex <= 0) { + return false; + } + + String localPart = email.substring(0, atIndex); + String domainPart = email.substring(atIndex + 1); + + // The local-part has a maximum length of 64 characters. + if (localPart.length() > 64) { + return false; + } + + return isValidDomain(domainPart); + } + + private String cleanDomain(String domain) { if (domain == null) { return null; diff --git a/src/test/java/com/maxmind/minfraud/request/EmailTest.java b/src/test/java/com/maxmind/minfraud/request/EmailTest.java index f7cf83b3..687c2519 100644 --- a/src/test/java/com/maxmind/minfraud/request/EmailTest.java +++ b/src/test/java/com/maxmind/minfraud/request/EmailTest.java @@ -4,6 +4,7 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; + import com.maxmind.minfraud.request.Email.Builder; import java.math.BigInteger; import java.nio.charset.StandardCharsets; @@ -244,11 +245,26 @@ private String toMD5(String s) throws NoSuchAlgorithmException { return String.format("%032x", i); } - @Test - public void testInvalidAddress() { + @ParameterizedTest(name = "Run #{index}: email = \"{0}\"") + @ValueSource(strings = { + "test.com", + "test@", + "@test.com", + "", + " ", + "test@test com.com", + "test@test_domain.com", + "test@-test.com", + "test@test-.com", + "test@.test.com", + "test@test..com", + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa@test.com", + "test@aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.com" + }) + void testInvalidAddresses(String invalidAddress) { assertThrows( IllegalArgumentException.class, - () -> new Builder().address("a@test@test.org").build() + () -> new Builder().address(invalidAddress).build() ); } From a93e832044b3599cbeb9a80db22619755f3226c3 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 9 Jun 2025 08:19:15 -0700 Subject: [PATCH 3/5] Remove commons-validator as a dependency --- CHANGELOG.md | 9 +++++++++ pom.xml | 5 ----- src/main/java/module-info.java | 1 - 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fd57892..2729ee78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,15 @@ CHANGELOG ========= +3.8.0 +------------------ + +* `commons-validator` has been removed as a dependency. This module now does + its own email and domain name validation. This was done to reduce the number + of dependencies and any security vulnerabilities in them. The new email + validation of the local part is somewhat more lax than the previous + validation. + 3.7.2 (2025-05-28) ------------------ diff --git a/pom.xml b/pom.xml index 43023b6a..be0692fb 100644 --- a/pom.xml +++ b/pom.xml @@ -62,11 +62,6 @@ geoip2 4.3.1 - - commons-validator - commons-validator - 1.9.0 - org.junit.jupiter junit-jupiter diff --git a/src/main/java/module-info.java b/src/main/java/module-info.java index dfc2ebf6..0d7600fb 100644 --- a/src/main/java/module-info.java +++ b/src/main/java/module-info.java @@ -7,7 +7,6 @@ requires com.fasterxml.jackson.databind; requires com.fasterxml.jackson.datatype.jsr310; requires transitive com.maxmind.geoip2; - requires org.apache.commons.validator; requires java.net.http; exports com.maxmind.minfraud; From c847c5966c6f9ddd08e59fff1939feccf9cb0d3f Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 9 Jun 2025 13:53:48 -0700 Subject: [PATCH 4/5] Use correct value when handling FQDNs I do this by getting rid of the asciiDomain variable as that just makes it easier to use the wrong one. --- src/main/java/com/maxmind/minfraud/request/Email.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/maxmind/minfraud/request/Email.java b/src/main/java/com/maxmind/minfraud/request/Email.java index 0ffe1a0d..9015303e 100644 --- a/src/main/java/com/maxmind/minfraud/request/Email.java +++ b/src/main/java/com/maxmind/minfraud/request/Email.java @@ -525,9 +525,8 @@ private static boolean isValidDomain(String domain) { return false; } - String asciiDomain; try { - asciiDomain = IDN.toASCII(domain); + domain = IDN.toASCII(domain); } catch (IllegalArgumentException e) { return false; } @@ -536,11 +535,11 @@ private static boolean isValidDomain(String domain) { domain = domain.substring(0, domain.length() - 1); } - if (asciiDomain.length() > 255) { + if (domain.length() > 255) { return false; } - String[] labels = asciiDomain.split("\\."); + String[] labels = domain.split("\\."); if (labels.length < 2) { return false; From dc526d74af9fe644f2c8dde57caf0572c30b1999 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Mon, 9 Jun 2025 13:55:34 -0700 Subject: [PATCH 5/5] Add missing word --- src/main/java/com/maxmind/minfraud/request/Email.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/maxmind/minfraud/request/Email.java b/src/main/java/com/maxmind/minfraud/request/Email.java index 9015303e..e4f94e9e 100644 --- a/src/main/java/com/maxmind/minfraud/request/Email.java +++ b/src/main/java/com/maxmind/minfraud/request/Email.java @@ -464,7 +464,7 @@ private static boolean isValidEmail(String email) { return false; } - // RFC 5321 the forward path limits the mailbox to 254 characters + // In RFC 5321, the forward path limits the mailbox to 254 characters // even though a domain can be 255 and the local part 64 if (email.length() > 254) { return false;