[url-detector] Fixing various bugs

Author: tzuhanjan

url-detector/src/main/java/com/linkedin/urls/detection/CharUtils.java

@@ -58,6 +58,10 @@ public static boolean isDot(char a) {
     return (a == '.' || a == '\u3002' || a == '\uFF0E' || a == '\uFF61');
   }
 
+  public static boolean isWhiteSpace(char a) {
+    return (a == '\n' || a == '\t' || a == '\r' || a == ' ');
+  }
+
   /**
    * Splits a string without the use of a regex, which could split either by isDot() or %2e
    * @param input the input string that will be split by dot
@@ -83,6 +87,5 @@ public static String[] splitByDot(String input) {
     }
     splitList.add(section.toString());
     return splitList.toArray(new String[splitList.size()]);
-
   }
 }

url-detector/src/main/java/com/linkedin/urls/detection/InputTextReader.java

@@ -53,7 +53,7 @@ public InputTextReader(String content) {
    */
   public char read() {
     char chr = _content[_index++];
-    return (chr == '\n' || chr == '\t') ? ' ' : chr;
+    return CharUtils.isWhiteSpace(chr) ? ' ' : chr;
   }
 
   /**

url-detector/src/main/java/com/linkedin/urls/detection/UrlDetector.java

@@ -295,6 +295,8 @@ private int processColon(int length) {
         length = _buffer.length(); //set length to be right after the scheme
     } else if (_buffer.length() > 0 && _options.hasFlag(UrlDetectorOptions.ALLOW_SINGLE_LEVEL_DOMAIN)
         && _reader.canReadChars(1)) { //takes care of case like hi:
+      _reader.goBack(); //unread the ":" so readDomainName can take care of the port
+      _buffer.delete(_buffer.length() - 1, _buffer.length());
       readDomainName(_buffer.toString());
     } else {
       readEnd(ReadEndState.InvalidUrl);

url-detector/src/main/java/com/linkedin/urls/url/HostNormalizer.java

@@ -81,9 +81,9 @@ private void normalizeHost() {
       return;
     }
 
-    host = UrlUtil.removeExtraDots(host).replace("\\x", "%");
+    host = UrlUtil.removeExtraDots(host);
 
-    _normalizedHost = UrlUtil.encode(host);
+    _normalizedHost = UrlUtil.encode(host).replace("\\x", "%");
   }
 
   /**

url-detector/src/main/java/com/linkedin/urls/url/NormalizedUrl.java

@@ -25,9 +25,8 @@ public NormalizedUrl(UrlMarker urlMarker) {
   /**
    * Returns a normalized url given a single url.
    */
