Skip to content

Commit 6e303d2

Browse files
poutsmapoutsma
committed
SPR-7427 - URL in a redirect is not escaped by RedirectView
1 parent ac1d2d9 commit 6e303d2

File tree

2 files changed

+59
-66
lines changed

2 files changed

+59
-66
lines changed

‎org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/view/RedirectView.java

Lines changed: 26 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2008 the original author or authors.
2+
* Copyright 2002-2010 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -19,7 +19,6 @@
1919
import java.io.IOException;
2020
import java.io.UnsupportedEncodingException;
2121
import java.lang.reflect.Array;
22-
import java.net.URLEncoder;
2322
import java.util.Arrays;
2423
import java.util.Collection;
2524
import java.util.Collections;
@@ -30,10 +29,11 @@
3029
import javax.servlet.http.HttpServletResponse;
3130

3231
import org.springframework.beans.BeanUtils;
32+
import org.springframework.http.HttpStatus;
3333
import org.springframework.util.ObjectUtils;
34-
import org.springframework.web.util.WebUtils;
3534
import org.springframework.web.servlet.View;
36-
import org.springframework.http.HttpStatus;
35+
import org.springframework.web.util.UriUtils;
36+
import org.springframework.web.util.WebUtils;
3737

3838
/**
3939
* <p>View that redirects to an absolute, context relative, or current request
@@ -207,27 +207,36 @@ protected void renderMergedOutputModel(
207207
Map<String, Object> model, HttpServletRequest request, HttpServletResponse response)
208208
throws IOException {
209209

210+
String encoding = getEncoding(request);
211+
210212
// Prepare target URL.
211213
StringBuilder targetUrl = new StringBuilder();
212214
if (this.contextRelative && getUrl().startsWith("/")) {
213215
// Do not apply context path to relative URLs.
214-
targetUrl.append(request.getContextPath());
216+
targetUrl.append(UriUtils.encodePath(request.getContextPath(), encoding));
217+
targetUrl.append(UriUtils.encodePath(getUrl(), encoding));
218+
}
219+
else {
220+
targetUrl.append(UriUtils.encodeUri(getUrl(), encoding));
215221
}
216-
targetUrl.append(getUrl());
217222
if (this.exposeModelAttributes) {
218-
String enc = this.encodingScheme;
219-
if (enc == null) {
220-
enc = request.getCharacterEncoding();
221-
}
222-
if (enc == null) {
223-
enc = WebUtils.DEFAULT_CHARACTER_ENCODING;
224-
}
225-
appendQueryProperties(targetUrl, model, enc);
223+
appendQueryProperties(targetUrl, model, encoding);
226224
}
227225

228226
sendRedirect(request, response, targetUrl.toString(), this.http10Compatible);
229227
}
230228

229+
private String getEncoding(HttpServletRequest request) {
230+
String enc = this.encodingScheme;
231+
if (enc == null) {
232+
enc = request.getCharacterEncoding();
233+
}
234+
if (enc == null) {
235+
enc = WebUtils.DEFAULT_CHARACTER_ENCODING;
236+
}
237+
return enc;
238+
}
239+
231240
/**
232241
* Append query properties to the redirect URL.
233242
* Stringifies, URL-encodes and formats model attributes as query properties.
@@ -271,16 +280,16 @@ else if (rawValue instanceof Collection) {
271280
else {
272281
targetUrl.append('&');
273282
}
274-
String encodedKey = urlEncode(entry.getKey(), encodingScheme);
275-
String encodedValue = (value != null ? urlEncode(value.toString(), encodingScheme) : "");
283+
String encodedKey = UriUtils.encodeQueryParam(entry.getKey(), encodingScheme);
284+
String encodedValue = (value != null ? UriUtils.encodeQueryParam(value.toString(), encodingScheme) : "");
276285
targetUrl.append(encodedKey).append('=').append(encodedValue);
277286
}
278287
}
279288

280289
// Append anchor fragment, if any, to end of URL.
281290
if (fragment != null) {
282291
targetUrl.append(fragment);
283-
}
292+
}
284293
}
285294

286295
/**
@@ -363,20 +372,6 @@ protected boolean isEligibleValue(Object value) {
363372
return (value != null && BeanUtils.isSimpleValueType(value.getClass()));
364373
}
365374

366-
/**
367-
* URL-encode the given input String with the given encoding scheme.
368-
* <p>The default implementation uses <code>URLEncoder.encode(input, enc)</code>.
369-
* @param input the unencoded input String
370-
* @param encodingScheme the encoding scheme
371-
* @return the encoded output String
372-
* @throws UnsupportedEncodingException if thrown by the JDK URLEncoder
373-
* @see java.net.URLEncoder#encode(String, String)
374-
* @see java.net.URLEncoder#encode(String)
375-
*/
376-
protected String urlEncode(String input, String encodingScheme) throws UnsupportedEncodingException {
377-
return (input != null ? URLEncoder.encode(input, encodingScheme) : null);
378-
}
379-
380375
/**
381376
* Send a redirect back to the HTTP client
382377
* @param request current HTTP request (allows for reacting to request method)

‎org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/RedirectViewTests.java

Lines changed: 33 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2008 the original author or authors.
2+
* Copyright 2002-2010 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -18,14 +18,13 @@
1818

1919
import java.util.ArrayList;
2020
import java.util.HashMap;
21-
import java.util.LinkedHashMap;
2221
import java.util.List;
2322
import java.util.Map;
2423
import javax.servlet.http.HttpServletRequest;
2524
import javax.servlet.http.HttpServletResponse;
2625

2726
import junit.framework.AssertionFailedError;
28-
import org.easymock.MockControl;
27+
import static org.easymock.EasyMock.*;
2928
import static org.junit.Assert.*;
3029
import org.junit.Test;
3130

@@ -34,6 +33,7 @@
3433
import org.springframework.mock.web.MockHttpServletRequest;
3534
import org.springframework.mock.web.MockHttpServletResponse;
3635
import org.springframework.web.servlet.View;
36+
import org.springframework.web.util.WebUtils;
3737

3838
/**
3939
* Tests for redirect view, and query string construction.
@@ -60,7 +60,7 @@ public void http11() throws Exception {
6060
rv.setHttp10Compatible(false);
6161
MockHttpServletRequest request = new MockHttpServletRequest();
6262
MockHttpServletResponse response = new MockHttpServletResponse();
63-
rv.render(new HashMap(), request, response);
63+
rv.render(new HashMap<String, Object>(), request, response);
6464
assertEquals(303, response.getStatus());
6565
assertEquals("http://url.somewhere.com", response.getHeader("Location"));
6666
}
@@ -73,7 +73,7 @@ public void explicitStatusCode() throws Exception {
7373
rv.setStatusCode(HttpStatus.CREATED);
7474
MockHttpServletRequest request = new MockHttpServletRequest();
7575
MockHttpServletResponse response = new MockHttpServletResponse();
76-
rv.render(new HashMap(), request, response);
76+
rv.render(new HashMap<String, Object>(), request, response);
7777
assertEquals(201, response.getStatus());
7878
assertEquals("http://url.somewhere.com", response.getHeader("Location"));
7979
}
@@ -86,29 +86,29 @@ public void attributeStatusCode() throws Exception {
8686
MockHttpServletRequest request = new MockHttpServletRequest();
8787
request.setAttribute(View.RESPONSE_STATUS_ATTRIBUTE, HttpStatus.CREATED);
8888
MockHttpServletResponse response = new MockHttpServletResponse();
89-
rv.render(new HashMap(), request, response);
89+
rv.render(new HashMap<String, Object>(), request, response);
9090
assertEquals(201, response.getStatus());
9191
assertEquals("http://url.somewhere.com", response.getHeader("Location"));
9292
}
9393

9494
@Test
9595
public void emptyMap() throws Exception {
9696
String url = "/myUrl";
97-
doTest(new HashMap(), url, false, url);
97+
doTest(new HashMap<String, Object>(), url, false, url);
9898
}
9999

100100
@Test
101101
public void emptyMapWithContextRelative() throws Exception {
102102
String url = "/myUrl";
103-
doTest(new HashMap(), url, true, url);
103+
doTest(new HashMap<String, Object>(), url, true, url);
104104
}
105105

106106
@Test
107107
public void singleParam() throws Exception {
108108
String url = "http://url.somewhere.com";
109109
String key = "foo";
110110
String val = "bar";
111-
Map model = new HashMap();
111+
Map<String, String> model = new HashMap<String, String>();
112112
model.put(key, val);
113113
String expectedUrlForEncoding = url + "?" + key + "=" + val;
114114
doTest(model, url, false, expectedUrlForEncoding);
@@ -119,7 +119,7 @@ public void singleParamWithoutExposingModelAttributes() throws Exception {
119119
String url = "http://url.somewhere.com";
120120
String key = "foo";
121121
String val = "bar";
122-
Map model = new HashMap();
122+
Map<String, String> model = new HashMap<String, String>();
123123
model.put(key, val);
124124
String expectedUrlForEncoding = url; // + "?" + key + "=" + val;
125125
doTest(model, url, false, false, expectedUrlForEncoding);
@@ -130,7 +130,7 @@ public void paramWithAnchor() throws Exception {
130130
String url = "http://url.somewhere.com/test.htm#myAnchor";
131131
String key = "foo";
132132
String val = "bar";
133-
Map model = new HashMap();
133+
Map<String, String> model = new HashMap<String, String>();
134134
model.put(key, val);
135135
String expectedUrlForEncoding = "http://url.somewhere.com/test.htm" + "?" + key + "=" + val + "#myAnchor";
136136
doTest(model, url, false, expectedUrlForEncoding);
@@ -143,7 +143,7 @@ public void twoParams() throws Exception {
143143
String val = "bar";
144144
String key2 = "thisIsKey2";
145145
String val2 = "andThisIsVal2";
146-
Map model = new HashMap();
146+
Map<String, String> model = new HashMap<String, String>();
147147
model.put(key, val);
148148
model.put(key2, val2);
149149
try {
@@ -162,7 +162,7 @@ public void arrayParam() throws Exception {
162162
String url = "http://url.somewhere.com";
163163
String key = "foo";
164164
String[] val = new String[] {"bar", "baz"};
165-
Map model = new HashMap();
165+
Map<String, String[]> model = new HashMap<String, String[]>();
166166
model.put(key, val);
167167
try {
168168
String expectedUrlForEncoding = "http://url.somewhere.com?" + key + "=" + val[0] + "&" + key + "=" + val[1];
@@ -179,10 +179,10 @@ public void arrayParam() throws Exception {
179179
public void collectionParam() throws Exception {
180180
String url = "http://url.somewhere.com";
181181
String key = "foo";
182-
List val = new ArrayList();
182+
List<String> val = new ArrayList<String>();
183183
val.add("bar");
184184
val.add("baz");
185-
Map model = new HashMap();
185+
Map<String, List<String>> model = new HashMap<String, List<String>>();
186186
model.put(key, val);
187187
try {
188188
String expectedUrlForEncoding = "http://url.somewhere.com?" + key + "=" + val.get(0) + "&" + key + "=" + val.get(1);
@@ -202,17 +202,17 @@ public void objectConversion() throws Exception {
202202
String val = "bar";
203203
String key2 = "int2";
204204
Object val2 = new Long(611);
205-
Object key3 = "tb";
205+
String key3 = "tb";
206206
Object val3 = new TestBean();
207-
Map model = new LinkedHashMap();
207+
Map<String, Object> model = new HashMap<String, Object>();
208208
model.put(key, val);
209209
model.put(key2, val2);
210210
model.put(key3, val3);
211211
String expectedUrlForEncoding = "http://url.somewhere.com?" + key + "=" + val + "&" + key2 + "=" + val2;
212212
doTest(model, url, false, expectedUrlForEncoding);
213213
}
214214

215-
private void doTest(Map map, String url, boolean contextRelative, String expectedUrlForEncoding)
215+
private void doTest(Map<String, ?> map, String url, boolean contextRelative, String expectedUrlForEncoding)
216216
throws Exception {
217217
doTest(map, url, contextRelative, true, expectedUrlForEncoding);
218218
}
@@ -227,7 +227,8 @@ class TestRedirectView extends RedirectView {
227227
/**
228228
* Test whether this callback method is called with correct args
229229
*/
230-
protected Map queryProperties(Map model) {
230+
@Override
231+
protected Map<String, Object> queryProperties(Map<String, Object> model) {
231232
// They may not be the same model instance, but they're still equal
232233
assertTrue("Map and model must be equal.", map.equals(model));
233234
this.queryPropertiesCalled = true;
@@ -240,30 +241,27 @@ protected Map queryProperties(Map model) {
240241
rv.setContextRelative(contextRelative);
241242
rv.setExposeModelAttributes(exposeModelAttributes);
242243

243-
MockControl requestControl = MockControl.createControl(HttpServletRequest.class);
244-
HttpServletRequest request = (HttpServletRequest) requestControl.getMock();
245-
request.getCharacterEncoding();
246-
requestControl.setReturnValue(null, 1);
244+
HttpServletRequest request = createNiceMock("request", HttpServletRequest.class);
245+
if (exposeModelAttributes) {
246+
expect(request.getCharacterEncoding()).andReturn(WebUtils.DEFAULT_CHARACTER_ENCODING);
247+
}
247248
if (contextRelative) {
248249
expectedUrlForEncoding = "/context" + expectedUrlForEncoding;
249-
request.getContextPath();
250-
requestControl.setReturnValue("/context");
250+
expect(request.getContextPath()).andReturn("/context");
251251
}
252-
requestControl.replay();
253252

254-
MockControl responseControl = MockControl.createControl(HttpServletResponse.class);
255-
HttpServletResponse resp = (HttpServletResponse) responseControl.getMock();
256-
resp.encodeRedirectURL(expectedUrlForEncoding);
257-
responseControl.setReturnValue(expectedUrlForEncoding);
258-
resp.sendRedirect(expectedUrlForEncoding);
259-
responseControl.setVoidCallable(1);
260-
responseControl.replay();
253+
HttpServletResponse response = createMock("response", HttpServletResponse.class);
254+
expect(response.encodeRedirectURL(expectedUrlForEncoding)).andReturn(expectedUrlForEncoding);
255+
response.sendRedirect(expectedUrlForEncoding);
261256

262-
rv.render(map, request, resp);
257+
replay(request, response);
258+
259+
rv.render(map, request, response);
263260
if (exposeModelAttributes) {
264261
assertTrue("queryProperties() should have been called.", rv.queryPropertiesCalled);
265262
}
266-
responseControl.verify();
263+
264+
verify(request, response);
267265
}
268266

269267
}

0 commit comments

Comments
 (0)