This is an automated email from the ASF dual-hosted git repository. reta pushed a commit to branch 4.1.x-fixes in repository https://gitbox.apache.org/repos/asf/cxf.git
commit 5bcf4e65f117a49c5180027ab67b00801f0660c0 Author: Andriy Redko <[email protected]> AuthorDate: Fri Oct 23 07:38:22 2026 -0400 CXF-7986: WebClient.path method overwrites base URL when given absolute URL as input (#3596) * CXF-7986: WebClient.path method overwrites base URL when given absolute URL as input * Address code review comments * Address code review comments * Address code review comments * Add tests for UriBuilder::toTemplate() * Address review comments * Address code review comments * Address review comments * Add more test cases (cherry picked from commit 89758caeb0823fa1c47b6801f61a3971deac040c) --- .../org/apache/cxf/jaxrs/impl/UriBuilderImpl.java | 51 +++- .../apache/cxf/jaxrs/impl/UriBuilderImplTest.java | 305 +++++++++++++++++++++ .../org/apache/cxf/jaxrs/client/WebClientTest.java | 25 ++ 3 files changed, 371 insertions(+), 10 deletions(-) diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/UriBuilderImpl.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/UriBuilderImpl.java index c1131863774..7e50ff59cd9 100644 --- a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/UriBuilderImpl.java +++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/UriBuilderImpl.java @@ -103,7 +103,7 @@ public class UriBuilderImpl extends UriBuilder implements Cloneable { @Override public URI build(Object... values) throws IllegalArgumentException, UriBuilderException { - return doBuild(false, true, values); + return build(values, true); } private static Map<String, Object> getResolvedTemplates(Map<String, Object> rtemplates) { @@ -185,13 +185,16 @@ public class UriBuilderImpl extends UriBuilder implements Cloneable { private URI buildURI(boolean fromEncoded, String thePath, String theQuery, String theFragment) throws URISyntaxException { + + // Always include the leading slash in order to prevent host overrides + if ((scheme != null || host != null || userInfo != null) + && thePath.length() != 0 && (!thePath.startsWith("/") && needsLeadingSlash(thePath))) { + thePath = "/" + thePath; + } + if (fromEncoded) { return buildURIFromEncoded(thePath, theQuery, theFragment); } else if (!isSchemeOpaque()) { - if ((scheme != null || host != null || userInfo != null) - && thePath.length() != 0 && !(thePath.startsWith("/") || thePath.startsWith(";"))) { - thePath = "/" + thePath; - } try { return buildURIFromEncoded(thePath, theQuery, theFragment); } catch (Exception ex) { @@ -235,8 +238,8 @@ public class UriBuilderImpl extends UriBuilder implements Cloneable { b.append(':').append(port); } if (thePath != null && thePath.length() > 0) { - b.append(thePath.startsWith("/") || b.length() == 0 || originalPathEmpty - ? thePath : '/' + thePath); + b.append(thePath.startsWith("/") || b.length() == 0 + || (originalPathEmpty && !needsLeadingSlash(thePath)) ? thePath : '/' + thePath); } if (theQuery != null && theQuery.length() != 0) { b.append('?').append(theQuery); @@ -250,6 +253,30 @@ public class UriBuilderImpl extends UriBuilder implements Cloneable { return b.toString(); } + /** + * The `;`, `&` and `+` are valid userinfo characters (RFC 3986), so a path + * glued to the host that contains `@` is parsed as userinfo. + * + * URI.create("http://foo.com;@evil.com";).getHost() -> "evil.com" + * URI.create("http://foo.com&@evil.com";).getHost() -> "evil.com" + * URI.create("http://[email protected]";).getHost() -> "evil.com" + * + * The straightforward encoding of @ -> %40 does not work either: + * + * URI.create("http://foo.com;%40evil.com";).getHost() -> null + * + * It does not even matter if the userinfo part is already provided: + * + * URI.create("http://a:[email protected];%40evil.com";).getHost() -> null + * + * To workaround the issue, we have to include leading path slash all the time. + */ + private boolean needsLeadingSlash(String path) { + return !path.startsWith(";") + || path.indexOf('@') >= 0 /* check if @ is present */ + || path.indexOf("%40") >= 0; /* @ encoded */ + } + private boolean isSchemeOpaque() { return schemeSpecificPart != null; } @@ -262,7 +289,7 @@ public class UriBuilderImpl extends UriBuilder implements Cloneable { @Override public URI buildFromMap(Map<String, ?> map) throws IllegalArgumentException, UriBuilderException { - return doBuildFromMap(map, false, true); + return buildFromMap(map, true); } private URI doBuildFromMap(Map<String, ? extends Object> map, boolean fromEncoded, @@ -548,7 +575,9 @@ public class UriBuilderImpl extends UriBuilder implements Cloneable { if (path == null) { throw new IllegalArgumentException("path is null"); } - if (isAbsoluteUriPath(path)) { + // Only set uri when path is absolute and the current URI's is not set + // (scheme and host are all null). + if (isAbsoluteUriPath(path) && scheme == null && host == null) { try { URI uri = URI.create(path); this.originalPathEmpty = StringUtils.isEmpty(uri.getPath()); @@ -778,7 +807,9 @@ public class UriBuilderImpl extends UriBuilder implements Cloneable { public UriBuilder replacePath(String path) { if (path == null) { clearPathAndMatrix(); - } else if (isAbsoluteUriPath(path)) { + } else if (isAbsoluteUriPath(path) && scheme == null && host == null) { + // Only set uri when path is absolute and the current URI's is not set + // (scheme and host are all null). clearPathAndMatrix(); uri(URI.create(path)); } else { diff --git a/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/impl/UriBuilderImplTest.java b/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/impl/UriBuilderImplTest.java index a5778ee9293..da70d70a2ca 100644 --- a/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/impl/UriBuilderImplTest.java +++ b/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/impl/UriBuilderImplTest.java @@ -40,6 +40,9 @@ import org.apache.cxf.jaxrs.utils.JAXRSUtils; import org.junit.Test; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.startsWith; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; import static org.junit.Assert.fail; @@ -134,7 +137,309 @@ public class UriBuilderImplTest { assertEquals("/?a=%25&a2=%7B%7D", uri.toString()); } + @Test + public void testReplaceFullPath() { + UriBuilder ub = UriBuilder.fromPath(""); + URI uri = ub.build(); + assertEquals("", uri.toString()); + uri = ub.replacePath("http://foo.com";).build(); + assertEquals("http://foo.com";, uri.toString()); + + uri = ub.buildFromEncoded(); + assertEquals("http://foo.com";, uri.toString()); + } + + @Test + public void testReplaceFullPathWhenUriIsSet() { + UriBuilder ub = UriBuilder.fromPath("http://foo.com";); + URI uri = ub.build(); + assertEquals("http://foo.com";, uri.toString()); + uri = ub.replacePath("http://bar.com/";).build(); + assertThat(uri.toString(), startsWith("http://foo.com";)); + assertThat(uri.getHost(), equalTo("foo.com")); + + uri = ub.buildFromEncoded(); + assertThat(uri.toString(), startsWith("http://foo.com";)); + assertThat(uri.getHost(), equalTo("foo.com")); + } + + @Test + public void testBuildAndSetFullPath() { + UriBuilder ub = UriBuilder.fromPath(""); + URI uri = ub.build(); + assertEquals("", uri.toString()); + uri = ub.path("http://foo.com";).build(); + assertEquals("http://foo.com";, uri.toString()); + + uri = ub.buildFromEncoded(); + assertThat(uri.toString(), startsWith("http://foo.com";)); + assertThat(uri.getHost(), equalTo("foo.com")); + } + + @Test + public void testBuildAndSetFullPathWhenUriIsSet() { + UriBuilder ub = UriBuilder.fromPath("http://foo.com";); + URI uri = ub.build(); + assertEquals("http://foo.com";, uri.toString()); + uri = ub.path("http://bar.com/";).build(); + assertThat(uri.toString(), startsWith("http://foo.com";)); + assertThat(uri.getHost(), equalTo("foo.com")); + + uri = ub.buildFromEncoded(); + assertThat(uri.toString(), startsWith("http://foo.com";)); + assertThat(uri.getHost(), equalTo("foo.com")); + } + + @Test + public void testPathDoesNotChangeHost() { + UriBuilder ub = UriBuilder.fromPath("http://foo.com";); + URI uri = ub.build(); + assertEquals("http://foo.com";, uri.toString()); + + uri = ub.path(".org").build(); + assertThat(uri.toString(), equalTo("http://foo.com/.org";)); + assertThat(uri.getHost(), equalTo("foo.com")); + + uri = ub.buildFromEncoded(); + assertThat(uri.toString(), equalTo("http://foo.com/.org";)); + assertThat(uri.getHost(), equalTo("foo.com")); + } + + @Test + public void testReplacePathDoesNotChangeHost() { + UriBuilder ub = UriBuilder.fromPath("http://foo.com";); + URI uri = ub.build(); + assertEquals("http://foo.com";, uri.toString()); + uri = ub.replacePath(".org").build(); + assertThat(uri.toString(), equalTo("http://foo.com/.org";)); + assertThat(uri.getHost(), equalTo("foo.com")); + + uri = ub.buildFromEncoded(); + assertThat(uri.toString(), equalTo("http://foo.com/.org";)); + assertThat(uri.getHost(), equalTo("foo.com")); + } + + @Test + public void testEncodedUserInfoAndUriWithMatrix() { + // The UriBuilder::fromUri will treat `evil.com` as the host name + UriBuilder ub = UriBuilder.fromUri("http://foo.com;%40evil.com";); + URI uri = ub.build(); + assertEquals("http://foo.com;@evil.com";, uri.toString()); + assertThat(uri.getHost(), equalTo("evil.com")); + } + + @Test + public void testUserInfoAndUriWithMatrix() { + // The UriBuilder::fromUri will treat `evil.com` as the host name + UriBuilder ub = UriBuilder.fromUri("http://foo.com;@evil.com";); + URI uri = ub.build(); + assertEquals("http://foo.com;@evil.com";, uri.toString()); + assertThat(uri.getHost(), equalTo("evil.com")); + } + + @Test + public void testEncodedUserInfoSeparatorAndPathWithMatrix() { + // The UriBuilder::fromPath will treat `evil.com` as the host name + UriBuilder ub = UriBuilder.fromPath("http://foo.com;%40evil.com";); + URI uri = ub.build(); + assertEquals("http://foo.com;@evil.com";, uri.toString()); + assertThat(uri.getHost(), equalTo("evil.com")); + } + + @Test + public void testUserInfoSeparatorAndPathWithMatrix() { + // The UriBuilder::fromPath will treat `evil.com` as the host name + UriBuilder ub = UriBuilder.fromPath("http://foo.com;@evil.com";); + URI uri = ub.build(); + assertEquals("http://foo.com;@evil.com";, uri.toString()); + assertThat(uri.getHost(), equalTo("evil.com")); + } + + @Test + public void testPathWithUserInfoDoesNotChangeHost() { + UriBuilder ub = UriBuilder.fromPath("http://a:[email protected]";); + URI uri = ub.build(); + assertEquals("http://a:[email protected]";, uri.toString()); + uri = ub.path(";@evil.com").build(); + assertThat(uri.toString(), equalTo("http://a:[email protected]/;@evil.com";)); + assertThat(uri.getHost(), equalTo("foo.com")); + + uri = ub.buildFromEncoded(); + assertThat(uri.toString(), equalTo("http://a:[email protected]/;@evil.com";)); + assertThat(uri.getHost(), equalTo("foo.com")); + + String template = ub.toTemplate(); + assertThat(template, equalTo("http://a:[email protected]/;@evil.com";)); + } + + @Test + public void testUriWithUserInfoDoesNotChangeHost() { + UriBuilder ub = UriBuilder.fromUri("http://a:[email protected]";); + URI uri = ub.build(); + assertEquals("http://a:[email protected]";, uri.toString()); + uri = ub.path(";@evil.com").build(); + assertThat(uri.toString(), equalTo("http://a:[email protected]/;@evil.com";)); + assertThat(uri.getHost(), equalTo("foo.com")); + + uri = ub.buildFromEncoded(); + assertThat(uri.toString(), equalTo("http://a:[email protected]/;@evil.com";)); + assertThat(uri.getHost(), equalTo("foo.com")); + + String template = ub.toTemplate(); + assertThat(template, equalTo("http://a:[email protected]/;@evil.com";)); + } + + @Test + public void testEncodedUserInfoSeparatorAndPathDoesNotChangeHost() { + UriBuilder ub = UriBuilder.fromPath("http://foo.com";); + URI uri = ub.build(); + assertEquals("http://foo.com";, uri.toString()); + uri = ub.path(";%40evil.com").build(); + assertThat(uri.toString(), equalTo("http://foo.com/;%40evil.com";)); + assertThat(uri.getHost(), equalTo("foo.com")); + + uri = ub.buildFromEncoded(); + assertThat(uri.toString(), equalTo("http://foo.com/;%40evil.com";)); + assertThat(uri.getHost(), equalTo("foo.com")); + + String template = ub.toTemplate(); + assertThat(template, equalTo("http://foo.com/;%40evil.com";)); + } + + @Test + public void testUserInfoSeparatorAndPathDoesNotChangeHost() { + UriBuilder ub = UriBuilder.fromPath("http://foo.com";); + URI uri = ub.build(); + assertEquals("http://foo.com";, uri.toString()); + uri = ub.path(";@evil.com").build(); + assertThat(uri.toString(), equalTo("http://foo.com/;@evil.com";)); + assertThat(uri.getHost(), equalTo("foo.com")); + + uri = ub.buildFromEncoded(); + assertThat(uri.toString(), equalTo("http://foo.com/;@evil.com";)); + assertThat(uri.getHost(), equalTo("foo.com")); + + String template = ub.toTemplate(); + assertThat(template, equalTo("http://foo.com/;@evil.com";)); + } + + @Test + public void testUserInfoSeparatorAndReplacePathDoesNotChangeHost() { + UriBuilder ub = UriBuilder.fromPath("http://foo.com";); + URI uri = ub.build(); + assertEquals("http://foo.com";, uri.toString()); + uri = ub.replacePath(";@evil.com").build(); + assertThat(uri.toString(), equalTo("http://foo.com/;@evil.com";)); + assertThat(uri.getHost(), equalTo("foo.com")); + + uri = ub.buildFromEncoded(); + assertThat(uri.toString(), equalTo("http://foo.com/;@evil.com";)); + assertThat(uri.getHost(), equalTo("foo.com")); + String template = ub.toTemplate(); + assertThat(template, equalTo("http://foo.com/;@evil.com";)); + } + + @Test + public void testUserInfoSeparatorAndReplaceMatrixDoesNotChangeHost() { + UriBuilder ub = UriBuilder.fromPath("http://foo.com";); + URI uri = ub.build(); + assertEquals("http://foo.com";, uri.toString()); + uri = ub.replaceMatrix("@evil.com").build(); + assertThat(uri.toString(), equalTo("http://foo.com/;@evil.com";)); + assertThat(uri.getHost(), equalTo("foo.com")); + + uri = ub.buildFromEncoded(); + assertThat(uri.toString(), equalTo("http://foo.com/;@evil.com";)); + assertThat(uri.getHost(), equalTo("foo.com")); + + String template = ub.toTemplate(); + assertThat(template, equalTo("http://foo.com/;@evil.com";)); + } + + @Test + public void testUserInfoSeparatorAndMatrixDoesNotChangeHost() { + UriBuilder ub = UriBuilder.fromPath("http://foo.com";); + URI uri = ub.build(); + assertEquals("http://foo.com";, uri.toString()); + uri = ub.matrixParam("@evil.com").build(); + assertThat(uri.toString(), equalTo("http://foo.com/;@evil.com";)); + assertThat(uri.getHost(), equalTo("foo.com")); + + uri = ub.buildFromEncoded(); + assertThat(uri.toString(), equalTo("http://foo.com/;@evil.com";)); + assertThat(uri.getHost(), equalTo("foo.com")); + + String template = ub.toTemplate(); + assertThat(template, equalTo("http://foo.com/;@evil.com";)); + } + + @Test + public void testUserInfoSeparatorAndMatrixParamDoesNotChangeHost() { + UriBuilder ub = UriBuilder.fromPath("http://foo.com";); + URI uri = ub.build(); + assertEquals("http://foo.com";, uri.toString()); + uri = ub.matrixParam("param", "@evil.com").build(); + assertThat(uri.toString(), equalTo("http://foo.com/;[email protected]";)); + assertThat(uri.getHost(), equalTo("foo.com")); + + uri = ub.buildFromEncoded(); + assertThat(uri.toString(), equalTo("http://foo.com/;[email protected]";)); + assertThat(uri.getHost(), equalTo("foo.com")); + + String template = ub.toTemplate(); + assertThat(template, equalTo("http://foo.com/;[email protected]";)); + } + + @Test + public void testUserInfoSeparatorAndTemplatedPathDoesNotChangeHost() { + UriBuilder ub = UriBuilder.fromPath("http://foo.com";); + URI uri = ub.build(); + assertEquals("http://foo.com";, uri.toString()); + uri = ub.path("{x}").build(";@evil.com"); + assertThat(uri.toString(), equalTo("http://foo.com/;@evil.com";)); + assertThat(uri.getHost(), equalTo("foo.com")); + + uri = ub.buildFromEncoded(";@evil.com"); + assertThat(uri.toString(), equalTo("http://foo.com/;@evil.com";)); + assertThat(uri.getHost(), equalTo("foo.com")); + } + + @Test + public void testTemplateUriDoesNotChangeHost() { + final String template = UriBuilder + .fromUri("http://foo.com";) + .path(".org") + .toTemplate(); + assertEquals("http://foo.com/.org";, template); + } + + @Test + public void testTemplatePathDoesNotChangeHost() { + final String template = UriBuilder + .fromPath("http://foo.com";) + .path(".org") + .toTemplate(); + assertEquals("http://foo.com/.org";, template); + } + + @Test + public void testTemplateWithPathDoesNotChangeHost() { + final String template = UriBuilder + .fromPath("http://foo.com/aaa";) + .path(".org") + .toTemplate(); + assertEquals("http://foo.com/aaa/.org";, template); + } + + @Test + public void testTemplatePathDoesNotReplaceUri() { + final String template = UriBuilder + .fromPath("http://foo.com/";) + .path("http://bar.com";) + .toTemplate(); + assertThat(template, startsWith("http://foo.com/";)); + } @Test public void testResolveTemplate() { diff --git a/rt/rs/client/src/test/java/org/apache/cxf/jaxrs/client/WebClientTest.java b/rt/rs/client/src/test/java/org/apache/cxf/jaxrs/client/WebClientTest.java index e6566de1f51..58db96c53f1 100644 --- a/rt/rs/client/src/test/java/org/apache/cxf/jaxrs/client/WebClientTest.java +++ b/rt/rs/client/src/test/java/org/apache/cxf/jaxrs/client/WebClientTest.java @@ -48,6 +48,7 @@ import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.CoreMatchers.nullValue; +import static org.hamcrest.CoreMatchers.startsWith; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.collection.IsIterableContainingInAnyOrder.containsInAnyOrder; import static org.junit.Assert.assertEquals; @@ -535,6 +536,30 @@ public class WebClientTest { assertThat(configuration.getBus().getState(), equalTo(BusState.SHUTDOWN)); } + @Test + public void pathAllowsAddingURLAsPathElement() { + final WebClient webClient = WebClient.create("http://www.foo.com";).path("http://www.bar.com";); + assertThat(webClient.getCurrentURI().toString(), startsWith("http://www.foo.com/";)); + assertThat(webClient.getCurrentURI().getHost(), equalTo("www.foo.com")); + } + + @Test + public void preservesPreviouslyAddedPathParameters() { + final WebClient webClient = WebClient.create("http://www.foo.com";) + .path("foo") + .path("bar") + .path("http://www.bar.com";); + assertThat(webClient.getCurrentURI().toString(), startsWith("http://www.foo.com/foo/bar/";)); + assertThat(webClient.getCurrentURI().getHost(), equalTo("www.foo.com")); + } + + @Test + public void outputOfUriUtilsEncodePathSegmentIsSafe() { + final WebClient webClient = WebClient.create("http://www.foo.com";).path("http:%2F%2Fwww.bar.com"); + assertThat(webClient.getCurrentURI().toString(), startsWith("http://www.foo.com/";)); + assertThat(webClient.getCurrentURI().getHost(), equalTo("www.foo.com")); + } + private static final class ParamConverterProviderImpl implements ParamConverterProvider { @SuppressWarnings("unchecked")
