From 918ac1535f37c47a2230f14b6404d411ee7df91e Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Sat, 19 Sep 2020 12:56:23 +0200 Subject: [PATCH] RFC 3986 conformance: corrected handling of path segments by `URIUtils#normalizeSyntax`; optimized path segment operations --- .../http/impl/cache/HttpCacheSupport.java | 4 +- .../impl/cache/TestCacheKeyGenerator.java | 6 +-- .../http/impl/DefaultRedirectStrategy.java | 6 +-- .../hc/client5/http/utils/URIUtils.java | 40 +++++++++---------- .../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 664434cc9..12330625a 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 ef409c227..c6c9ba0f3 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 ed82bfaae..889976671 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 0f8814f57..3fe9cf06d 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 inputSegments = builder.getPathSegments(); final Stack 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 (!inputSegments.isEmpty()) { + final String lastSegment = inputSegments.get(inputSegments.size() - 1); + if (lastSegment.isEmpty()) { + outputSegments.push(""); + } } - if (path.lastIndexOf('/') == path.length() - 1) { - // path.endsWith("/") || path.equals("") - outputBuffer.append('/'); - } - 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 11632a486..d70cfb66d 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/",