Polish DefaultUriBuilderFactory

spring-test/src/test/java/org/springframework/test/web/reactive/server/samples/HeaderAndCookieTests.java

@@ -47,7 +47,7 @@ public class HeaderAndCookieTests {
 
 	@Test
 	public void headerMultipleValues() throws Exception {
-		this.client.get().uri("header-multi-value")
+		this.client.get().uri("/header-multi-value")
 				.exchange()
 				.expectStatus().isOk()
 				.expectHeader().valueEquals("h1", "v1", "v2", "v3");

spring-web/src/main/java/org/springframework/web/util/DefaultUriBuilderFactory.java

@@ -20,11 +20,9 @@ import java.net.URI;
 import java.nio.charset.Charset;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
 
 import org.springframework.lang.Nullable;
-import org.springframework.util.Assert;
 import org.springframework.util.MultiValueMap;
 import org.springframework.util.ObjectUtils;
 import org.springframework.util.StringUtils;
@@ -51,43 +49,33 @@ public class DefaultUriBuilderFactory implements UriBuilderFactory {
 	public enum EncodingMode {
 
 		/**
-		 * The default way of encoding that {@link UriComponents} supports:
+		 * Apply strict encoding to URI variables at the time of expanding,
-		 * <ol>
+		 * quoting both illegal characters and characters with reserved meaning
-		 * <li>Expand URI variables.
+		 * via {@link UriUtils#encode(String, Charset)}.
-		 * <li>Encode individual URI components as described in
-		 * {@link UriComponents#encode(Charset)}.
-		 * </ol>
-		 * <p>This mode <strong>does not</strong> encode all characters with
-		 * reserved meaning but only the ones that are illegal within a given
-		 * URI component as defined in RFC 396. This matches the way the
-		 * multi-argument {@link URI} constructor does encoding.
-		 */
-		URI_COMPONENT,
-
-		/**
-		 * Comprehensive encoding of URI variable values prior to expanding:
-		 * <ol>
-		 * <li>Apply {@link UriUtils#encode(String, Charset)} to each URI variable value.
-		 * <li>Expand URI variable values.
-		 * </ol>
-		 * <p>This mode encodes all characters with reserved meaning, therefore
-		 * ensuring that expanded URI variable do not have any impact on the
-		 * structure or meaning of the URI.
 		 */
 		VALUES_ONLY,
 
+		/**
+		 * Expand URI variables first, then encode the resulting URI component
+		 * values, quoting <em>only</em> illegal characters within a given URI
+		 * component type, but not all characters with reserved meaning.
+		 */
+		URI_COMPONENT,
+
 		/**
 		 * No encoding should be applied.
 		 */
-		NONE }
+		NONE
+	}
 
 
+	@Nullable
 	private final UriComponentsBuilder baseUri;
 
-	private final Map<String, Object> defaultUriVariables = new HashMap<>();
-
 	private EncodingMode encodingMode = EncodingMode.URI_COMPONENT;
 
+	private final Map<String, Object> defaultUriVariables = new HashMap<>();
+
 	private boolean parsePath = true;
 
 
@@ -96,7 +84,7 @@ public class DefaultUriBuilderFactory implements UriBuilderFactory {
 	 * <p>The target address must be specified on each UriBuilder.
 	 */
 	public DefaultUriBuilderFactory() {
-		this(UriComponentsBuilder.newInstance());
+		this.baseUri = null;
 	}
 
 	/**
@@ -109,7 +97,7 @@ public class DefaultUriBuilderFactory implements UriBuilderFactory {
 	 * @param baseUriTemplate the URI template to use a base URL
 	 */
 	public DefaultUriBuilderFactory(String baseUriTemplate) {
-		this(UriComponentsBuilder.fromUriString(baseUriTemplate));
+		this.baseUri = UriComponentsBuilder.fromUriString(baseUriTemplate);
 	}
 
 	/**
@@ -117,11 +105,27 @@ public class DefaultUriBuilderFactory implements UriBuilderFactory {
 	 * {@code UriComponentsBuilder}.
 	 */
 	public DefaultUriBuilderFactory(UriComponentsBuilder baseUri) {
-		Assert.notNull(baseUri, "'baseUri' is required");
 		this.baseUri = baseUri;
 	}
 
 
+	/**
+	 * Specify the {@link EncodingMode EncodingMode} to use when building URIs.
+	 * <p>By default set to
+	 * {@link EncodingMode#URI_COMPONENT EncodingMode.URI_COMPONENT}.
+	 * @param encodingMode the encoding mode to use
+	 */
+	public void setEncodingMode(EncodingMode encodingMode) {
+		this.encodingMode = encodingMode;
+	}
+
+	/**
+	 * Return the configured encoding mode.
+	 */
+	public EncodingMode getEncodingMode() {
+		return this.encodingMode;
+	}
+
 	/**
 	 * Provide default URI variable values to use when expanding URI templates
 	 * with a Map of variables.
@@ -142,30 +146,10 @@ public class DefaultUriBuilderFactory implements UriBuilderFactory {
 	}
 
 	/**
-	 * Specify the {@link EncodingMode EncodingMode} to use when building URIs.
+	 * Whether to parse the input path into path segments if the encoding mode
-	 * <p>By default set to
+	 * is set to {@link EncodingMode#URI_COMPONENT EncodingMode.URI_COMPONENT},
-	 * {@link EncodingMode#URI_COMPONENT EncodingMode.URI_COMPONENT}.
+	 * which ensures that URI variables in the path are encoded according to
-	 * @param encodingMode the encoding mode to use
+	 * path segment rules and for example a '/' is encoded.
-	 */
-	public void setEncodingMode(EncodingMode encodingMode) {
-		this.encodingMode = encodingMode;
-	}
-
-	/**
-	 * Return the configured encoding mode.
-	 */
-	public EncodingMode getEncodingMode() {
-		return this.encodingMode;
-	}
-
-	/**
-	 * Whether to parse the path into path segments for the URI string passed
-	 * into {@link #uriString(String)} or one of the expand methods.
-	 * <p>Setting this property to {@code true} ensures that URI variables
-	 * expanded into the path are subject to path segment encoding rules and
-	 * "/" characters are percent-encoded. If set to {@code false} the path is
-	 * kept as a full path and expanded URI variables will have "/" characters
-	 * preserved.
 	 * <p>By default this is set to {@code true}.
 	 * @param parsePath whether to parse the path into path segments
 	 */
@@ -174,7 +158,8 @@ public class DefaultUriBuilderFactory implements UriBuilderFactory {
 	}
 
 	/**
-	 * Whether the handler is configured to parse the path into path segments.
+	 * Whether to parse the path into path segments if the encoding mode is set
+	 * to {@link EncodingMode#URI_COMPONENT EncodingMode.URI_COMPONENT}.
 	 */
 	public boolean shouldParsePath() {
 		return this.parsePath;
@@ -210,30 +195,47 @@ public class DefaultUriBuilderFactory implements UriBuilderFactory {
 
 		private final UriComponentsBuilder uriComponentsBuilder;
 
+
 		public DefaultUriBuilder(String uriTemplate) {
 			this.uriComponentsBuilder = initUriComponentsBuilder(uriTemplate);
 		}
 
 		private UriComponentsBuilder initUriComponentsBuilder(String uriTemplate) {
-			UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.fromUriString(uriTemplate);
-			UriComponents uriComponents = uriComponentsBuilder.build();
-			UriComponentsBuilder result = (uriComponents.getHost() == null ?
-					baseUri.cloneBuilder().uriComponents(uriComponents) : uriComponentsBuilder);
 
-			if (shouldParsePath()) {
+			if (StringUtils.isEmpty(uriTemplate)) {
+				return baseUri != null ? baseUri.cloneBuilder() : UriComponentsBuilder.newInstance();
+			}
+
+			UriComponentsBuilder result;
+			if (baseUri != null) {
+				UriComponentsBuilder uricBuilder = UriComponentsBuilder.fromUriString(uriTemplate);
+				UriComponents uric = uricBuilder.build();
+				result = uric.getHost() == null ? baseUri.cloneBuilder().uriComponents(uric) : uricBuilder;
+			}
+			else {
+				result = UriComponentsBuilder.fromUriString(uriTemplate);
+			}
+
+			parsePathIfNecessary(result);
+
+			return result;
+		}
+
+		private void parsePathIfNecessary(UriComponentsBuilder result) {
+			if (shouldParsePath() && encodingMode.equals(EncodingMode.URI_COMPONENT)) {
 				UriComponents uric = result.build();
 				String path = uric.getPath();
-				List<String> pathSegments = uric.getPathSegments();
 				result.replacePath(null);
-				result.pathSegment(StringUtils.toStringArray(pathSegments));
+				for (String segment : uric.getPathSegments()) {
+					result.pathSegment(segment);
+				}
 				if (path != null && path.endsWith("/")) {
 					result.path("/");
 				}
 			}
-
-			return result;
 		}
 
+
 		@Override
 		public DefaultUriBuilder scheme(@Nullable String scheme) {
 			this.uriComponentsBuilder.scheme(scheme);
@@ -335,11 +337,8 @@ public class DefaultUriBuilderFactory implements UriBuilderFactory {
 			if (encodingMode.equals(EncodingMode.VALUES_ONLY)) {
 				uriVars = UriUtils.encodeUriVariables(uriVars);
 			}
-			UriComponents uriComponents = this.uriComponentsBuilder.build().expand(uriVars);
+			UriComponents uric = this.uriComponentsBuilder.build().expand(uriVars);
-			if (encodingMode.equals(EncodingMode.URI_COMPONENT)) {
+			return createUri(uric);
-				uriComponents = uriComponents.encode();
-			}
-			return URI.create(uriComponents.toString());
 		}
 
 		@Override
@@ -350,11 +349,15 @@ public class DefaultUriBuilderFactory implements UriBuilderFactory {
 			if (encodingMode.equals(EncodingMode.VALUES_ONLY)) {
 				uriVars = UriUtils.encodeUriVariables(uriVars);
 			}
-			UriComponents uriComponents = this.uriComponentsBuilder.build().expand(uriVars);
+			UriComponents uric = this.uriComponentsBuilder.build().expand(uriVars);
+			return createUri(uric);
+		}
+
+		private URI createUri(UriComponents uric) {
 			if (encodingMode.equals(EncodingMode.URI_COMPONENT)) {
-				uriComponents = uriComponents.encode();
+				uric = uric.encode();
 			}
-			return URI.create(uriComponents.toString());
+			return URI.create(uric.toString());
 		}
 	}
 

spring-web/src/test/java/org/springframework/web/util/DefaultUriBuilderFactoryTests.java

@@ -33,21 +33,21 @@ import static junit.framework.TestCase.assertEquals;
 public class DefaultUriBuilderFactoryTests {
 
 	@Test
-	public void defaultSettings() throws Exception {
+	public void defaultSettings() {
 		DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory();
 		URI uri = factory.uriString("/foo").pathSegment("{id}").build("a/b");
 		assertEquals("/foo/a%2Fb", uri.toString());
 	}
 
 	@Test
-	public void baseUri() throws Exception {
+	public void baseUri() {
 		DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory("http://foo.com/v1?id=123");
 		URI uri = factory.uriString("/bar").port(8080).build();
 		assertEquals("http://foo.com:8080/v1/bar?id=123", uri.toString());
 	}
 
 	@Test
-	public void baseUriWithFullOverride() throws Exception {
+	public void baseUriWithFullOverride() {
 		DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory("http://foo.com/v1?id=123");
 		URI uri = factory.uriString("http://example.com/1/2").build();
 		assertEquals("Use of host should case baseUri to be completely ignored",
@@ -55,14 +55,14 @@ public class DefaultUriBuilderFactoryTests {
 	}
 
 	@Test
-	public void baseUriWithPathOverride() throws Exception {
+	public void baseUriWithPathOverride() {
 		DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory("http://foo.com/v1");
 		URI uri = factory.builder().replacePath("/baz").build();
 		assertEquals("http://foo.com/baz", uri.toString());
 	}
 
 	@Test
-	public void defaultUriVars() throws Exception {
+	public void defaultUriVars() {
 		DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory("http://{host}/v1");
 		factory.setDefaultUriVariables(singletonMap("host", "foo.com"));
 		URI uri = factory.uriString("/{id}").build(singletonMap("id", "123"));
@@ -70,7 +70,7 @@ public class DefaultUriBuilderFactoryTests {
 	}
 
 	@Test
-	public void defaultUriVarsWithOverride() throws Exception {
+	public void defaultUriVarsWithOverride() {
 		DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory("http://{host}/v1");
 		factory.setDefaultUriVariables(singletonMap("host", "spring.io"));
 		URI uri = factory.uriString("/bar").build(singletonMap("host", "docs.spring.io"));
@@ -78,7 +78,7 @@ public class DefaultUriBuilderFactoryTests {
 	}
 
 	@Test
-	public void defaultUriVarsWithEmptyVarArg() throws Exception {
+	public void defaultUriVarsWithEmptyVarArg() {
 		DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory("http://{host}/v1");
 		factory.setDefaultUriVariables(singletonMap("host", "foo.com"));
 		URI uri = factory.uriString("/bar").build();
@@ -86,7 +86,7 @@ public class DefaultUriBuilderFactoryTests {
 	}
 
 	@Test
-	public void defaultUriVarsSpr14147() throws Exception {
+	public void defaultUriVarsSpr14147() {
 		Map<String, String> defaultUriVars = new HashMap<>(2);
 		defaultUriVars.put("host", "api.example.com");
 		defaultUriVars.put("port", "443");
@@ -98,7 +98,7 @@ public class DefaultUriBuilderFactoryTests {
 	}
 
 	@Test
-	public void encodingValuesOnly() throws Exception {
+	public void encodingValuesOnly() {
 		DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory();
 		factory.setEncodingMode(EncodingMode.VALUES_ONLY);
 		UriBuilder uriBuilder = factory.uriString("/foo/a%2Fb/{id}");
@@ -111,7 +111,7 @@ public class DefaultUriBuilderFactoryTests {
 	}
 
 	@Test
-	public void encodingValuesOnlySpr14147() throws Exception {
+	public void encodingValuesOnlySpr14147() {
 		DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory();
 		factory.setEncodingMode(EncodingMode.VALUES_ONLY);
 		factory.setDefaultUriVariables(singletonMap("host", "www.example.com"));
@@ -122,7 +122,7 @@ public class DefaultUriBuilderFactoryTests {
 	}
 
 	@Test
-	public void encodingNone() throws Exception {
+	public void encodingNone() {
 		DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory();
 		factory.setEncodingMode(EncodingMode.NONE);
 		UriBuilder uriBuilder = factory.uriString("/foo/a%2Fb/{id}");
@@ -135,14 +135,14 @@ public class DefaultUriBuilderFactoryTests {
 	}
 
 	@Test
-	public void parsePathWithDefaultSettings() throws Exception {
+	public void parsePathWithDefaultSettings() {
 		DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory("/foo/{bar}");
 		URI uri = factory.uriString("/baz/{id}").build("a/b", "c/d");
 		assertEquals("/foo/a%2Fb/baz/c%2Fd", uri.toString());
 	}
 
 	@Test
-	public void parsePathIsTurnedOff() throws Exception {
+	public void parsePathIsTurnedOff() {
 		DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory("/foo/{bar}");
 		factory.setParsePath(false);
 		URI uri = factory.uriString("/baz/{id}").build("a/b", "c/d");
@@ -150,14 +150,14 @@ public class DefaultUriBuilderFactoryTests {
 	}
 
 	@Test // SPR-15201
-	public void pathWithTrailingSlash() throws Exception {
+	public void pathWithTrailingSlash() {
 		DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory();
 		URI uri = factory.expand("http://localhost:8080/spring/");
 		assertEquals("http://localhost:8080/spring/", uri.toString());
 	}
 
 	@Test
-	public void pathWithDuplicateSlashes() throws Exception {
+	public void pathWithDuplicateSlashes() {
 		DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory();
 		URI uri = factory.expand("/foo/////////bar");
 		assertEquals("/foo/bar", uri.toString());