-  public static NormalizedUrl createNormalized(String url)
-      throws MalformedURLException {
-    return normalize(create(url));
+  public static NormalizedUrl create(String url) throws MalformedURLException {
+    return normalize(Url.create(url));
   }
 
   /**

url-detector/src/main/java/com/linkedin/urls/url/PathNormalizer.java

@@ -9,6 +9,7 @@
  */
 package com.linkedin.urls.url;
 
+import java.util.Stack;
 import org.apache.commons.lang3.StringUtils;
 
 class PathNormalizer {
@@ -23,31 +24,50 @@ protected String normalizePath(String path) {
     if (StringUtils.isEmpty(path)) {
       return path;
     }
-    path = UrlUtil.removeSpecialSpaces(path);
     path = UrlUtil.decode(path);
     path = sanitizeDotsAndSlashes(path);
     return UrlUtil.encode(path);
   }
 
   /**
-   * Replaces "/../" and "/./" with "/" recursively.
+   * 1. Replaces "/./" with "/" recursively.
+   * 2. "/blah/asdf/.." -> "/blah"
+   * 3. "/blah/blah2/blah3/../../blah4" -> "/blah/blah4"
+   * 4. "//" -> "/"
+   * 5. Adds a slash at the end if there isn't one
    */
   private static String sanitizeDotsAndSlashes(String path) {
     StringBuilder stringBuilder = new StringBuilder(path);
+    Stack<Integer> slashIndexStack = new Stack<Integer>();
     int index = 0;
-    while (index < stringBuilder.length() - 2) {
-      if ( stringBuilder.charAt(index) == '/' && stringBuilder.charAt(index + 1) == '.') {
-        if (index + 3 < stringBuilder.length() && stringBuilder.charAt(index + 2) == '.' &&
-            stringBuilder.charAt(index + 3) == '/') {
-          stringBuilder.delete(index, index + 3); // "/../" -> "/"
-          index--; // backtrack so we can detect if this / is part of another replacement
-        } else if (stringBuilder.charAt(index + 2) == '/') {
-          stringBuilder.delete(index, index + 2); // "/./" -> "/"
-          index--; // backtrack so we can detect if this / is part of another replacement
+    while (index < stringBuilder.length() - 1) {
+      if (stringBuilder.charAt(index) == '/') {
+        slashIndexStack.add(index);
+        if (stringBuilder.charAt(index + 1) == '.') {
+          if (index < stringBuilder.length() - 2) {
+            if (stringBuilder.charAt(index + 2) == '.') {
+              slashIndexStack.pop();
+              int endIndex = index + 3;
+              // backtrack so we can detect if this / is part of another replacement
+              index = slashIndexStack.empty() ? 0 : slashIndexStack.pop();
+              stringBuilder.delete(index, endIndex); // "/asdf/../" -> ""
+            } else if (stringBuilder.charAt(index + 2) == '/') {
+              stringBuilder.delete(index, index + 2); // "/./" -> "/"
+              index--; // backtrack so we can detect if this / is part of another replacement
+            }
+          }
+        } else if (stringBuilder.charAt(index + 1) == '/') {
+          stringBuilder.deleteCharAt(index);
+          index--;
         }
       }
       index++;
     }
+
+    if (stringBuilder.length() == 0) {
+      stringBuilder.append("/"); //Every path has at least a slash
+    }
+
     return stringBuilder.toString();
   }
 }

url-detector/src/main/java/com/linkedin/urls/url/Url.java

@@ -61,7 +61,8 @@ protected Url(UrlMarker urlMarker) {
    */
   public static Url create(String url)
       throws MalformedURLException {
-    List<Url> urls = new UrlDetector(url, UrlDetectorOptions.ALLOW_SINGLE_LEVEL_DOMAIN).detect();
+    String formattedString = UrlUtil.removeSpecialSpaces(url.trim().replace(" ", "%20"));
+    List<Url> urls = new UrlDetector(formattedString, UrlDetectorOptions.ALLOW_SINGLE_LEVEL_DOMAIN).detect();
     if (urls.size() == 1) {
       return urls.get(0);
     } else if (urls.size() == 0) {

url-detector/src/main/java/com/linkedin/urls/url/UrlUtil.java

@@ -11,15 +11,10 @@
 
 import com.linkedin.urls.detection.CharUtils;
 import com.linkedin.urls.detection.InputTextReader;
-import java.net.IDN;
 import java.util.Stack;
 
 class UrlUtil {
 
-  private static final char TAB = 0x09;
-  private static final char CR = 0x0d;
-  private static final char LF = 0x0a;
-
   /**
    * Decodes the url by iteratively removing hex characters with backtracking.
    * For example: %2525252525252525 becomes %
@@ -65,7 +60,7 @@ protected static String removeSpecialSpaces(String urlPart) {
     StringBuilder stringBuilder = new StringBuilder(urlPart);
     for (int i = 0; i < stringBuilder.length(); i++) {
       char curr = stringBuilder.charAt(i);
-      if (curr == TAB || curr == CR || curr == LF) {
+      if (CharUtils.isWhiteSpace(curr)) {
         stringBuilder.deleteCharAt(i);
       }
     }

url-detector/src/test/java/com/linkedin/urls/url/TestNormalizedUrl.java

@@ -34,9 +34,62 @@ private Object[][] getHostPathUrls() {
   @Test(dataProvider = "getHostPathUrls")
   public void testUsernamePasswordUrls(String testString, String host, String path)
       throws MalformedURLException {
-    Url url = NormalizedUrl.createNormalized(testString);
+    Url url = NormalizedUrl.create(testString);
     Assert.assertNotNull(url);
     Assert.assertEquals(url.getHost(), host);
     Assert.assertEquals(url.getPath(), path);
   }
+
+
+  /**
+   * https://developers.google.com/safe-browsing/developers_guide_v3#Canonicalization
+   * @return
+   */
+  @DataProvider
+  private Object[][] getFullUrls() {
+    return new Object[][] {
+        {"http://host/%25%32%35", "http://host/%25"},
+        {"http://host/%25%32%35%25%32%35", "http://host/%25%25"},
+        {"http://host/%2525252525252525", "http://host/%25"},
+        {"http://host/asdf%25%32%35asd", "http://host/asdf%25asd"},
+        {"http://host/%%%25%32%35asd%%", "http://host/%25%25%25asd%25%25"},
+        {"http://www.google.com/", "http://www.google.com/"},
+        {"http://%31%36%38%2e%31%38%38%2e%39%39%2e%32%36/%2E%73%65%63%75%72%65/%77%77%77%2E%65%62%61%79%2E%63%6F%6D/",
+            "http://168.188.99.26/.secure/www.ebay.com/"},
+        {"http://195.127.0.11/uploads/%20%20%20%20/.verify/.eBaysecure=updateuserdataxplimnbqmn-xplmvalidateinfoswqpcmlx=hgplmcx/",
+            "http://195.127.0.11/uploads/%20%20%20%20/.verify/.eBaysecure=updateuserdataxplimnbqmn-xplmvalidateinfoswqpcmlx=hgplmcx/"},
+        {"http://host%23.com/%257Ea%2521b%2540c%2523d%2524e%25f%255E00%252611%252A22%252833%252944_55%252B",
+            "http://host%23.com/~a!b@c%23d$e%25f^00&11*22(33)44_55+"},
+        {"http://3279880203/blah", "http://195.127.0.11/blah"},
+        {"http://www.google.com/blah/..", "http://www.google.com/"},
+        {"www.google.com/", "http://www.google.com/"},
+        {"www.google.com", "http://www.google.com/"},
+        {"http://www.evil.com/blah#frag", "http://www.evil.com/blah"},
+        {"http://www.GOOgle.com/", "http://www.google.com/"},
+        {"http://www.google.com/foo\tbar\rbaz\n2", "http://www.google.com/foobarbaz2"},
+        {"http://www.google.com/q?", "http://www.google.com/q?"},
+        {"http://www.google.com/q?r?", "http://www.google.com/q?r?"},
+        {"http://www.google.com/q?r?s", "http://www.google.com/q?r?s"},
+        {"http://evil.com/foo#bar#baz", "http://evil.com/foo"},
+        {"http://evil.com/foo;", "http://evil.com/foo;"},
+        {"http://evil.com/foo?bar;", "http://evil.com/foo?bar;"},
+        {"http://\\x01\\x80.com/", "http://%01%80.com/"},
+        {"http://notrailingslash.com", "http://notrailingslash.com/"},
+        {"http://www.gotaport.com:1234/", "http://www.gotaport.com:1234/"},
+        {"  http://www.google.com/  ", "http://www.google.com/"},
+        {"http:// leadingspace.com/", "http://%20leadingspace.com/"},
+        {"http://%20leadingspace.com/", "http://%20leadingspace.com/"},
+        {"%20leadingspace.com/", "http://%20leadingspace.com/"},
+        {"https://www.securesite.com/", "https://www.securesite.com/"},
+        {"http://host.com/ab%23cd", "http://host.com/ab%23cd"},
+        {"http://host.com//twoslashes?more//slashes", "http://host.com/twoslashes?more//slashes"},
+        {"http://go.co/a/b/../c", "http://go.co/a/c"}
+    };
+  }
+
+  @Test(dataProvider = "getFullUrls")
+  public void testFullUrls(String host, String expectedHost) throws MalformedURLException {
+    NormalizedUrl normalizedUrl = NormalizedUrl.create(host);
+    Assert.assertEquals(normalizedUrl.getFullUrlWithoutFragment(), expectedHost);
+  }
 }

url-detector/src/test/java/com/linkedin/urls/url/TestPathNormalizer.java

@@ -1,10 +1,10 @@
 /**
  * Copyright 2015 LinkedIn Corp. All rights reserved.
- * Licensed 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
+ * Licensed 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.
+ * 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.
  *
  */
 package com.linkedin.urls.url;
@@ -30,12 +30,19 @@ private Object[][] getPaths() {
             "/uploads/%20%20%20%20/.verify/.eBaysecure=updateuserdataxplimnbqmn-xplmvalidateinfoswqpcmlx=hgplmcx/"},
         {"/%257Ea%2521b%2540c%2523d%2524e%25f%255E00%252611%252A22%252833%252944_55%252B",
             "/~a!b@c%23d$e%25f^00&11*22(33)44_55+"},
-        {"/lala/.././../..../","/lala/..../"}
+        {"/lala/.././../..../", "/..../"},
+        {"//asdfasdf/awef/sadf/sdf//", "/asdfasdf/awef/sadf/sdf/"},
+        {"/", "/"},
+        {"/a/../b/c", "/b/c"},
+        {"/blah/..", "/"},
+        {"../", "../"},
+        {"/asdf/.", "/asdf/."}
+
     };
   }
 
   @Test(dataProvider = "getPaths")
-  public void testSanityAddresses(String path, String expectedPath) {
+  public void testPaths(String path, String expectedPath) {
     PathNormalizer pathNormalizer = new PathNormalizer();
 
     Assert.assertEquals(pathNormalizer.normalizePath(path), expectedPath);