Skip to content

Commit 60d2fdf

Browse files
committed
SPR-6188 - UriTemplate: Insufficient handling of characters that need to be escaped.
1 parent 8be36fa commit 60d2fdf

File tree

7 files changed

+454
-152
lines changed

7 files changed

+454
-152
lines changed

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/tags/UrlTag.java

Lines changed: 22 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.springframework.web.util.JavaScriptUtils;
3535
import org.springframework.web.util.TagUtils;
3636
import org.springframework.web.util.UriUtils;
37+
import org.springframework.util.StringUtils;
3738

3839
/**
3940
* JSP tag for creating URLs. Modeled after the JSTL c:url tag with backwards
@@ -236,23 +237,29 @@ private String createUrl() throws JspException {
236237
* @return the query string
237238
* @throws JspException
238239
*/
239-
protected String createQueryString(
240-
List<Param> params, Set<String> usedParams, boolean includeQueryStringDelimiter)
240+
protected String createQueryString(List<Param> params, Set<String> usedParams, boolean includeQueryStringDelimiter)
241241
throws JspException {
242242

243+
String encoding = pageContext.getResponse().getCharacterEncoding();
244+
243245
StringBuilder qs = new StringBuilder();
244246
for (Param param : params) {
245-
if (!usedParams.contains(param.getName()) && param.getName() != null && !"".equals(param.getName())) {
247+
if (!usedParams.contains(param.getName()) && StringUtils.hasLength(param.getName())) {
246248
if (includeQueryStringDelimiter && qs.length() == 0) {
247249
qs.append("?");
248250
}
249251
else {
250252
qs.append("&");
251253
}
252-
qs.append(urlEncode(param.getName()));
253-
if (param.getValue() != null) {
254-
qs.append("=");
255-
qs.append(urlEncode(param.getValue()));
254+
try {
255+
qs.append(UriUtils.encodeQueryParam(param.getName(), encoding));
256+
if (param.getValue() != null) {
257+
qs.append("=");
258+
qs.append(UriUtils.encodeQueryParam(param.getValue(), encoding));
259+
}
260+
}
261+
catch (UnsupportedEncodingException ex) {
262+
throw new JspException(ex);
256263
}
257264
}
258265
}
@@ -271,39 +278,23 @@ protected String createQueryString(
271278
*/
272279
protected String replaceUriTemplateParams(String uri, List<Param> params, Set<String> usedParams)
273280
throws JspException {
281+
String encoding = pageContext.getResponse().getCharacterEncoding();
282+
274283
for (Param param : params) {
275284
String template = URL_TEMPLATE_DELIMITER_PREFIX + param.getName() + URL_TEMPLATE_DELIMITER_SUFFIX;
276285
if (uri.contains(template)) {
277286
usedParams.add(param.getName());
278-
uri = uri.replace(template, urlEncode(param.getValue()));
287+
try {
288+
uri = uri.replace(template, UriUtils.encodePath(param.getValue(), encoding));
289+
}
290+
catch (UnsupportedEncodingException ex) {
291+
throw new JspException(ex);
292+
}
279293
}
280294
}
281295
return uri;
282296
}
283297

284-
/**
285-
* URL-encode the provided String using the character encoding for the response.
286-
* <p>This method will <strong>not</strong> URL-encode according to the
287-
* <code>application/x-www-form-urlencoded</code> MIME format. Spaces will
288-
* encoded as regular character instead of <code>+</code>. In <code>UTF-8</code>
289-
* a space encodes to <code>%20</code>.
290-
* @param value the value to encode
291-
* @return the URL encoded value
292-
* @throws JspException if the character encoding is invalid
293-
*/
294-
protected String urlEncode(String value) throws JspException {
295-
if (value == null) {
296-
return null;
297-
}
298-
try {
299-
String encoding = pageContext.getResponse().getCharacterEncoding();
300-
return UriUtils.encode(value, encoding);
301-
}
302-
catch (UnsupportedEncodingException ex) {
303-
throw new JspException(ex);
304-
}
305-
}
306-
307298
/**
308299
* Internal enum that classifies URLs by type.
309300
*/

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/tags/UrlTagTests.java

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -550,26 +550,6 @@ public void testCreateUrlWithParamAndExsistingQueryString()
550550
assertEquals("url/path?foo=bar&name=value", uri);
551551
}
552552

553-
public void testUrlEncode() throws JspException {
554-
assertEquals("my%20name%2Bis", tag.urlEncode("my name+is"));
555-
}
556-
557-
public void testUrlEncodeNull() throws JspException {
558-
assertNull(tag.urlEncode(null));
559-
}
560-
561-
public void testUrlEncodeBadEncoding() {
562-
context.getResponse().setCharacterEncoding("bad encoding");
563-
564-
try {
565-
tag.urlEncode("my name");
566-
fail("expected JspException");
567-
}
568-
catch (JspException e) {
569-
// we want this
570-
}
571-
}
572-
573553
public void testJspWriterOutput() {
574554
// TODO assert that the output to the JspWriter is the expected output
575555
}

org.springframework.web/src/main/java/org/springframework/web/util/UriTemplate.java

Lines changed: 7 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.util.Map;
2626
import java.util.regex.Matcher;
2727
import java.util.regex.Pattern;
28+
import java.io.UnsupportedEncodingException;
2829

2930
import org.springframework.util.Assert;
3031

@@ -175,48 +176,12 @@ public String toString() {
175176

176177
private static URI encodeUri(String uri) {
177178
try {
178-
int schemeIdx = uri.indexOf(':');
179-
if (schemeIdx == -1) {
180-
int queryIdx = uri.indexOf('?');
181-
if (queryIdx == -1) {
182-
int fragmentIdx = uri.indexOf('#');
183-
if (fragmentIdx == -1) {
184-
return new URI(null, null, uri, null);
185-
}
186-
else {
187-
String path = uri.substring(0, fragmentIdx);
188-
String fragment = uri.substring(fragmentIdx + 1);
189-
return new URI(null, null, path, fragment);
190-
}
191-
}
192-
else {
193-
int fragmentIdx = uri.indexOf('#', queryIdx + 1);
194-
if (fragmentIdx == -1) {
195-
String path = uri.substring(0, queryIdx);
196-
String query = uri.substring(queryIdx + 1);
197-
return new URI(null, null, path, query, null);
198-
}
199-
else {
200-
String path = uri.substring(0, queryIdx);
201-
String query = uri.substring(queryIdx + 1, fragmentIdx);
202-
String fragment = uri.substring(fragmentIdx + 1);
203-
return new URI(null, null, path, query, fragment);
204-
}
205-
}
206-
}
207-
else {
208-
int fragmentIdx = uri.indexOf('#', schemeIdx + 1);
209-
String scheme = uri.substring(0, schemeIdx);
210-
if (fragmentIdx == -1) {
211-
String ssp = uri.substring(schemeIdx + 1);
212-
return new URI(scheme, ssp, null);
213-
}
214-
else {
215-
String ssp = uri.substring(schemeIdx + 1, fragmentIdx);
216-
String fragment = uri.substring(fragmentIdx + 1);
217-
return new URI(scheme, ssp, fragment);
218-
}
219-
}
179+
String encoded = UriUtils.encodeUri(uri, "UTF-8");
180+
return new URI(encoded);
181+
}
182+
catch (UnsupportedEncodingException ex) {
183+
// should not happen, UTF-8 is always supported
184+
throw new IllegalStateException(ex);
220185
}
221186
catch (URISyntaxException ex) {
222187
throw new IllegalArgumentException("Could not create URI from [" + uri + "]: " + ex);

0 commit comments

Comments
 (0)