From 6e303d25c4e49f05096b8abac64c6bddc127b39c Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Wed, 11 Aug 2010 11:44:44 +0000 Subject: [PATCH] SPR-7427 - URL in a redirect is not escaped by RedirectView --- .../web/servlet/view/RedirectView.java | 59 ++++++++-------- .../web/servlet/view/RedirectViewTests.java | 68 +++++++++---------- 2 files changed, 60 insertions(+), 67 deletions(-) diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/view/RedirectView.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/view/RedirectView.java index a8be1b1419c..4233461b8ac 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/view/RedirectView.java +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/view/RedirectView.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2008 the original author or authors. + * Copyright 2002-2010 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,7 +19,6 @@ package org.springframework.web.servlet.view; import java.io.IOException; import java.io.UnsupportedEncodingException; import java.lang.reflect.Array; -import java.net.URLEncoder; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -30,10 +29,11 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.springframework.beans.BeanUtils; -import org.springframework.util.ObjectUtils; -import org.springframework.web.util.WebUtils; -import org.springframework.web.servlet.View; import org.springframework.http.HttpStatus; +import org.springframework.util.ObjectUtils; +import org.springframework.web.servlet.View; +import org.springframework.web.util.UriUtils; +import org.springframework.web.util.WebUtils; /** *

View that redirects to an absolute, context relative, or current request @@ -207,27 +207,36 @@ public class RedirectView extends AbstractUrlBasedView { Map model, HttpServletRequest request, HttpServletResponse response) throws IOException { + String encoding = getEncoding(request); + // Prepare target URL. StringBuilder targetUrl = new StringBuilder(); if (this.contextRelative && getUrl().startsWith("/")) { // Do not apply context path to relative URLs. - targetUrl.append(request.getContextPath()); + targetUrl.append(UriUtils.encodePath(request.getContextPath(), encoding)); + targetUrl.append(UriUtils.encodePath(getUrl(), encoding)); + } + else { + targetUrl.append(UriUtils.encodeUri(getUrl(), encoding)); } - targetUrl.append(getUrl()); if (this.exposeModelAttributes) { - String enc = this.encodingScheme; - if (enc == null) { - enc = request.getCharacterEncoding(); - } - if (enc == null) { - enc = WebUtils.DEFAULT_CHARACTER_ENCODING; - } - appendQueryProperties(targetUrl, model, enc); + appendQueryProperties(targetUrl, model, encoding); } sendRedirect(request, response, targetUrl.toString(), this.http10Compatible); } + private String getEncoding(HttpServletRequest request) { + String enc = this.encodingScheme; + if (enc == null) { + enc = request.getCharacterEncoding(); + } + if (enc == null) { + enc = WebUtils.DEFAULT_CHARACTER_ENCODING; + } + return enc; + } + /** * Append query properties to the redirect URL. * Stringifies, URL-encodes and formats model attributes as query properties. @@ -271,8 +280,8 @@ public class RedirectView extends AbstractUrlBasedView { else { targetUrl.append('&'); } - String encodedKey = urlEncode(entry.getKey(), encodingScheme); - String encodedValue = (value != null ? urlEncode(value.toString(), encodingScheme) : ""); + String encodedKey = UriUtils.encodeQueryParam(entry.getKey(), encodingScheme); + String encodedValue = (value != null ? UriUtils.encodeQueryParam(value.toString(), encodingScheme) : ""); targetUrl.append(encodedKey).append('=').append(encodedValue); } } @@ -280,7 +289,7 @@ public class RedirectView extends AbstractUrlBasedView { // Append anchor fragment, if any, to end of URL. if (fragment != null) { targetUrl.append(fragment); - } + } } /** @@ -363,20 +372,6 @@ public class RedirectView extends AbstractUrlBasedView { return (value != null && BeanUtils.isSimpleValueType(value.getClass())); } - /** - * URL-encode the given input String with the given encoding scheme. - *

The default implementation uses URLEncoder.encode(input, enc). - * @param input the unencoded input String - * @param encodingScheme the encoding scheme - * @return the encoded output String - * @throws UnsupportedEncodingException if thrown by the JDK URLEncoder - * @see java.net.URLEncoder#encode(String, String) - * @see java.net.URLEncoder#encode(String) - */ - protected String urlEncode(String input, String encodingScheme) throws UnsupportedEncodingException { - return (input != null ? URLEncoder.encode(input, encodingScheme) : null); - } - /** * Send a redirect back to the HTTP client * @param request current HTTP request (allows for reacting to request method) diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/RedirectViewTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/RedirectViewTests.java index af79dc521c1..1a33a01ef16 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/RedirectViewTests.java +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/RedirectViewTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2008 the original author or authors. + * Copyright 2002-2010 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,14 +18,13 @@ package org.springframework.web.servlet.view; import java.util.ArrayList; import java.util.HashMap; -import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import junit.framework.AssertionFailedError; -import org.easymock.MockControl; +import static org.easymock.EasyMock.*; import static org.junit.Assert.*; import org.junit.Test; @@ -34,6 +33,7 @@ import org.springframework.http.HttpStatus; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.web.servlet.View; +import org.springframework.web.util.WebUtils; /** * Tests for redirect view, and query string construction. @@ -60,7 +60,7 @@ public class RedirectViewTests { rv.setHttp10Compatible(false); MockHttpServletRequest request = new MockHttpServletRequest(); MockHttpServletResponse response = new MockHttpServletResponse(); - rv.render(new HashMap(), request, response); + rv.render(new HashMap(), request, response); assertEquals(303, response.getStatus()); assertEquals("http://url.somewhere.com", response.getHeader("Location")); } @@ -73,7 +73,7 @@ public class RedirectViewTests { rv.setStatusCode(HttpStatus.CREATED); MockHttpServletRequest request = new MockHttpServletRequest(); MockHttpServletResponse response = new MockHttpServletResponse(); - rv.render(new HashMap(), request, response); + rv.render(new HashMap(), request, response); assertEquals(201, response.getStatus()); assertEquals("http://url.somewhere.com", response.getHeader("Location")); } @@ -86,7 +86,7 @@ public class RedirectViewTests { MockHttpServletRequest request = new MockHttpServletRequest(); request.setAttribute(View.RESPONSE_STATUS_ATTRIBUTE, HttpStatus.CREATED); MockHttpServletResponse response = new MockHttpServletResponse(); - rv.render(new HashMap(), request, response); + rv.render(new HashMap(), request, response); assertEquals(201, response.getStatus()); assertEquals("http://url.somewhere.com", response.getHeader("Location")); } @@ -94,13 +94,13 @@ public class RedirectViewTests { @Test public void emptyMap() throws Exception { String url = "/myUrl"; - doTest(new HashMap(), url, false, url); + doTest(new HashMap(), url, false, url); } @Test public void emptyMapWithContextRelative() throws Exception { String url = "/myUrl"; - doTest(new HashMap(), url, true, url); + doTest(new HashMap(), url, true, url); } @Test @@ -108,7 +108,7 @@ public class RedirectViewTests { String url = "http://url.somewhere.com"; String key = "foo"; String val = "bar"; - Map model = new HashMap(); + Map model = new HashMap(); model.put(key, val); String expectedUrlForEncoding = url + "?" + key + "=" + val; doTest(model, url, false, expectedUrlForEncoding); @@ -119,7 +119,7 @@ public class RedirectViewTests { String url = "http://url.somewhere.com"; String key = "foo"; String val = "bar"; - Map model = new HashMap(); + Map model = new HashMap(); model.put(key, val); String expectedUrlForEncoding = url; // + "?" + key + "=" + val; doTest(model, url, false, false, expectedUrlForEncoding); @@ -130,7 +130,7 @@ public class RedirectViewTests { String url = "http://url.somewhere.com/test.htm#myAnchor"; String key = "foo"; String val = "bar"; - Map model = new HashMap(); + Map model = new HashMap(); model.put(key, val); String expectedUrlForEncoding = "http://url.somewhere.com/test.htm" + "?" + key + "=" + val + "#myAnchor"; doTest(model, url, false, expectedUrlForEncoding); @@ -143,7 +143,7 @@ public class RedirectViewTests { String val = "bar"; String key2 = "thisIsKey2"; String val2 = "andThisIsVal2"; - Map model = new HashMap(); + Map model = new HashMap(); model.put(key, val); model.put(key2, val2); try { @@ -162,7 +162,7 @@ public class RedirectViewTests { String url = "http://url.somewhere.com"; String key = "foo"; String[] val = new String[] {"bar", "baz"}; - Map model = new HashMap(); + Map model = new HashMap(); model.put(key, val); try { String expectedUrlForEncoding = "http://url.somewhere.com?" + key + "=" + val[0] + "&" + key + "=" + val[1]; @@ -179,10 +179,10 @@ public class RedirectViewTests { public void collectionParam() throws Exception { String url = "http://url.somewhere.com"; String key = "foo"; - List val = new ArrayList(); + List val = new ArrayList(); val.add("bar"); val.add("baz"); - Map model = new HashMap(); + Map> model = new HashMap>(); model.put(key, val); try { String expectedUrlForEncoding = "http://url.somewhere.com?" + key + "=" + val.get(0) + "&" + key + "=" + val.get(1); @@ -202,9 +202,9 @@ public class RedirectViewTests { String val = "bar"; String key2 = "int2"; Object val2 = new Long(611); - Object key3 = "tb"; + String key3 = "tb"; Object val3 = new TestBean(); - Map model = new LinkedHashMap(); + Map model = new HashMap(); model.put(key, val); model.put(key2, val2); model.put(key3, val3); @@ -212,7 +212,7 @@ public class RedirectViewTests { doTest(model, url, false, expectedUrlForEncoding); } - private void doTest(Map map, String url, boolean contextRelative, String expectedUrlForEncoding) + private void doTest(Map map, String url, boolean contextRelative, String expectedUrlForEncoding) throws Exception { doTest(map, url, contextRelative, true, expectedUrlForEncoding); } @@ -227,7 +227,8 @@ public class RedirectViewTests { /** * Test whether this callback method is called with correct args */ - protected Map queryProperties(Map model) { + @Override + protected Map queryProperties(Map model) { // They may not be the same model instance, but they're still equal assertTrue("Map and model must be equal.", map.equals(model)); this.queryPropertiesCalled = true; @@ -240,30 +241,27 @@ public class RedirectViewTests { rv.setContextRelative(contextRelative); rv.setExposeModelAttributes(exposeModelAttributes); - MockControl requestControl = MockControl.createControl(HttpServletRequest.class); - HttpServletRequest request = (HttpServletRequest) requestControl.getMock(); - request.getCharacterEncoding(); - requestControl.setReturnValue(null, 1); + HttpServletRequest request = createNiceMock("request", HttpServletRequest.class); + if (exposeModelAttributes) { + expect(request.getCharacterEncoding()).andReturn(WebUtils.DEFAULT_CHARACTER_ENCODING); + } if (contextRelative) { expectedUrlForEncoding = "/context" + expectedUrlForEncoding; - request.getContextPath(); - requestControl.setReturnValue("/context"); + expect(request.getContextPath()).andReturn("/context"); } - requestControl.replay(); - MockControl responseControl = MockControl.createControl(HttpServletResponse.class); - HttpServletResponse resp = (HttpServletResponse) responseControl.getMock(); - resp.encodeRedirectURL(expectedUrlForEncoding); - responseControl.setReturnValue(expectedUrlForEncoding); - resp.sendRedirect(expectedUrlForEncoding); - responseControl.setVoidCallable(1); - responseControl.replay(); + HttpServletResponse response = createMock("response", HttpServletResponse.class); + expect(response.encodeRedirectURL(expectedUrlForEncoding)).andReturn(expectedUrlForEncoding); + response.sendRedirect(expectedUrlForEncoding); - rv.render(map, request, resp); + replay(request, response); + + rv.render(map, request, response); if (exposeModelAttributes) { assertTrue("queryProperties() should have been called.", rv.queryPropertiesCalled); } - responseControl.verify(); + + verify(request, response); } }