Repository: crawler-commons/crawler-commons (java)

Issue:
EffectiveTldFinder throws IllegalArgumentException on IDN domain names containing prohibited characters
EffectiveTldFinder throws an IllegalArgumentException when parsing domain names containing characters prohibited in [Internationalized Domain Names](https://en.wikipedia.org/wiki/Internationalized_domain_name) (IDNs):
```
Exception in thread "main" java.lang.IllegalArgumentException: java.text.ParseException: A prohibited code point was found in the inputp?
        at java.net.IDN.toASCIIInternal(IDN.java:274)
        at java.net.IDN.toASCII(IDN.java:122)
        at java.net.IDN.toASCII(IDN.java:151)
        at crawlercommons.domains.EffectiveTldFinder$EffectiveTLD.asciiConvert(EffectiveTldFinder.java:480)
        at crawlercommons.domains.EffectiveTldFinder$EffectiveTLD.normalizeName(EffectiveTldFinder.java:428)
        at crawlercommons.domains.EffectiveTldFinder$EffectiveTLD.<init>(EffectiveTldFinder.java:416)
        at crawlercommons.domains.EffectiveTldFinder.findEffectiveTLD(EffectiveTldFinder.java:235)
        at crawlercommons.domains.EffectiveTldFinder.getAssignedDomain(EffectiveTldFinder.java:289)
        at crawlercommons.domains.EffectiveTldFinder.main(EffectiveTldFinder.java:391)
Caused by: java.text.ParseException: A prohibited code point was found in the inputp?
        at sun.net.idn.StringPrep.prepare(StringPrep.java:450)
        at java.net.IDN.toASCIIInternal(IDN.java:272)
        ... 8 more
```

Specifically, the exception was caused by a wrong encoding conversion which caused that the domain name contained the replacement character "�" (U+FFFD) which is prohibited by [RFC3490](https://tools.ietf.org/html/rfc3490) resp. [RFC3454](https://tools.ietf.org/html/rfc3454#page-78). The exception only occurs if the prohibited character is contained in the dot-separated segment "matched" by a wildcard public suffix (e.g., `*.kawasaki.jp`). 

A fix should address these two points:
1. catch the runtime exception and return null or the host name depending on the configuration (parameter `strict`). Alternatively, declare and document it to avoid that applications unexpectedly crash.
2. define the behavior of EffectiveTldFinder in case the domain or host name contains prohibited characters and there is no valid ASCII representation.



Pull request:
EffectiveTldFinder throws IllegalArgumentException on IDN domain names containing prohibited characters, fixes #231
- catch IllegalArgumentException when converting IDNs to ASCII
- validate domain names returned by getAssignedDomain(...)
- add unit tests, complete Javadoc

The behavior of `EffectiveTldFinder.getAssignedDomain(hostname)` in case there are prohibited characters is now (see updated unit test):
1. in the public suffix ("effective TLD") or the domain name: return null or hostname depending on the `strict` flag
2. as part of the hostname but not the domain name part: return the valid domain name



Patch:
diff --git a/CHANGES.txt b/CHANGES.txt
index b6efd03..e9a66f9 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,6 +1,7 @@
 Crawler-Commons Change Log
 
 Current Development 0.11-SNAPSHOT (yyyy-mm-dd)
+- EffectiveTldFinder throws IllegalArgumentException on IDN domain names containing prohibited characters (sebastian-nagel) #231
 - [Sitemaps] Trim Unicode whitespace around URLs (sebastian-nagel, kkrugler) #224
 - [Sitemaps] Sitemap index: stop URL at closing </loc> (sebastian-nagel, kkrugler) #213
 - [Sitemaps] Allow empty price in video sitemaps (sebastian-nagel) #221
diff --git a/src/main/java/crawlercommons/domains/EffectiveTldFinder.java b/src/main/java/crawlercommons/domains/EffectiveTldFinder.java
index 79df473..3f0d33d 100644
--- a/src/main/java/crawlercommons/domains/EffectiveTldFinder.java
+++ b/src/main/java/crawlercommons/domains/EffectiveTldFinder.java
@@ -222,6 +222,7 @@ public class EffectiveTldFinder {
                 if (offset == 0 || foundTld.isException() || !foundTld.isWild()) {
                     return res;
                 }
+
                 // wildcards create an open ETLD namespace
                 int wildcardOffset = hostname.lastIndexOf(DOT, offset - 2);
                 String retryTld;
@@ -232,7 +233,14 @@ public class EffectiveTldFinder {
                 } else {
                     retryTld = hostname.substring(wildcardOffset + 1);
                 }
-                foundTld = new EffectiveTLD(retryTld, foundTld.isPrivate);
+
+                try {
+                    foundTld = new EffectiveTLD(retryTld, foundTld.isPrivate);
+                } catch (IllegalArgumentException e) {
+                    // retryTld contains forbidden characters
+                    return null;
+                }
+
                 return new SuffixTrie.LookupResult<EffectiveTLD>(wildcardOffset + 1, foundTld);
             }
         }
@@ -319,6 +327,15 @@ public class EffectiveTldFinder {
                 break;
             start = pos + 1;
         }
+        String domainSegment = hostname.substring(start, etldStartPos);
+        if (!EffectiveTLD.isAscii(domainSegment)) {
+            try {
+                IDN.toASCII(domainSegment);
+            } catch (IllegalArgumentException e) {
+                // not a valid IDN segment
+                return (strict ? null : hostname);
+            }
+        }
         return hostname.substring(start);
     }
 
