#2198 preserve query strings containing slashes

core/src/main/java/org/apache/stormcrawler/filtering/basic/BasicURLNormalizer.java

@@ -336,23 +336,32 @@ public int compare(NameValuePair p1, NameValuePair p2) {
             };
 
     /**
-     * A common error to find is a query string that starts with an & instead of a ? This will fix
-     * that error. So http://foo.com&a=b will be changed to http://foo.com?a=b.
+     * Repairs a query string that starts with an & instead of a ?. The query check ignores the
+     * fragment, and the repair only checks the last path segment so an & in an earlier path segment
+     * is not mistaken for a query separator.
      *
      * @param urlToFilter the url to filter
      * @return corrected url
      */
     private String unmangleQueryString(String urlToFilter) {
-        String[] pathElements = urlToFilter.split("/");
-        final String lastPathElement = pathElements[pathElements.length - 1];
-        int firstAmp = lastPathElement.indexOf('&');
+        if (urlToFilter.indexOf('&') == -1) {
+            return urlToFilter;
+        }
+
+        int fragmentStart = urlToFilter.indexOf('#');
+        String fragment = fragmentStart == -1 ? "" : urlToFilter.substring(fragmentStart);
+        String url = fragmentStart == -1 ? urlToFilter : urlToFilter.substring(0, fragmentStart);
+        if (url.indexOf('?') != -1) {
+            return urlToFilter;
+        }
+
+        int lastSlash = url.lastIndexOf('/');
+        int firstAmp = url.indexOf('&', lastSlash + 1);
         if (firstAmp == -1) {
             return urlToFilter;
         }
-        int firstQuestionMark = lastPathElement.indexOf('?');
-        if (firstQuestionMark == -1 && lastPathElement.indexOf("=") > 0) {
-            pathElements[pathElements.length - 1] = lastPathElement.replaceFirst("&", "?");
-            return String.join("/", pathElements);
+        if (url.indexOf('=', firstAmp + 1) > firstAmp) {
+            return url.substring(0, firstAmp) + '?' + url.substring(firstAmp + 1) + fragment;
         }
         return urlToFilter;
     }

core/src/test/java/org/apache/stormcrawler/filtering/BasicURLNormalizerTest.java

@@ -35,6 +35,8 @@
 import org.apache.stormcrawler.util.URLUtil;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
 
 /**
  * Utility class which encapsulates the filtering of URLs based on the hostname or domain of the
@@ -243,6 +245,76 @@ void testFixMangledQueryString() throws MalformedURLException {
         assertEquals(expectedResult, normalizedUrl, "Failed to filter query string");
     }
 
+    @ParameterizedTest
+    @CsvSource({
+        "http://a.example/article.pl?sid=01/10/23/1816257&mode=thread&tid=107",
+        "http://a.example/p?u=http://b.example/x&z=1",
+        "http://a.example/login/?next=/a/b/&utm_source=ref&z=1"
+    })
+    void testDoesNotMangleQueryWhenValueContainsSlashes(String inputUrl)
+            throws MalformedURLException {
+        URLFilter urlFilter = createFilter(new ObjectNode(JsonNodeFactory.instance));
+        URL testSourceUrl = URLUtil.toURL("http://a.example/");
+        assertEquals(
+                inputUrl,
+                urlFilter.filter(testSourceUrl, new Metadata(), inputUrl),
+                "An existing query must not be unmangled because a value contains slashes");
+    }
+
+    @Test
+    void testUnmanglesQueryBeforeFragmentQuestionMark() throws MalformedURLException {
+        URLFilter urlFilter = createFilter(false, false);
+        URL testSourceUrl = URLUtil.toURL("http://a.example/");
+        String inputUrl = "http://a.example/p&a=b#frag?x";
+        assertEquals(
+                "http://a.example/p?a=b#frag?x",
+                urlFilter.filter(testSourceUrl, new Metadata(), inputUrl),
+                "A question mark in the fragment must not suppress query repair");
+    }
+
+    @ParameterizedTest
+    @CsvSource({"false, http://a.example/x&a=b/#frag", "true, http://a.example/x&a=b/"})
+    void testDoesNotDropTrailingSlashWhenUnmangling(boolean removeAnchor, String inputUrl)
+            throws MalformedURLException {
+        URLFilter urlFilter = createFilter(removeAnchor, true, List.of());
+        URL testSourceUrl = URLUtil.toURL("http://a.example/");
+        assertEquals(
+                inputUrl,
+                urlFilter.filter(testSourceUrl, new Metadata(), inputUrl),
+                "The URL must remain unchanged when the last path segment has no query parameters");
+    }
+
+    @ParameterizedTest
+    @CsvSource({
+        "http://a.example/p&a=b, http://a.example/p?a=b",
+        "http://a.example/p&a=b&c=d, http://a.example/p?a=b&c=d"
+    })
+    void testRepairsMangledQueryString(String inputUrl, String expectedUrl)
+            throws MalformedURLException {
+        URLFilter urlFilter = createFilter(false, true, List.of());
+        URL testSourceUrl = URLUtil.toURL("http://a.example/");
+        assertEquals(expectedUrl, urlFilter.filter(testSourceUrl, new Metadata(), inputUrl));
+    }
+
+    @Test
+    void testDoesNotRewriteAmpersandInFragment() throws MalformedURLException {
+        URLFilter urlFilter = createFilter(false, true, List.of());
+        URL testSourceUrl = URLUtil.toURL("http://a.example/");
+        String inputUrl = "http://a.example/p#a&b=c";
+        assertEquals(inputUrl, urlFilter.filter(testSourceUrl, new Metadata(), inputUrl));
+    }
+
+    @Test
+    void testRemovesQueryElementWhenValueContainsSlashes() throws MalformedURLException {
+        URLFilter urlFilter = createFilter(List.of("utm_source"));
+        URL testSourceUrl = URLUtil.toURL("http://a.example/");
+        String inputUrl = "http://a.example/login/?next=/a/b/&utm_source=ref&z=1";
+        assertEquals(
+                "http://a.example/login/?next=%2Fa%2Fb%2F&z=1",
+                urlFilter.filter(testSourceUrl, new Metadata(), inputUrl),
+                "Query elements following a slash-containing value must remain removable");
+    }
+
     @Test
     void testProperURLEncodingWithoutQueryParameter() throws MalformedURLException {
         URLFilter urlFilter = createFilter(queryParamsToFilter);