Avoid parsing and decoding the query when extracting the request path prefix

httpclient5/src/main/java/org/apache/hc/client5/http/impl/RequestSupport.java

@@ -43,31 +43,106 @@
 @Internal
 public final class RequestSupport {
 
+    /**
+     * Extracts the path prefix (the path component up to and including the last segment
+     * separator) of the request, with dot segments removed and percent encoding normalized.
+     * The query and the fragment are ignored.
+     */
     public static String extractPathPrefix(final HttpRequest request) {
         final String path = request.getPath();
+        final int pathEnd = findPathEnd(path);
+        final String prefix = extractNormalizedPathPrefix(path, pathEnd);
+        if (prefix != null) {
+            return prefix;
+        }
+        if (pathEnd < path.length()) {
+            try {
+                // Avoid parsing and decoding the query
+                return normalizePathPrefix(new URIBuilder(path.substring(0, pathEnd)));
+            } catch (final URISyntaxException ignore) {
+                // The path alone may not be a valid URI reference (such as "//" from "//?query")
+            }
+        }
         try {
-            final URIBuilder uriBuilder = new URIBuilder(path);
-            uriBuilder.setFragment(null);
-            uriBuilder.clearParameters();
-            uriBuilder.optimize();
-            final List<String> pathSegments = uriBuilder.getPathSegments();
+            return normalizePathPrefix(new URIBuilder(path));
+        } catch (final URISyntaxException ex) {
+            return path;
+        }
+    }
 
-            if (!pathSegments.isEmpty()) {
-                pathSegments.remove(pathSegments.size() - 1);
-            }
-            if (pathSegments.isEmpty()) {
-                return "/";
-            }
-            final StringBuilder buf = new StringBuilder();
+    private static String normalizePathPrefix(final URIBuilder uriBuilder) {
+        uriBuilder.setFragment(null);
+        uriBuilder.clearParameters();
+        uriBuilder.optimize();
+        final List<String> pathSegments = uriBuilder.getPathSegments();
+
+        if (!pathSegments.isEmpty()) {
+            pathSegments.remove(pathSegments.size() - 1);
+        }
+        if (pathSegments.isEmpty()) {
+            return "/";
+        }
+        final StringBuilder buf = new StringBuilder();
+        buf.append('/');
+        for (final String pathSegment : pathSegments) {
+            PercentCodec.encode(buf, pathSegment, StandardCharsets.US_ASCII);
             buf.append('/');
-            for (final String pathSegment : pathSegments) {
-                PercentCodec.encode(buf, pathSegment, StandardCharsets.US_ASCII);
-                buf.append('/');
+        }
+        return buf.toString();
+    }
+
+    /**
+     * Returns the index of the first query or fragment delimiter, or the length of the path if there is none.
+     */
+    private static int findPathEnd(final String path) {
+        for (int i = 0; i < path.length(); i++) {
+            final char ch = path.charAt(i);
+            if (ch == '?' || ch == '#') {
+                return i;
+            }
+        }
+        return path.length();
+    }
+
+    /**
+     * Returns the path prefix without decoding and re-encoding the path if the path is already
+     * in normalized form: it starts with a separator, has no empty or dot segments other than
+     * a trailing empty segment, and consists of unreserved characters only. Returns {@code null}
+     * otherwise.
+     */
+    private static String extractNormalizedPathPrefix(final String path, final int pathEnd) {
+        if (pathEnd == 0 || path.charAt(0) != '/') {
+            return null;
+        }
+        int lastSeparator = 0;
+        int segmentStart = 1;
+        for (int i = 1; i <= pathEnd; i++) {
+            final char ch = i < pathEnd ? path.charAt(i) : '/';
+            if (ch == '/') {
+                final int segmentLength = i - segmentStart;
+                if (segmentLength == 0 && i < pathEnd) {
+                    return null;
+                }
+                if (segmentLength == 1 && path.charAt(segmentStart) == '.') {
+                    return null;
+                }
+                if (segmentLength == 2 && path.charAt(segmentStart) == '.' && path.charAt(segmentStart + 1) == '.') {
+                    return null;
+                }
+                if (i < pathEnd) {
+                    lastSeparator = i;
+                }
+                segmentStart = i + 1;
+            } else if (!isUnreserved(ch)) {
+                return null;
             }
-            return buf.toString();
-        } catch (final URISyntaxException ex) {
-            return path;
         }
+        return path.substring(0, lastSeparator + 1);
+    }
+
+    private static boolean isUnreserved(final char ch) {
+        return ch >= 'a' && ch <= 'z' || ch >= 'A' && ch <= 'Z' || ch >= '0' && ch <= '9'
+                || ch == '-' || ch == '.' || ch == '_' || ch == '~';
     }
 
 }

httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestRequestSupport.java

@@ -26,7 +26,16 @@
  */
 package org.apache.hc.client5.http.impl;
 
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+import java.util.Random;
+
+import org.apache.hc.core5.http.HttpRequest;
 import org.apache.hc.core5.http.message.BasicHttpRequest;
+import org.apache.hc.core5.net.PercentCodec;
+import org.apache.hc.core5.net.URIBuilder;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
@@ -50,4 +59,104 @@ void testPathPrefixExtraction() {
         Assertions.assertEquals("/", RequestSupport.extractPathPrefix(new BasicHttpRequest("GET", "")));
     }
 
+    @Test
+    void testPathPrefixExtractionIgnoresQueryAndFragment() {
+        Assertions.assertEquals("/aaaa/", extractPathPrefix("/aaaa/bbbb?q=%E4%BD%A0%20x&r=a%2Fb&s=..%2F..&t"));
+        Assertions.assertEquals("/aaaa/", extractPathPrefix("/aaaa/bbbb?/../cccc/dddd/"));
+        Assertions.assertEquals("/aaaa/", extractPathPrefix("/aaaa/bbbb#cccc/dddd"));
+        Assertions.assertEquals("/aaaa/", extractPathPrefix("/aaaa/bbbb?x=1#cccc/dddd"));
+        Assertions.assertEquals("/aaaa/", extractPathPrefix("/aaaa/bbbb#cccc?x=1/2"));
+        Assertions.assertEquals("/aaaa/bbbb/", extractPathPrefix("/aaaa/bbbb/?x=1"));
+        Assertions.assertEquals("/aaaa/bbbb/", extractPathPrefix("/aaaa/bbbb/#x"));
+        Assertions.assertEquals("/", extractPathPrefix("/?x=1/2/3"));
+        Assertions.assertEquals("/", extractPathPrefix("?x=1/2/3"));
+        Assertions.assertEquals("/aa%2Faa/", extractPathPrefix("/aa%2faa/bbbb?x=%2F"));
+        Assertions.assertEquals("/aaaa/", extractPathPrefix("/aaaa/bb%2Fbb?x=1"));
+        Assertions.assertEquals("/aaaa/", extractPathPrefix("/aaaa/bbbb?q=a b"));
+        Assertions.assertEquals("/aa%3Baa/", extractPathPrefix("/aa;aa/bbbb?q=a b"));
+    }
+
+    @Test
+    void testPathPrefixExtractionNormalization() {
+        Assertions.assertEquals("/a-b.c_d~e/F0/", extractPathPrefix("/a-b.c_d~e/F0/g"));
+        Assertions.assertEquals("/...a/b../", extractPathPrefix("/...a/b../c"));
+        Assertions.assertEquals("/aaaa/bbbb/", extractPathPrefix("/aaaa/./bbbb/cccc"));
+        Assertions.assertEquals("/aaaa/bbbb/", extractPathPrefix("/aaaa//bbbb/cccc"));
+        Assertions.assertEquals("/bbbb/", extractPathPrefix("/aaaa/%2E%2E/bbbb/cccc"));
+        Assertions.assertEquals("/aaaa%3Bv%3D1/", extractPathPrefix("/aaaa;v=1/bbbb;w=2"));
+        Assertions.assertEquals("/aaaa%3Abbbb%40cccc/", extractPathPrefix("/aaaa:bbbb@cccc/dddd"));
+        Assertions.assertEquals("/aaaa%20bbbb/", extractPathPrefix("/aaaa%20bbbb/cccc"));
+        Assertions.assertEquals("/aaaa/", extractPathPrefix("http://host/aaaa/bbbb?x=1"));
+        Assertions.assertEquals("/", extractPathPrefix("*"));
+    }
+
+    /**
+     * Compares {@link RequestSupport#extractPathPrefix(HttpRequest)} with the implementation that
+     * parsed the request path including the query and the fragment.
+     */
+    @Test
+    void testPathPrefixExtractionMatchesFullUriParsing() {
+        final String[] tokens = {
+                "/", "/", "/", "//", "a", "Bc", "0", "-", "_", "~", ".", "..", "...",
+                "%2F", "%2f", "%61", "%2E", "%20", "%3B", "%C3%A9", "%", "%G1",
+                ";", "=", ":", "@", "!", "$", "+", ",", "*", "'", "(", " ", "|", "é", "?", "#", "&"
+        };
+        final Random random = new Random(42);
+        for (int n = 0; n < 20000; n++) {
+            final StringBuilder buf = new StringBuilder();
+            if (random.nextInt(10) > 0) {
+                buf.append('/');
+            }
+            final int count = random.nextInt(12);
+            for (int i = 0; i < count; i++) {
+                buf.append(tokens[random.nextInt(tokens.length)]);
+            }
+            final String path = buf.toString();
+            if (isValidUri(path) || !isValidUri(path.split("[?#]", 2)[0])) {
+                Assertions.assertEquals(extractPathPrefixFromUri(path), extractPathPrefix(path), path);
+            }
+        }
+    }
+
+    private static String extractPathPrefix(final String path) {
+        final BasicHttpRequest request = new BasicHttpRequest("GET", "/");
+        request.setPath(path);
+        return RequestSupport.extractPathPrefix(request);
+    }
+
+    private static boolean isValidUri(final String s) {
+        try {
+            new URI(s);
+            return true;
+        } catch (final URISyntaxException ex) {
+            return false;
+        }
+    }
+
+    private static String extractPathPrefixFromUri(final String path) {
+        try {
+            final URIBuilder uriBuilder = new URIBuilder(path);
+            uriBuilder.setFragment(null);
+            uriBuilder.clearParameters();
+            uriBuilder.optimize();
+            final List<String> pathSegments = uriBuilder.getPathSegments();
+
+            if (!pathSegments.isEmpty()) {
+                pathSegments.remove(pathSegments.size() - 1);
+            }
+            if (pathSegments.isEmpty()) {
+                return "/";
+            }
+            final StringBuilder buf = new StringBuilder();
+            buf.append('/');
+            for (final String pathSegment : pathSegments) {
+                PercentCodec.encode(buf, pathSegment, StandardCharsets.US_ASCII);
+                buf.append('/');
+            }
+            return buf.toString();
+        } catch (final URISyntaxException ex) {
+            return path;
+        }
+    }
+
 }