@@ -394,6 +411,17 @@ public class EffectiveTldFinder {
         }
     }
 
+    /**
+     * EffectiveTLD objects hold one line of the public suffix list:
+     * <ul>
+     * <li>the suffix (<code>com</code>, <code>co.uk</code>, etc.)</li>
+     * <li>for IDN suffixes: both the ASCII and IDN variant
+     * (<code>xn--p1ai</code> and <code>рф</code>)</li>
+     * <li>and the properties required to parse host/domain names given in the
+     * public suffix list (wildcard suffix, exception, in private domain
+     * section)</li>
+     * </ul>
+     */
     public static class EffectiveTLD {
 
         private boolean exception = false;
@@ -402,7 +430,20 @@ public class EffectiveTldFinder {
         private String domain = null;
         private String idn = null;
 
-        public EffectiveTLD(String line, boolean isPrivateDomain) {
+        /**
+         * Parse one non-empty, non-comment line in the public suffix list and
+         * hold the public suffix and its properties in the created object.
+         * 
+         * @param line
+         *            non-empty, non-comment line in the public suffix list
+         * @param isPrivateDomain
+         *            whether line is in the section of &quot;PRIVATE
+         *            DOMAINS&quot; of the public suffix list
+         * @throws IllegalArgumentException
+         *             if the input line contains non-ASCII Unicode characters
+         *             prohibited in IDNs, cf. {@link IDN#toASCII(String)}
+         */
+        public EffectiveTLD(String line, boolean isPrivateDomain) throws IllegalArgumentException {
             if (line.startsWith(EXCEPTION)) {
                 exception = true;
                 domain = line.substring(EXCEPTION.length(), line.length());
@@ -421,7 +462,18 @@ public class EffectiveTldFinder {
             isPrivate = isPrivateDomain;
         }
 
-        private String normalizeName(String name) {
+        /**
+         * Normalize a domain name: convert characters into to lowercase and
+         * encode dot-separated segments containing non-ASCII characters. Cf.
+         * {@link #asciiConvert(String)} and {@link IDN#toASCII(String)}
+         * 
+         * @param str
+         *            domain name segment
+         * @return normalized domain name containing only ASCII characters
+         * @throws IllegalArgumentException
+         *             if the input contains prohibited characters
+         */
+        private String normalizeName(String name) throws IllegalArgumentException {
             String[] parts = name.split(DOT_REGEX);
             String[] ary = new String[parts.length];
             for (int i = 0; i < parts.length; i++) {
@@ -473,14 +525,24 @@ public class EffectiveTldFinder {
             return res;
         }
 
-        private String asciiConvert(String str) {
+        /**
+         * Converts a single domain name segment (separated by dots) to ASCII if
+         * it contains non-ASCII character, cf. {@link IDN#toASCII(String)}.
+         * 
+         * @param str
+         *            domain name segment
+         * @return ASCII "Punycode" representation of the domain name segment
+         * @throws IllegalArgumentException
+         *             if the input contains prohibited characters
+         */
+        private static String asciiConvert(String str) throws IllegalArgumentException {
             if (isAscii(str)) {
                 return str.toLowerCase(Locale.ROOT);
             }
             return IDN.toASCII(str);
         }
 
-        private boolean isAscii(String str) {
+        private static boolean isAscii(String str) {
             char[] chars = str.toCharArray();
             for (char c : chars) {
                 if (c > 127) {


Test patch:
diff --git a/src/test/java/crawlercommons/domains/EffectiveTldFinderTest.java b/src/test/java/crawlercommons/domains/EffectiveTldFinderTest.java
index 53b458e..9a0e088 100644
--- a/src/test/java/crawlercommons/domains/EffectiveTldFinderTest.java
+++ b/src/test/java/crawlercommons/domains/EffectiveTldFinderTest.java
@@ -219,6 +219,20 @@ public class EffectiveTldFinderTest {
         // in strict mode: there should nothing be returned for invalid
         // hostnames
         assertNull(EffectiveTldFinder.getAssignedDomain("www..example..com", true, false));
+        // prohibited Unicode characters in internationalized domain name (IDN),
+        // test for #231): � (U+FFFD REPLACEMENT CHARACTER) is prohibited by
+        // RFC3490
+        // (1) in public suffix
+        assertNull(EffectiveTldFinder.getAssignedDomain("www.example.c\ufffdm", true, false));
+        // (1a) in wildcard part of a public suffix
+        assertNull(EffectiveTldFinder.getAssignedDomain("\ufffd.kawasaki.jp", true, false));
+        // (2) in dot-separated segment immediately before public suffix
+        // => fail / null : there is no valid domain name
+        assertNull(EffectiveTldFinder.getAssignedDomain("www.ex\ufffdmple.com", true, false));
+        // (3) in dot-separated segment not part of the domain name
+        // => "example.com" is a valid domain name, no check whether the input
+        // host name is valid
+        assertEquals("example.com", EffectiveTldFinder.getAssignedDomain("\ufffd.example.com", true, false));
     }
 
 }