You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2020/09/19 10:59:26 UTC

[httpcomponents-client] branch 5.1.x updated: RFC 3986 conformance: corrected handling of path segments by `URIUtils#normalizeSyntax`; optimized path segment operations

This is an automated email from the ASF dual-hosted git repository.

olegk pushed a commit to branch 5.1.x
in repository https://gitbox.apache.org/repos/asf/httpcomponents-client.git


The following commit(s) were added to refs/heads/5.1.x by this push:
     new e30519f  RFC 3986 conformance: corrected handling of path segments by `URIUtils#normalizeSyntax`; optimized path segment operations
e30519f is described below

commit e30519f9431935d4507d73391bc03f5140eeff93
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Sat Sep 19 12:56:23 2020 +0200

    RFC 3986 conformance: corrected handling of path segments by `URIUtils#normalizeSyntax`; optimized path segment operations
---
 .../client5/http/impl/cache/HttpCacheSupport.java  |  4 +--
 .../http/impl/cache/TestCacheKeyGenerator.java     |  6 ++--
 .../client5/http/impl/DefaultRedirectStrategy.java |  6 ++--
 .../org/apache/hc/client5/http/utils/URIUtils.java | 40 +++++++++++-----------
 .../apache/hc/client5/http/utils/TestURIUtils.java |  2 +-
 5 files changed, 28 insertions(+), 30 deletions(-)

diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/HttpCacheSupport.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/HttpCacheSupport.java
index 664434c..1233062 100644
--- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/HttpCacheSupport.java
+++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/HttpCacheSupport.java
@@ -115,8 +115,8 @@ public final class HttpCacheSupport {
             }
         }
         builder.setFragment(null);
-        if (builder.getPath() == null) {
-            builder.setPath("/");
+        if (builder.isPathEmpty()) {
+            builder.setPathSegments("");
         }
         return builder.build();
     }
diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheKeyGenerator.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheKeyGenerator.java
index ef409c2..c6c9ba0 100644
--- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheKeyGenerator.java
+++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCacheKeyGenerator.java
@@ -382,7 +382,7 @@ public class TestCacheKeyGenerator {
     public void testIntermidateEncodedDotDotSegementsAreEquivalent() {
         final HttpHost host = new HttpHost("foo.example.com");
         final HttpRequest req1 = new BasicHttpRequest("GET", "/home.html");
-        final HttpRequest req2 = new BasicHttpRequest("GET", "/%7Esmith%2F../home.html");
+        final HttpRequest req2 = new BasicHttpRequest("GET", "/%7Esmith/../home.html");
         Assert.assertEquals(extractor.generateKey(host, req1), extractor.generateKey(host, req2));
     }
 
@@ -406,7 +406,7 @@ public class TestCacheKeyGenerator {
     public void testEquivalentExtraPathEncodingsAreEquivalent() {
         final HttpHost host = new HttpHost("foo.example.com");
         final HttpRequest req1 = new BasicHttpRequest("GET", "/~smith/home.html");
-        final HttpRequest req2 = new BasicHttpRequest("GET", "/%7Esmith%2Fhome.html");
+        final HttpRequest req2 = new BasicHttpRequest("GET", "/%7Esmith/home.html");
         Assert.assertEquals(extractor.generateKey(host, req1), extractor.generateKey(host, req2));
     }
 
@@ -414,7 +414,7 @@ public class TestCacheKeyGenerator {
     public void testEquivalentExtraPathEncodingsWithPercentAreEquivalent() {
         final HttpHost host = new HttpHost("foo.example.com");
         final HttpRequest req1 = new BasicHttpRequest("GET", "/~smith/home%20folder.html");
-        final HttpRequest req2 = new BasicHttpRequest("GET", "/%7Esmith%2Fhome%20folder.html");
+        final HttpRequest req2 = new BasicHttpRequest("GET", "/%7Esmith/home%20folder.html");
         Assert.assertEquals(extractor.generateKey(host, req1), extractor.generateKey(host, req2));
     }
 }
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultRedirectStrategy.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultRedirectStrategy.java
index ed82bfa..8899766 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultRedirectStrategy.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultRedirectStrategy.java
@@ -45,7 +45,6 @@ import org.apache.hc.core5.http.ProtocolException;
 import org.apache.hc.core5.http.protocol.HttpContext;
 import org.apache.hc.core5.net.URIBuilder;
 import org.apache.hc.core5.util.Args;
-import org.apache.hc.core5.util.TextUtils;
 
 /**
  * Default implementation of {@link RedirectStrategy}.
@@ -119,9 +118,8 @@ public class DefaultRedirectStrategy implements RedirectStrategy {
             if (host != null) {
                 b.setHost(host.toLowerCase(Locale.ROOT));
             }
-            final String path = b.getPath();
-            if (TextUtils.isEmpty(path)) {
-                b.setPath("/");
+            if (b.isPathEmpty()) {
+                b.setPathSegments("");
             }
             return b.build();
         } catch (final URISyntaxException ex) {
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/utils/URIUtils.java b/httpclient5/src/main/java/org/apache/hc/client5/http/utils/URIUtils.java
index 0f8814f..3fe9cf0 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/utils/URIUtils.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/utils/URIUtils.java
@@ -133,8 +133,8 @@ public class URIUtils {
         if (uribuilder.getUserInfo() != null) {
             uribuilder.setUserInfo(null);
         }
-        if (TextUtils.isEmpty(uribuilder.getPath())) {
-            uribuilder.setPath("/");
+        if (uribuilder.isPathEmpty()) {
+            uribuilder.setPathSegments("");
         }
         if (uribuilder.getHost() != null) {
             uribuilder.setHost(uribuilder.getHost().toLowerCase(Locale.ROOT));
@@ -203,30 +203,30 @@ public class URIUtils {
         }
         Args.check(uri.isAbsolute(), "Base URI must be absolute");
         final URIBuilder builder = new URIBuilder(uri);
-        final String path = builder.getPath();
-        if (path != null && !path.equals("/")) {
-            final String[] inputSegments = path.split("/");
+        if (!builder.isPathEmpty()) {
+            final List<String> inputSegments = builder.getPathSegments();
             final Stack<String> outputSegments = new Stack<>();
             for (final String inputSegment : inputSegments) {
-                if ((inputSegment.isEmpty()) || (".".equals(inputSegment))) {
-                    // Do nothing
-                } else if ("..".equals(inputSegment)) {
-                    if (!outputSegments.isEmpty()) {
-                        outputSegments.pop();
+                if (!inputSegment.isEmpty() && !".".equals(inputSegment)) {
+                    if ("..".equals(inputSegment)) {
+                        if (!outputSegments.isEmpty()) {
+                            outputSegments.pop();
+                        }
+                    } else {
+                        outputSegments.push(inputSegment);
                     }
-                } else {
-                    outputSegments.push(inputSegment);
                 }
             }
-            final StringBuilder outputBuffer = new StringBuilder();
-            for (final String outputSegment : outputSegments) {
-                outputBuffer.append('/').append(outputSegment);
-            }
-            if (path.lastIndexOf('/') == path.length() - 1) {
-                // path.endsWith("/") || path.equals("")
-                outputBuffer.append('/');
+            if (!inputSegments.isEmpty()) {
+                final String lastSegment = inputSegments.get(inputSegments.size() - 1);
+                if (lastSegment.isEmpty()) {
+                    outputSegments.push("");
+                }
             }
-            builder.setPath(outputBuffer.toString());
+            builder.setPathSegments(outputSegments);
+        }
+        if (builder.isPathEmpty()) {
+            builder.setPathSegments("");
         }
         if (builder.getScheme() != null) {
             builder.setScheme(builder.getScheme().toLowerCase(Locale.ROOT));
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/utils/TestURIUtils.java b/httpclient5/src/test/java/org/apache/hc/client5/http/utils/TestURIUtils.java
index 11632a4..d70cfb6 100644
--- a/httpclient5/src/test/java/org/apache/hc/client5/http/utils/TestURIUtils.java
+++ b/httpclient5/src/test/java/org/apache/hc/client5/http/utils/TestURIUtils.java
@@ -108,7 +108,7 @@ public class TestURIUtils {
         Assert.assertEquals("example://a/b/c/%7Bfoo%7D", URIUtils.resolve(this.baseURI, "eXAMPLE://a/./b/../b/%63/%7bfoo%7d").toString());
         Assert.assertEquals("http://www.example.com/%3C", URIUtils.resolve(this.baseURI, "http://www.example.com/%3c").toString());
         Assert.assertEquals("http://www.example.com/", URIUtils.resolve(this.baseURI, "HTTP://www.EXAMPLE.com/").toString());
-        Assert.assertEquals("http://www.example.com/a/", URIUtils.resolve(this.baseURI, "http://www.example.com/a%2f").toString());
+        Assert.assertEquals("http://www.example.com/a%2F", URIUtils.resolve(this.baseURI, "http://www.example.com/a%2f").toString());
         Assert.assertEquals("http://www.example.com/?q=%26", URIUtils.resolve(this.baseURI, "http://www.example.com/?q=%26").toString());
         Assert.assertEquals("http://www.example.com/%23?q=%26", URIUtils.resolve(this.baseURI, "http://www.example.com/%23?q=%26").toString());
         Assert.assertEquals("http://www.example.com/blah-%28%20-blah-%20%26%20-blah-%20%29-blah/",