Skip to content

Commit 9458186

Browse files
committed
Polish DefaultUriBuilderFactory
1 parent d81ec55 commit 9458186

File tree

3 files changed

+89
-86
lines changed

3 files changed

+89
-86
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public void requestResponseHeaderPair() throws Exception {
4747

4848
@Test
4949
public void headerMultipleValues() throws Exception {
50-
this.client.get().uri("header-multi-value")
50+
this.client.get().uri("/header-multi-value")
5151
.exchange()
5252
.expectStatus().isOk()
5353
.expectHeader().valueEquals("h1", "v1", "v2", "v3");

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

Lines changed: 73 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,9 @@
2020
import java.nio.charset.Charset;
2121
import java.util.Collections;
2222
import java.util.HashMap;
23-
import java.util.List;
2423
import java.util.Map;
2524

2625
import org.springframework.lang.Nullable;
27-
import org.springframework.util.Assert;
2826
import org.springframework.util.MultiValueMap;
2927
import org.springframework.util.ObjectUtils;
3028
import org.springframework.util.StringUtils;
@@ -51,43 +49,33 @@ public class DefaultUriBuilderFactory implements UriBuilderFactory {
5149
public enum EncodingMode {
5250

5351
/**
54-
* The default way of encoding that {@link UriComponents} supports:
55-
* <ol>
56-
* <li>Expand URI variables.
57-
* <li>Encode individual URI components as described in
58-
* {@link UriComponents#encode(Charset)}.
59-
* </ol>
60-
* <p>This mode <strong>does not</strong> encode all characters with
61-
* reserved meaning but only the ones that are illegal within a given
62-
* URI component as defined in RFC 396. This matches the way the
63-
* multi-argument {@link URI} constructor does encoding.
52+
* Apply strict encoding to URI variables at the time of expanding,
53+
* quoting both illegal characters and characters with reserved meaning
54+
* via {@link UriUtils#encode(String, Charset)}.
6455
*/
65-
URI_COMPONENT,
56+
VALUES_ONLY,
6657

6758
/**
68-
* Comprehensive encoding of URI variable values prior to expanding:
69-
* <ol>
70-
* <li>Apply {@link UriUtils#encode(String, Charset)} to each URI variable value.
71-
* <li>Expand URI variable values.
72-
* </ol>
73-
* <p>This mode encodes all characters with reserved meaning, therefore
74-
* ensuring that expanded URI variable do not have any impact on the
75-
* structure or meaning of the URI.
59+
* Expand URI variables first, then encode the resulting URI component
60+
* values, quoting <em>only</em> illegal characters within a given URI
61+
* component type, but not all characters with reserved meaning.
7662
*/
77-
VALUES_ONLY,
63+
URI_COMPONENT,
7864

7965
/**
8066
* No encoding should be applied.
8167
*/
82-
NONE }
68+
NONE
69+
}
8370

8471

72+
@Nullable
8573
private final UriComponentsBuilder baseUri;
8674

87-
private final Map<String, Object> defaultUriVariables = new HashMap<>();
88-
8975
private EncodingMode encodingMode = EncodingMode.URI_COMPONENT;
9076

77+
private final Map<String, Object> defaultUriVariables = new HashMap<>();
78+
9179
private boolean parsePath = true;
9280

9381

@@ -96,7 +84,7 @@ public enum EncodingMode {
9684
* <p>The target address must be specified on each UriBuilder.
9785
*/
9886
public DefaultUriBuilderFactory() {
99-
this(UriComponentsBuilder.newInstance());
87+
this.baseUri = null;
10088
}
10189

10290
/**
@@ -109,19 +97,35 @@ public DefaultUriBuilderFactory() {
10997
* @param baseUriTemplate the URI template to use a base URL
11098
*/
11199
public DefaultUriBuilderFactory(String baseUriTemplate) {
112-
this(UriComponentsBuilder.fromUriString(baseUriTemplate));
100+
this.baseUri = UriComponentsBuilder.fromUriString(baseUriTemplate);
113101
}
114102

115103
/**
116104
* Variant of {@link #DefaultUriBuilderFactory(String)} with a
117105
* {@code UriComponentsBuilder}.
118106
*/
119107
public DefaultUriBuilderFactory(UriComponentsBuilder baseUri) {
120-
Assert.notNull(baseUri, "'baseUri' is required");
121108
this.baseUri = baseUri;
122109
}
123110

124111

112+
/**
113+
* Specify the {@link EncodingMode EncodingMode} to use when building URIs.
114+
* <p>By default set to
115+
* {@link EncodingMode#URI_COMPONENT EncodingMode.URI_COMPONENT}.
116+
* @param encodingMode the encoding mode to use
117+
*/
118+
public void setEncodingMode(EncodingMode encodingMode) {
119+
this.encodingMode = encodingMode;
120+
}
121+
122+
/**
123+
* Return the configured encoding mode.
124+
*/
125+
public EncodingMode getEncodingMode() {
126+
return this.encodingMode;
127+
}
128+
125129
/**
126130
* Provide default URI variable values to use when expanding URI templates
127131
* with a Map of variables.
@@ -142,30 +146,10 @@ public void setDefaultUriVariables(@Nullable Map<String, ?> defaultUriVariables)
142146
}
143147

144148
/**
145-
* Specify the {@link EncodingMode EncodingMode} to use when building URIs.
146-
* <p>By default set to
147-
* {@link EncodingMode#URI_COMPONENT EncodingMode.URI_COMPONENT}.
148-
* @param encodingMode the encoding mode to use
149-
*/
150-
public void setEncodingMode(EncodingMode encodingMode) {
151-
this.encodingMode = encodingMode;
152-
}
153-
154-
/**
155-
* Return the configured encoding mode.
156-
*/
157-
public EncodingMode getEncodingMode() {
158-
return this.encodingMode;
159-
}
160-
161-
/**
162-
* Whether to parse the path into path segments for the URI string passed
163-
* into {@link #uriString(String)} or one of the expand methods.
164-
* <p>Setting this property to {@code true} ensures that URI variables
165-
* expanded into the path are subject to path segment encoding rules and
166-
* "/" characters are percent-encoded. If set to {@code false} the path is
167-
* kept as a full path and expanded URI variables will have "/" characters
168-
* preserved.
149+
* Whether to parse the input path into path segments if the encoding mode
150+
* is set to {@link EncodingMode#URI_COMPONENT EncodingMode.URI_COMPONENT},
151+
* which ensures that URI variables in the path are encoded according to
152+
* path segment rules and for example a '/' is encoded.
169153
* <p>By default this is set to {@code true}.
170154
* @param parsePath whether to parse the path into path segments
171155
*/
@@ -174,7 +158,8 @@ public void setParsePath(boolean parsePath) {
174158
}
175159

176160
/**
177-
* Whether the handler is configured to parse the path into path segments.
161+
* Whether to parse the path into path segments if the encoding mode is set
162+
* to {@link EncodingMode#URI_COMPONENT EncodingMode.URI_COMPONENT}.
178163
*/
179164
public boolean shouldParsePath() {
180165
return this.parsePath;
@@ -210,30 +195,47 @@ private class DefaultUriBuilder implements UriBuilder {
210195

211196
private final UriComponentsBuilder uriComponentsBuilder;
212197

198+
213199
public DefaultUriBuilder(String uriTemplate) {
214200
this.uriComponentsBuilder = initUriComponentsBuilder(uriTemplate);
215201
}
216202

217203
private UriComponentsBuilder initUriComponentsBuilder(String uriTemplate) {
218-
UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.fromUriString(uriTemplate);
219-
UriComponents uriComponents = uriComponentsBuilder.build();
220-
UriComponentsBuilder result = (uriComponents.getHost() == null ?
221-
baseUri.cloneBuilder().uriComponents(uriComponents) : uriComponentsBuilder);
222204

223-
if (shouldParsePath()) {
205+
if (StringUtils.isEmpty(uriTemplate)) {
206+
return baseUri != null ? baseUri.cloneBuilder() : UriComponentsBuilder.newInstance();
207+
}
208+
209+
UriComponentsBuilder result;
210+
if (baseUri != null) {
211+
UriComponentsBuilder uricBuilder = UriComponentsBuilder.fromUriString(uriTemplate);
212+
UriComponents uric = uricBuilder.build();
213+
result = uric.getHost() == null ? baseUri.cloneBuilder().uriComponents(uric) : uricBuilder;
214+
}
215+
else {
216+
result = UriComponentsBuilder.fromUriString(uriTemplate);
217+
}
218+
219+
parsePathIfNecessary(result);
220+
221+
return result;
222+
}
223+
224+
private void parsePathIfNecessary(UriComponentsBuilder result) {
225+
if (shouldParsePath() && encodingMode.equals(EncodingMode.URI_COMPONENT)) {
224226
UriComponents uric = result.build();
225227
String path = uric.getPath();
226-
List<String> pathSegments = uric.getPathSegments();
227228
result.replacePath(null);
228-
result.pathSegment(StringUtils.toStringArray(pathSegments));
229+
for (String segment : uric.getPathSegments()) {
230+
result.pathSegment(segment);
231+
}
229232
if (path != null && path.endsWith("/")) {
230233
result.path("/");
231234
}
232235
}
233-
234-
return result;
235236
}
236237

238+
237239
@Override
238240
public DefaultUriBuilder scheme(@Nullable String scheme) {
239241
this.uriComponentsBuilder.scheme(scheme);
@@ -335,11 +337,8 @@ public URI build(Map<String, ?> uriVars) {
335337
if (encodingMode.equals(EncodingMode.VALUES_ONLY)) {
336338
uriVars = UriUtils.encodeUriVariables(uriVars);
337339
}
338-
UriComponents uriComponents = this.uriComponentsBuilder.build().expand(uriVars);
339-
if (encodingMode.equals(EncodingMode.URI_COMPONENT)) {
340-
uriComponents = uriComponents.encode();
341-
}
342-
return URI.create(uriComponents.toString());
340+
UriComponents uric = this.uriComponentsBuilder.build().expand(uriVars);
341+
return createUri(uric);
343342
}
344343

345344
@Override
@@ -350,11 +349,15 @@ public URI build(Object... uriVars) {
350349
if (encodingMode.equals(EncodingMode.VALUES_ONLY)) {
351350
uriVars = UriUtils.encodeUriVariables(uriVars);
352351
}
353-
UriComponents uriComponents = this.uriComponentsBuilder.build().expand(uriVars);
352+
UriComponents uric = this.uriComponentsBuilder.build().expand(uriVars);
353+
return createUri(uric);
354+
}
355+
356+
private URI createUri(UriComponents uric) {
354357
if (encodingMode.equals(EncodingMode.URI_COMPONENT)) {
355-
uriComponents = uriComponents.encode();
358+
uric = uric.encode();
356359
}
357-
return URI.create(uriComponents.toString());
360+
return URI.create(uric.toString());
358361
}
359362
}
360363

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

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -33,60 +33,60 @@
3333
public class DefaultUriBuilderFactoryTests {
3434

3535
@Test
36-
public void defaultSettings() throws Exception {
36+
public void defaultSettings() {
3737
DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory();
3838
URI uri = factory.uriString("/foo").pathSegment("{id}").build("a/b");
3939
assertEquals("/foo/a%2Fb", uri.toString());
4040
}
4141

4242
@Test
43-
public void baseUri() throws Exception {
43+
public void baseUri() {
4444
DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory("http://foo.com/v1?id=123");
4545
URI uri = factory.uriString("/bar").port(8080).build();
4646
assertEquals("http://foo.com:8080/v1/bar?id=123", uri.toString());
4747
}
4848

4949
@Test
50-
public void baseUriWithFullOverride() throws Exception {
50+
public void baseUriWithFullOverride() {
5151
DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory("http://foo.com/v1?id=123");
5252
URI uri = factory.uriString("http://example.com/1/2").build();
5353
assertEquals("Use of host should case baseUri to be completely ignored",
5454
"http://example.com/1/2", uri.toString());
5555
}
5656

5757
@Test
58-
public void baseUriWithPathOverride() throws Exception {
58+
public void baseUriWithPathOverride() {
5959
DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory("http://foo.com/v1");
6060
URI uri = factory.builder().replacePath("/baz").build();
6161
assertEquals("http://foo.com/baz", uri.toString());
6262
}
6363

6464
@Test
65-
public void defaultUriVars() throws Exception {
65+
public void defaultUriVars() {
6666
DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory("http://{host}/v1");
6767
factory.setDefaultUriVariables(singletonMap("host", "foo.com"));
6868
URI uri = factory.uriString("/{id}").build(singletonMap("id", "123"));
6969
assertEquals("http://foo.com/v1/123", uri.toString());
7070
}
7171

7272
@Test
73-
public void defaultUriVarsWithOverride() throws Exception {
73+
public void defaultUriVarsWithOverride() {
7474
DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory("http://{host}/v1");
7575
factory.setDefaultUriVariables(singletonMap("host", "spring.io"));
7676
URI uri = factory.uriString("/bar").build(singletonMap("host", "docs.spring.io"));
7777
assertEquals("http://docs.spring.io/v1/bar", uri.toString());
7878
}
7979

8080
@Test
81-
public void defaultUriVarsWithEmptyVarArg() throws Exception {
81+
public void defaultUriVarsWithEmptyVarArg() {
8282
DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory("http://{host}/v1");
8383
factory.setDefaultUriVariables(singletonMap("host", "foo.com"));
8484
URI uri = factory.uriString("/bar").build();
8585
assertEquals("Expected delegation to build(Map) method", "http://foo.com/v1/bar", uri.toString());
8686
}
8787

8888
@Test
89-
public void defaultUriVarsSpr14147() throws Exception {
89+
public void defaultUriVarsSpr14147() {
9090
Map<String, String> defaultUriVars = new HashMap<>(2);
9191
defaultUriVars.put("host", "api.example.com");
9292
defaultUriVars.put("port", "443");
@@ -98,7 +98,7 @@ public void defaultUriVarsSpr14147() throws Exception {
9898
}
9999

100100
@Test
101-
public void encodingValuesOnly() throws Exception {
101+
public void encodingValuesOnly() {
102102
DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory();
103103
factory.setEncodingMode(EncodingMode.VALUES_ONLY);
104104
UriBuilder uriBuilder = factory.uriString("/foo/a%2Fb/{id}");
@@ -111,7 +111,7 @@ public void encodingValuesOnly() throws Exception {
111111
}
112112

113113
@Test
114-
public void encodingValuesOnlySpr14147() throws Exception {
114+
public void encodingValuesOnlySpr14147() {
115115
DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory();
116116
factory.setEncodingMode(EncodingMode.VALUES_ONLY);
117117
factory.setDefaultUriVariables(singletonMap("host", "www.example.com"));
@@ -122,7 +122,7 @@ public void encodingValuesOnlySpr14147() throws Exception {
122122
}
123123

124124
@Test
125-
public void encodingNone() throws Exception {
125+
public void encodingNone() {
126126
DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory();
127127
factory.setEncodingMode(EncodingMode.NONE);
128128
UriBuilder uriBuilder = factory.uriString("/foo/a%2Fb/{id}");
@@ -135,29 +135,29 @@ public void encodingNone() throws Exception {
135135
}
136136

137137
@Test
138-
public void parsePathWithDefaultSettings() throws Exception {
138+
public void parsePathWithDefaultSettings() {
139139
DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory("/foo/{bar}");
140140
URI uri = factory.uriString("/baz/{id}").build("a/b", "c/d");
141141
assertEquals("/foo/a%2Fb/baz/c%2Fd", uri.toString());
142142
}
143143

144144
@Test
145-
public void parsePathIsTurnedOff() throws Exception {
145+
public void parsePathIsTurnedOff() {
146146
DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory("/foo/{bar}");
147147
factory.setParsePath(false);
148148
URI uri = factory.uriString("/baz/{id}").build("a/b", "c/d");
149149
assertEquals("/foo/a/b/baz/c/d", uri.toString());
150150
}
151151

152152
@Test // SPR-15201
153-
public void pathWithTrailingSlash() throws Exception {
153+
public void pathWithTrailingSlash() {
154154
DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory();
155155
URI uri = factory.expand("http://localhost:8080/spring/");
156156
assertEquals("http://localhost:8080/spring/", uri.toString());
157157
}
158158

159159
@Test
160-
public void pathWithDuplicateSlashes() throws Exception {
160+
public void pathWithDuplicateSlashes() {
161161
DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory();
162162
URI uri = factory.expand("/foo/////////bar");
163163
assertEquals("/foo/bar", uri.toString());

0 commit comments

Comments
 (0)