From 6e5cb7fbcddb08b794d2163022bf4a9534e9ea13 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Thu, 14 Feb 2013 21:09:28 -0800 Subject: [PATCH] Protect against '//' in UriComponentsBuilder Refactor UriComponentsBuilder to ensure that paths do not contain empty segments. For example, prior to this commit: fromUriString("http://example.com/abc/").path("/x/y/z") would build the URL "http://example.com/abc//x/y/z" where as it will now build "http://example.com/abc/x/y/z". Issue: SPR-10270 --- .../web/util/UriComponentsBuilder.java | 213 ++++++++---------- .../web/util/UriComponentsBuilderTests.java | 8 + 2 files changed, 108 insertions(+), 113 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java b/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java index 4a1aa85af9..30dcac0f84 100644 --- a/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java +++ b/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java @@ -18,7 +18,7 @@ package org.springframework.web.util; import java.net.URI; import java.util.ArrayList; -import java.util.Collection; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.regex.Matcher; @@ -29,6 +29,7 @@ import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; +import org.springframework.web.util.HierarchicalUriComponents.PathComponent; /** * Builder for {@link UriComponents}. @@ -46,6 +47,7 @@ import org.springframework.util.StringUtils; * * @author Arjen Poutsma * @author Rossen Stoyanchev + * @author Phillip Webb * @since 3.1 * @see #newInstance() * @see #fromPath(String) @@ -91,7 +93,7 @@ public class UriComponentsBuilder { private int port = -1; - private PathComponentBuilder pathBuilder = NULL_PATH_COMPONENT_BUILDER; + private CompositePathComponentBuilder pathBuilder = new CompositePathComponentBuilder(); private final MultiValueMap queryParams = new LinkedMultiValueMap(); @@ -334,7 +336,7 @@ public class UriComponentsBuilder { this.port = uri.getPort(); } if (StringUtils.hasLength(uri.getRawPath())) { - this.pathBuilder = new FullPathComponentBuilder(uri.getRawPath()); + this.pathBuilder = new CompositePathComponentBuilder(uri.getRawPath()); } if (StringUtils.hasLength(uri.getRawQuery())) { this.queryParams.clear(); @@ -352,7 +354,7 @@ public class UriComponentsBuilder { this.userInfo = null; this.host = null; this.port = -1; - this.pathBuilder = NULL_PATH_COMPONENT_BUILDER; + this.pathBuilder = new CompositePathComponentBuilder(); this.queryParams.clear(); } @@ -436,12 +438,7 @@ public class UriComponentsBuilder { * @return this UriComponentsBuilder */ public UriComponentsBuilder path(String path) { - if (path != null) { - this.pathBuilder = this.pathBuilder.appendPath(path); - } - else { - this.pathBuilder = NULL_PATH_COMPONENT_BUILDER; - } + this.pathBuilder.addPath(path); resetSchemeSpecificPart(); return this; } @@ -453,22 +450,21 @@ public class UriComponentsBuilder { * @return this UriComponentsBuilder */ public UriComponentsBuilder replacePath(String path) { - this.pathBuilder = NULL_PATH_COMPONENT_BUILDER; - path(path); + this.pathBuilder = new CompositePathComponentBuilder(path); resetSchemeSpecificPart(); return this; } /** - * Appends the given path segments to the existing path of this builder. Each given path segments may contain URI - * template variables. + * Appends the given path segments to the existing path of this builder. Each given + * path segments may contain URI template variables. * * @param pathSegments the URI path segments * @return this UriComponentsBuilder */ public UriComponentsBuilder pathSegment(String... pathSegments) throws IllegalArgumentException { Assert.notNull(pathSegments, "'segments' must not be null"); - this.pathBuilder = this.pathBuilder.appendPathSegments(pathSegments); + this.pathBuilder.addPathSegments(pathSegments); resetSchemeSpecificPart(); return this; } @@ -590,131 +586,122 @@ public class UriComponentsBuilder { return this; } - /** - * Represents a builder for {@link HierarchicalUriComponents.PathComponent} - */ + private interface PathComponentBuilder { - - HierarchicalUriComponents.PathComponent build(); - - PathComponentBuilder appendPath(String path); - - PathComponentBuilder appendPathSegments(String... pathSegments); + PathComponent build(); } - /** - * Represents a builder for full string paths. - */ - private static class FullPathComponentBuilder implements PathComponentBuilder { + private static class CompositePathComponentBuilder implements PathComponentBuilder { - private final StringBuilder path; + private LinkedList componentBuilders = new LinkedList(); - private FullPathComponentBuilder(String path) { - this.path = new StringBuilder(path); + public CompositePathComponentBuilder() { } - public HierarchicalUriComponents.PathComponent build() { - return new HierarchicalUriComponents.FullPathComponent(path.toString()); + public CompositePathComponentBuilder(String path) { + addPath(path); } - public PathComponentBuilder appendPath(String path) { - this.path.append(path); - return this; + public void addPathSegments(String... pathSegments) { + if (!ObjectUtils.isEmpty(pathSegments)) { + PathSegmentComponentBuilder psBuilder = getLastBuilder(PathSegmentComponentBuilder.class); + FullPathComponentBuilder fpBuilder = getLastBuilder(FullPathComponentBuilder.class); + if (psBuilder == null) { + psBuilder = new PathSegmentComponentBuilder(); + this.componentBuilders.add(psBuilder); + if (fpBuilder != null) { + fpBuilder.removeTrailingSlash(); + } + } + psBuilder.append(pathSegments); + } } - public PathComponentBuilder appendPathSegments(String... pathSegments) { - PathComponentCompositeBuilder builder = new PathComponentCompositeBuilder(this); - builder.appendPathSegments(pathSegments); - return builder; - } - } - - /** - * Represents a builder for paths segment paths. - */ - private static class PathSegmentComponentBuilder implements PathComponentBuilder { - - private final List pathSegments = new ArrayList(); - - private PathSegmentComponentBuilder(String... pathSegments) { - this.pathSegments.addAll(removeEmptyPathSegments(pathSegments)); + public void addPath(String path) { + if (StringUtils.hasText(path)) { + PathSegmentComponentBuilder psBuilder = getLastBuilder(PathSegmentComponentBuilder.class); + FullPathComponentBuilder fpBuilder = getLastBuilder(FullPathComponentBuilder.class); + if (psBuilder != null) { + path = path.startsWith("/") ? path : "/" + path; + } + if (fpBuilder == null) { + fpBuilder = new FullPathComponentBuilder(); + this.componentBuilders.add(fpBuilder); + } + fpBuilder.append(path); + } } - private Collection removeEmptyPathSegments(String... pathSegments) { - List result = new ArrayList(); - for (String segment : pathSegments) { - if (StringUtils.hasText(segment)) { - result.add(segment); + @SuppressWarnings("unchecked") + private T getLastBuilder(Class builderClass) { + if (!this.componentBuilders.isEmpty()) { + PathComponentBuilder last = this.componentBuilders.getLast(); + if (builderClass.isInstance(last)) { + return (T) last; } } - return result; + return null; } - public HierarchicalUriComponents.PathComponent build() { - return new HierarchicalUriComponents.PathSegmentComponent(pathSegments); - } - - public PathComponentBuilder appendPath(String path) { - PathComponentCompositeBuilder builder = new PathComponentCompositeBuilder(this); - builder.appendPath(path); - return builder; - } - - public PathComponentBuilder appendPathSegments(String... pathSegments) { - this.pathSegments.addAll(removeEmptyPathSegments(pathSegments)); - return this; - } - } - - /** - * Represents a builder for a collection of PathComponents. - */ - private static class PathComponentCompositeBuilder implements PathComponentBuilder { - - private final List pathComponentBuilders = new ArrayList(); - - private PathComponentCompositeBuilder(PathComponentBuilder builder) { - pathComponentBuilders.add(builder); - } - - public HierarchicalUriComponents.PathComponent build() { - List pathComponents = - new ArrayList(pathComponentBuilders.size()); - - for (PathComponentBuilder pathComponentBuilder : pathComponentBuilders) { - pathComponents.add(pathComponentBuilder.build()); + public PathComponent build() { + int size = this.componentBuilders.size(); + List components = new ArrayList(size); + for (int i = 0; i < size; i++) { + PathComponent pathComponent = this.componentBuilders.get(i).build(); + if (pathComponent != null) { + components.add(pathComponent); + } } - return new HierarchicalUriComponents.PathComponentComposite(pathComponents); - } - - public PathComponentBuilder appendPath(String path) { - this.pathComponentBuilders.add(new FullPathComponentBuilder(path)); - return this; - } - - public PathComponentBuilder appendPathSegments(String... pathSegments) { - this.pathComponentBuilders.add(new PathSegmentComponentBuilder(pathSegments)); - return this; + if (components.isEmpty()) { + return HierarchicalUriComponents.NULL_PATH_COMPONENT; + } + if (components.size() == 1) { + return components.get(0); + } + return new HierarchicalUriComponents.PathComponentComposite(components); } } + private static class FullPathComponentBuilder implements PathComponentBuilder { - /** - * Represents a builder for an empty path. - */ - private static PathComponentBuilder NULL_PATH_COMPONENT_BUILDER = new PathComponentBuilder() { + private StringBuilder path = new StringBuilder(); - public HierarchicalUriComponents.PathComponent build() { - return HierarchicalUriComponents.NULL_PATH_COMPONENT; + public void append(String path) { + this.path.append(path); } - public PathComponentBuilder appendPath(String path) { - return new FullPathComponentBuilder(path); + public PathComponent build() { + if (this.path.length() == 0) { + return null; + } + String path = this.path.toString().replace("//", "/"); + return new HierarchicalUriComponents.FullPathComponent(path); } - public PathComponentBuilder appendPathSegments(String... pathSegments) { - return new PathSegmentComponentBuilder(pathSegments); + public void removeTrailingSlash() { + int index = this.path.length() - 1; + if (this.path.charAt(index) == '/') { + this.path.deleteCharAt(index); + } } - }; + } + + private static class PathSegmentComponentBuilder implements PathComponentBuilder { + + private List pathSegments = new LinkedList(); + + public void append(String... pathSegments) { + for (String pathSegment : pathSegments) { + if (StringUtils.hasText(pathSegment)) { + this.pathSegments.add(pathSegment); + } + } + } + + public PathComponent build() { + return this.pathSegments.isEmpty() ? + null : new HierarchicalUriComponents.PathSegmentComponent(this.pathSegments); + } + } } diff --git a/spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java b/spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java index 3040ee4aa9..25fbb98d59 100644 --- a/spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java +++ b/spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java @@ -346,4 +346,12 @@ public class UriComponentsBuilderTests { assertThat(UriComponentsBuilder.fromUriString("http://example.com").path("foo/../bar").build().toUriString(), equalTo("http://example.com/foo/../bar")); assertThat(UriComponentsBuilder.fromUriString("http://example.com").path("foo/../bar").build().toUri().getPath(), equalTo("/foo/../bar")); } + + @Test + public void emptySegments() throws Exception { + assertThat(UriComponentsBuilder.fromUriString("http://example.com/abc/").path("/x/y/z").build().toString(), equalTo("http://example.com/abc/x/y/z")); + assertThat(UriComponentsBuilder.fromUriString("http://example.com/abc/").pathSegment("x", "y", "z").build().toString(), equalTo("http://example.com/abc/x/y/z")); + assertThat(UriComponentsBuilder.fromUriString("http://example.com/abc/").path("/x/").path("/y/z").build().toString(), equalTo("http://example.com/abc/x/y/z")); + assertThat(UriComponentsBuilder.fromUriString("http://example.com/abc/").pathSegment("x").path("y").build().toString(), equalTo("http://example.com/abc/x/y")); + } }