From 402a3786ebb830ad28aa9108982d1dad15398580 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Thu, 2 Sep 2010 10:45:07 +0000 Subject: [PATCH] SWF-1385 Fix for servletRelative redirect where default servlet is used. --- .../mvc/servlet/FlowHandlerAdapter.java | 38 +++++++--- .../mvc/servlet/FlowHandlerAdapterTests.java | 72 +++++++++++++++++++ 2 files changed, 102 insertions(+), 8 deletions(-) diff --git a/spring-webflow/src/main/java/org/springframework/webflow/mvc/servlet/FlowHandlerAdapter.java b/spring-webflow/src/main/java/org/springframework/webflow/mvc/servlet/FlowHandlerAdapter.java index 8975b2a7..9d21f214 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/mvc/servlet/FlowHandlerAdapter.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/mvc/servlet/FlowHandlerAdapter.java @@ -396,13 +396,8 @@ public class FlowHandlerAdapter extends WebContentGenerator implements HandlerAd sendServletRelativeRedirect(location.substring(SERVLET_RELATIVE_LOCATION_PREFIX.length()), request, response); } else if (location.startsWith(CONTEXT_RELATIVE_LOCATION_PREFIX)) { - StringBuffer url = new StringBuffer(request.getContextPath()); - String contextRelativeUrl = location.substring(CONTEXT_RELATIVE_LOCATION_PREFIX.length()); - if (!contextRelativeUrl.startsWith("/")) { - url.append('/'); - } - url.append(contextRelativeUrl); - sendRedirect(url.toString(), request, response); + sendContextRelativeRedirect(location.substring(CONTEXT_RELATIVE_LOCATION_PREFIX.length()), request, + response); } else if (location.startsWith(SERVER_RELATIVE_LOCATION_PREFIX)) { String url = location.substring(SERVER_RELATIVE_LOCATION_PREFIX.length()); if (!url.startsWith("/")) { @@ -412,10 +407,37 @@ public class FlowHandlerAdapter extends WebContentGenerator implements HandlerAd } else if (location.startsWith("http://") || location.startsWith("https://")) { sendRedirect(location, request, response); } else { - sendServletRelativeRedirect(location, request, response); + if (isRedirectServletRelative(request)) { + sendServletRelativeRedirect(location, request, response); + } else { + sendContextRelativeRedirect(location, request, response); + } } } + /** + * Returns true if the servlet path should automatically be prepended to an external redirect URL for which a prefix + * such as "contextRelative: was not specified. This answer depends on how the MVC Dispatcher Servlet is mapped: (1) + * default servlet, (2) prefix, (3) extension, (4) exact match. In (1), (3), and (4) it doesn't make sense to + * prepend the servlet path, which contains the entire URL after the context path. + * + * Because there is no simple way to get the servlet mapping, this method is implemented to return True if path info + * is not null. Also see SWF-1385. + */ + private boolean isRedirectServletRelative(HttpServletRequest request) { + return (request.getPathInfo() != null); + } + + private void sendContextRelativeRedirect(String location, HttpServletRequest request, HttpServletResponse response) + throws IOException { + StringBuffer url = new StringBuffer(request.getContextPath()); + if (!location.startsWith("/")) { + url.append('/'); + } + url.append(location); + sendRedirect(url.toString(), request, response); + } + private void sendServletRelativeRedirect(String location, HttpServletRequest request, HttpServletResponse response) throws IOException { StringBuffer url = new StringBuffer(request.getContextPath()); diff --git a/spring-webflow/src/test/java/org/springframework/webflow/mvc/servlet/FlowHandlerAdapterTests.java b/spring-webflow/src/test/java/org/springframework/webflow/mvc/servlet/FlowHandlerAdapterTests.java index e6b545cf..a2888c2d 100644 --- a/spring-webflow/src/test/java/org/springframework/webflow/mvc/servlet/FlowHandlerAdapterTests.java +++ b/spring-webflow/src/test/java/org/springframework/webflow/mvc/servlet/FlowHandlerAdapterTests.java @@ -395,6 +395,78 @@ public class FlowHandlerAdapterTests extends TestCase { EasyMock.verify(new Object[] { flowExecutor }); } + public void testSwf1385DefaultServletExternalRedirect() throws Exception { + // The default case in accordance with the servlet spec: + // "A string containing only the ’/’ character indicates the "default" servlet of the application. + // In this case the servlet path is the request URI minus the context path and the path info is null." + request.setContextPath("/springtravel"); + request.setServletPath("/foo"); + request.setPathInfo(null); + request.setRequestURI("/springtravel/foo"); + request.setMethod("GET"); + context.requestExternalRedirect("/bar"); + flowExecutor.launchExecution("foo", flowInput, context); + FlowExecutionResult result = FlowExecutionResult.createPausedResult("foo", "12345"); + EasyMock.expectLastCall().andReturn(result); + EasyMock.replay(new Object[] { flowExecutor }); + flowHandlerAdapter.handle(request, response, flowHandler); + EasyMock.verify(new Object[] { flowExecutor }); + assertEquals("/springtravel/bar", response.getRedirectedUrl()); + EasyMock.verify(new Object[] { flowExecutor }); + } + + public void testSwf1385DefaultServletExternalRedirectDeviation() throws Exception { + // Deviation from the default case: + // In some containers the default behavior can be switched so that the contents of the URI after + // the context path is in the path info while the servlet path is empty. + request.setContextPath("/springtravel"); + request.setServletPath(""); + request.setPathInfo("/foo"); + request.setRequestURI("/springtravel/foo"); + request.setMethod("GET"); + context.requestExternalRedirect("/bar"); + flowExecutor.launchExecution("foo", flowInput, context); + FlowExecutionResult result = FlowExecutionResult.createPausedResult("foo", "12345"); + EasyMock.expectLastCall().andReturn(result); + EasyMock.replay(new Object[] { flowExecutor }); + flowHandlerAdapter.handle(request, response, flowHandler); + EasyMock.verify(new Object[] { flowExecutor }); + assertEquals("/springtravel/bar", response.getRedirectedUrl()); + EasyMock.verify(new Object[] { flowExecutor }); + } + + public void testSwf1385DefaultServletExternalRedirectServletRelative() throws Exception { + request.setContextPath("/springtravel"); + request.setServletPath("/foo"); + request.setRequestURI("/springtravel/foo"); + request.setMethod("GET"); + context.requestExternalRedirect("/bar"); + flowExecutor.launchExecution("foo", flowInput, context); + FlowExecutionResult result = FlowExecutionResult.createPausedResult("foo", "12345"); + EasyMock.expectLastCall().andReturn(result); + EasyMock.replay(new Object[] { flowExecutor }); + flowHandlerAdapter.handle(request, response, flowHandler); + EasyMock.verify(new Object[] { flowExecutor }); + assertEquals("/springtravel/bar", response.getRedirectedUrl()); + EasyMock.verify(new Object[] { flowExecutor }); + } + + public void testExternalRedirectServletRelativeWithDefaultServletMapping() throws Exception { + request.setContextPath("/springtravel"); + request.setServletPath("/foo"); + request.setRequestURI("/springtravel/foo"); + request.setMethod("GET"); + context.requestExternalRedirect("servletRelative:bar"); + flowExecutor.launchExecution("foo", flowInput, context); + FlowExecutionResult result = FlowExecutionResult.createPausedResult("foo", "12345"); + EasyMock.expectLastCall().andReturn(result); + EasyMock.replay(new Object[] { flowExecutor }); + flowHandlerAdapter.handle(request, response, flowHandler); + EasyMock.verify(new Object[] { flowExecutor }); + assertEquals("/springtravel/foo/bar", response.getRedirectedUrl()); + EasyMock.verify(new Object[] { flowExecutor }); + } + public void testDefaultHandleFlowException() throws Exception { request.setContextPath("/springtravel"); request.setServletPath("/app");