Fix NPE vulnerability and improve code centralization in URIBuilder (#235)
diff --git a/httpcore5/src/main/java/org/apache/hc/core5/net/URIBuilder.java b/httpcore5/src/main/java/org/apache/hc/core5/net/URIBuilder.java
index 25da44d..c54ca8e 100644
--- a/httpcore5/src/main/java/org/apache/hc/core5/net/URIBuilder.java
+++ b/httpcore5/src/main/java/org/apache/hc/core5/net/URIBuilder.java
@@ -517,11 +517,7 @@
* @return this.
*/
public URIBuilder setPathSegments(final String... pathSegments) {
- this.pathSegments = pathSegments.length > 0 ? Arrays.asList(pathSegments) : null;
- this.encodedSchemeSpecificPart = null;
- this.encodedPath = null;
- this.pathRootless = false;
- return this;
+ return setPathSegments(Arrays.asList(pathSegments));
}
/**
@@ -542,11 +538,7 @@
* @since 5.1
*/
public URIBuilder setPathSegmentsRootless(final String... pathSegments) {
- this.pathSegments = pathSegments.length > 0 ? Arrays.asList(pathSegments) : null;
- this.encodedSchemeSpecificPart = null;
- this.encodedPath = null;
- this.pathRootless = true;
- return this;
+ return setPathSegmentsRootless(Arrays.asList(pathSegments));
}
/**
@@ -568,7 +560,7 @@
* @return this.
*/
public URIBuilder appendPathSegments(final List<String> pathSegments) {
- if (pathSegments != null & pathSegments.size() > 0) {
+ if (pathSegments != null && pathSegments.size() > 0) {
final List<String> segments = new ArrayList<>(getPathSegments());
segments.addAll(pathSegments);
setPathSegments(segments);
diff --git a/httpcore5/src/test/java/org/apache/hc/core5/net/TestURIBuilder.java b/httpcore5/src/test/java/org/apache/hc/core5/net/TestURIBuilder.java
index cdea710..4d8a857 100644
--- a/httpcore5/src/test/java/org/apache/hc/core5/net/TestURIBuilder.java
+++ b/httpcore5/src/test/java/org/apache/hc/core5/net/TestURIBuilder.java
@@ -545,14 +545,53 @@
}
@Test
+ public void testSetPathSegmentList() throws Exception {
+ final URI uri = new URIBuilder()
+ .setScheme("https")
+ .setHost("somehost")
+ .setPathSegments(Arrays.asList("api", "products"))
+ .build();
+ MatcherAssert.assertThat(uri, CoreMatchers.equalTo(URI.create("https://somehost/api/products")));
+ }
+
+ @Test
+ public void testSetPathSegmentsVarargs() throws Exception {
+ final URI uri = new URIBuilder()
+ .setScheme("https")
+ .setHost("somehost")
+ .setPathSegments("api", "products")
+ .build();
+ MatcherAssert.assertThat(uri, CoreMatchers.equalTo(URI.create("https://somehost/api/products")));
+ }
+
+ @Test
+ public void testSetPathSegmentsRootlessList() throws Exception {
+ final URI uri = new URIBuilder()
+ .setScheme("file")
+ .setPathSegmentsRootless(Arrays.asList("dir", "foo"))
+ .build();
+ MatcherAssert.assertThat(uri, CoreMatchers.equalTo(URI.create("file:dir/foo")));
+ }
+
+ @Test
+ public void testSetPathSegmentsRootlessVarargs() throws Exception {
+ final URI uri = new URIBuilder()
+ .setScheme("file")
+ .setPathSegmentsRootless("dir", "foo")
+ .build();
+ MatcherAssert.assertThat(uri, CoreMatchers.equalTo(URI.create("file:dir/foo")));
+ }
+
+ @Test
public void testAppendToExistingPath() throws Exception {
final URI uri = new URIBuilder()
.setScheme("https")
.setHost("somehost")
.setPath("api")
.appendPath("v1/resources")
+ .appendPath("idA")
.build();
- MatcherAssert.assertThat(uri, CoreMatchers.equalTo(URI.create("https://somehost/api/v1/resources")));
+ MatcherAssert.assertThat(uri, CoreMatchers.equalTo(URI.create("https://somehost/api/v1/resources/idA")));
}
@Test
@@ -561,8 +600,9 @@
.setScheme("https")
.setHost("somehost")
.appendPath("api/v2/customers")
+ .appendPath("idA")
.build();
- MatcherAssert.assertThat(uri, CoreMatchers.equalTo(URI.create("https://somehost/api/v2/customers")));
+ MatcherAssert.assertThat(uri, CoreMatchers.equalTo(URI.create("https://somehost/api/v2/customers/idA")));
}
@Test
@@ -587,24 +627,37 @@
}
@Test
- public void testAppendSegmentsToExistingPath() throws Exception {
+ public void testAppendSegmentsVarargsToExistingPath() throws Exception {
final URI uri = new URIBuilder()
.setScheme("https")
.setHost("myhost")
.setPath("api")
.appendPathSegments("v3", "products")
+ .appendPathSegments("idA")
.build();
- MatcherAssert.assertThat(uri, CoreMatchers.equalTo(URI.create("https://myhost/api/v3/products")));
+ MatcherAssert.assertThat(uri, CoreMatchers.equalTo(URI.create("https://myhost/api/v3/products/idA")));
}
@Test
- public void testAppendSegmentsToNonExistingPath() throws Exception {
+ public void testAppendSegmentsVarargsToNonExistingPath() throws Exception {
final URI uri = new URIBuilder()
.setScheme("https")
.setHost("somehost")
.appendPathSegments("api", "v2", "customers")
+ .appendPathSegments("idA")
.build();
- MatcherAssert.assertThat(uri, CoreMatchers.equalTo(URI.create("https://somehost/api/v2/customers")));
+ MatcherAssert.assertThat(uri, CoreMatchers.equalTo(URI.create("https://somehost/api/v2/customers/idA")));
+ }
+
+ @Test
+ public void testAppendNullSegmentsVarargs() throws Exception {
+ final String pathSegment = null;
+ final URI uri = new URIBuilder()
+ .setScheme("https")
+ .setHost("somehost")
+ .appendPathSegments(pathSegment)
+ .build();
+ MatcherAssert.assertThat(uri, CoreMatchers.equalTo(URI.create("https://somehost/")));
}
@Test
@@ -629,6 +682,17 @@
}
@Test
+ public void testAppendNullSegmentsList() throws Exception {
+ final List<String> pathSegments = null;
+ final URI uri = new URIBuilder()
+ .setScheme("http")
+ .setHost("myhost")
+ .appendPathSegments(pathSegments)
+ .build();
+ MatcherAssert.assertThat(uri, CoreMatchers.equalTo(URI.create("http://myhost")));
+ }
+
+ @Test
public void testNoAuthorityAndPathSegments() throws Exception {
final URI uri = new URIBuilder()
.setScheme("file")
