This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch WW-5842-container-servlet-path in repository https://gitbox.apache.org/repos/asf/struts.git
commit a93972589b05012293faa1c634728530104b0328 Author: Lukasz Lenart <[email protected]> AuthorDate: Sat Nov 7 11:39:46 2026 +0100 WW-5842 fix(core)!: map actions from the servlet path and path info the container reports RequestUtils.getServletPath returned a path rebuilt from the raw request URI whenever the container's servlet path did not end the URI (WW-2268) or was empty, so the mapper worked on a second interpretation of the request that could differ from the container's. It now returns the servlet path followed by the path info, both as reported by the container, and getUri no longer falls back to the raw request URI. With the container stripping path parameters before computing those values, the mappers no longer need to handle ';' themselves: DefaultActionMapper drops its cut at the first ';', RestActionMapper drops the ';jsessionid' handling (including the WW-4585 '!method;' form) and the ';edit' URL form, and ExecuteOperations drops its path-info fallback for an empty servlet path. RestActionMapper.setEditMethodName has no effect any more and is deprecated for removal in 8.0.0 (WW-5843); RestWorkflowInterceptor keeps reading struts.mapper.editMethodName. Test fixtures that set only the request URI now also set the servlet path, as a container would; the JUnit plugin helpers do the same for the mock request they build. BREAKING CHANGE: the REST plugin no longer recognises the ';edit' URL form; use the documented '/edit' form. Tests driving Struts with a mock request must set the servlet path. Co-Authored-By: Claude Mythos 5.1 <[email protected]> --- .../main/java/org/apache/struts2/RequestUtils.java | 42 ++----------- .../struts2/dispatcher/ExecuteOperations.java | 4 -- .../dispatcher/mapper/DefaultActionMapper.java | 3 - .../apache/struts2/views/util/ResourceUtil.java | 2 +- .../java/org/apache/struts2/RequestUtilsTest.java | 70 ++++++++++++---------- .../dispatcher/ActionExtensionEndToEndTest.java | 1 + ...rutsPrepareAndExecuteFilterIntegrationTest.java | 7 +++ .../dispatcher/TwoFilterIntegrationTest.java | 4 ++ .../dispatcher/mapper/DefaultActionMapperTest.java | 16 +++-- .../struts2/result/ServletRedirectResultTest.java | 4 +- .../freemarker/FreemarkerResultMockedTest.java | 8 +++ .../struts2/views/util/ResourceUtilTest.java | 4 +- .../convention/ConventionHttpMethodTest.java | 1 + .../apache/struts2/junit/StrutsJUnit4TestCase.java | 3 + .../apache/struts2/junit/StrutsRestTestCase.java | 2 + .../org/apache/struts2/junit/StrutsTestCase.java | 3 + .../org/apache/struts2/rest/RestActionMapper.java | 34 +++-------- .../apache/struts2/rest/RestActionMapperTest.java | 20 +++---- 18 files changed, 106 insertions(+), 122 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/RequestUtils.java b/core/src/main/java/org/apache/struts2/RequestUtils.java index 651a0a27f..9356fc3f7 100644 --- a/core/src/main/java/org/apache/struts2/RequestUtils.java +++ b/core/src/main/java/org/apache/struts2/RequestUtils.java @@ -50,41 +50,15 @@ public class RequestUtils { }; /** - * Retrieves the current request servlet path. - * Deals with differences between servlet specs (2.2 vs 2.3+) - * - * <p>Note: the fallback branch extracts from the raw {@code requestURI}, which may - * retain percent-encoded characters. Callers must not apply additional URL decoding - * to the returned value because the servlet container has already performed decoding - * where applicable.</p> + * Retrieves the path of the current request below the context path, as the servlet container + * reports it: the servlet path followed by the path info. Path parameters such as + * {@code ;jsessionid=...} never appear in it, the container strips them before computing both values. * * @param request the request * @return the servlet path */ public static String getServletPath(HttpServletRequest request) { - String servletPath = request.getServletPath(); - - String requestUri = request.getRequestURI(); - // Detecting other characters that the servlet container cut off (like anything after ';') - if (requestUri != null && servletPath != null && !requestUri.endsWith(servletPath)) { - int pos = requestUri.indexOf(servletPath); - if (pos > -1) { - servletPath = requestUri.substring(requestUri.indexOf(servletPath)); - } - } - - if (StringUtils.isNotEmpty(servletPath)) { - return servletPath; - } - - int startIndex = request.getContextPath().isEmpty() ? 0 : request.getContextPath().length(); - int endIndex = request.getPathInfo() == null ? requestUri.length() : requestUri.lastIndexOf(request.getPathInfo()); - - if (startIndex > endIndex) { // this should not happen - endIndex = startIndex; - } - - return requestUri.substring(startIndex, endIndex); + return StringUtils.defaultString(request.getServletPath()) + StringUtils.defaultString(request.getPathInfo()); } /** @@ -100,13 +74,7 @@ public class RequestUtils { return uri; } - uri = getServletPath(request); - if (StringUtils.isNotEmpty(uri)) { - return uri; - } - - uri = request.getRequestURI(); - return uri.substring(request.getContextPath().length()); + return getServletPath(request); } /** diff --git a/core/src/main/java/org/apache/struts2/dispatcher/ExecuteOperations.java b/core/src/main/java/org/apache/struts2/dispatcher/ExecuteOperations.java index 9fd0f6e7e..2f1eb1b2d 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/ExecuteOperations.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/ExecuteOperations.java @@ -50,10 +50,6 @@ public class ExecuteOperations { // there is no action in this request, should we look for a static resource? String resourcePath = RequestUtils.getServletPath(request); - if ("".equals(resourcePath) && null != request.getPathInfo()) { - resourcePath = request.getPathInfo(); - } - StaticContentLoader staticResourceLoader = dispatcher.getStaticContentLoader(); if (staticResourceLoader.canHandle(resourcePath)) { staticResourceLoader.findStaticResource(resourcePath, request, response); diff --git a/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java b/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java index 2d46913eb..b5d5f4c0a 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java @@ -283,9 +283,6 @@ public class DefaultActionMapper implements ActionMapper { ActionMapping mapping = new ActionMapping(); String uri = RequestUtils.getUri(request); - int indexOfSemicolon = uri.indexOf(';'); - uri = (indexOfSemicolon > -1) ? uri.substring(0, indexOfSemicolon) : uri; - String declaredUri = stripDeclaredExtension(uri, mapping, configManager); uri = declaredUri != null ? declaredUri : dropExtension(uri, mapping); if (uri == null) { diff --git a/core/src/main/java/org/apache/struts2/views/util/ResourceUtil.java b/core/src/main/java/org/apache/struts2/views/util/ResourceUtil.java index 27c72a87c..27f93f4b2 100644 --- a/core/src/main/java/org/apache/struts2/views/util/ResourceUtil.java +++ b/core/src/main/java/org/apache/struts2/views/util/ResourceUtil.java @@ -24,7 +24,7 @@ import org.apache.struts2.RequestUtils; public class ResourceUtil { public static String getResourceBase(HttpServletRequest req) { String path = RequestUtils.getServletPath(req); - if (path == null || path.isEmpty()) { + if (path.isEmpty()) { return ""; } diff --git a/core/src/test/java/org/apache/struts2/RequestUtilsTest.java b/core/src/test/java/org/apache/struts2/RequestUtilsTest.java index d51a55d06..88158e736 100644 --- a/core/src/test/java/org/apache/struts2/RequestUtilsTest.java +++ b/core/src/test/java/org/apache/struts2/RequestUtilsTest.java @@ -25,6 +25,7 @@ package org.apache.struts2; import junit.framework.TestCase; import static org.easymock.EasyMock.*; +import jakarta.servlet.RequestDispatcher; import jakarta.servlet.http.HttpServletRequest; import java.util.Date; @@ -32,54 +33,61 @@ public class RequestUtilsTest extends TestCase { private HttpServletRequest requestMock; - public void testGetServletPathWithServletPathSet() throws Exception { + public void testServletPathWithoutPathInfo() { expect(requestMock.getServletPath()).andStubReturn("/mycontext/"); - expect(requestMock.getRequestURI()).andStubReturn("/mycontext/"); + expect(requestMock.getPathInfo()).andStubReturn(null); replay(requestMock); assertEquals("/mycontext/", RequestUtils.getServletPath(requestMock)); - verify(requestMock); } - public void testGetServletPathWithRequestURIAndEmptyContextPath() throws Exception { - expect(requestMock.getServletPath()).andStubReturn(null); - expect(requestMock.getRequestURI()).andStubReturn("/mycontext/test.jsp"); - expect(requestMock.getContextPath()).andStubReturn(""); - expect(requestMock.getPathInfo()).andStubReturn("test.jsp"); - expect(requestMock.getPathInfo()).andStubReturn("test.jsp"); + public void testServletPathFollowedByPathInfoForPrefixMapping() { + expect(requestMock.getServletPath()).andStubReturn("/app"); + expect(requestMock.getPathInfo()).andStubReturn("/admin/index.action"); replay(requestMock); - assertEquals("/mycontext/", RequestUtils.getServletPath(requestMock)); - verify(requestMock); + assertEquals("/app/admin/index.action", RequestUtils.getServletPath(requestMock)); } - public void testGetServletPathWithRequestURIAndContextPathSet() throws Exception { - expect(requestMock.getServletPath()).andStubReturn(null); - expect(requestMock.getRequestURI()).andStubReturn("/servlet/mycontext/test.jsp"); - expect(requestMock.getContextPath()).andStubReturn("/servlet"); - expect(requestMock.getContextPath()).andStubReturn("/servlet"); - expect(requestMock.getPathInfo()).andStubReturn("test.jsp"); - expect(requestMock.getPathInfo()).andStubReturn("test.jsp"); + public void testPathInfoAloneWhenServletPathIsEmpty() { + expect(requestMock.getServletPath()).andStubReturn(""); + expect(requestMock.getPathInfo()).andStubReturn("/admin/index.action"); replay(requestMock); - assertEquals("/mycontext/", RequestUtils.getServletPath(requestMock)); - verify(requestMock); + assertEquals("/admin/index.action", RequestUtils.getServletPath(requestMock)); } - public void testGetServletPathWithRequestURIAndContextPathSetButNoPatchInfo() throws Exception { + public void testEmptyPathWhenContainerReportsNeither() { expect(requestMock.getServletPath()).andStubReturn(null); - expect(requestMock.getRequestURI()).andStubReturn("/servlet/mycontext/"); - expect(requestMock.getContextPath()).andStubReturn("/servlet"); - expect(requestMock.getContextPath()).andStubReturn("/servlet"); expect(requestMock.getPathInfo()).andStubReturn(null); replay(requestMock); - assertEquals("/mycontext/", RequestUtils.getServletPath(requestMock)); - verify(requestMock); + assertEquals("", RequestUtils.getServletPath(requestMock)); } - - public void testGetServletPathWithSemicolon() throws Exception { - expect(requestMock.getRequestURI()).andStubReturn("/friend/mycontext/jim;bob"); + + public void testRequestUriIsNeverConsulted() { expect(requestMock.getServletPath()).andStubReturn("/mycontext/jim"); + expect(requestMock.getPathInfo()).andStubReturn(null); + replay(requestMock); + assertEquals("/mycontext/jim", RequestUtils.getServletPath(requestMock)); + } + + public void testUriPrefersIncludedServletPath() { + expect(requestMock.getAttribute(RequestDispatcher.INCLUDE_SERVLET_PATH)).andStubReturn("/included.action"); + replay(requestMock); + assertEquals("/included.action", RequestUtils.getUri(requestMock)); + } + + public void testUriIsTheContainerPath() { + expect(requestMock.getAttribute(RequestDispatcher.INCLUDE_SERVLET_PATH)).andStubReturn(null); + expect(requestMock.getServletPath()).andStubReturn(""); + expect(requestMock.getPathInfo()).andStubReturn("/admin/index.action"); + replay(requestMock); + assertEquals("/admin/index.action", RequestUtils.getUri(requestMock)); + } + + public void testUriIsEmptyWhenContainerReportsNoPath() { + expect(requestMock.getAttribute(RequestDispatcher.INCLUDE_SERVLET_PATH)).andStubReturn(null); + expect(requestMock.getServletPath()).andStubReturn(""); + expect(requestMock.getPathInfo()).andStubReturn(null); replay(requestMock); - assertEquals("/mycontext/jim;bob", RequestUtils.getServletPath(requestMock)); - verify(requestMock); + assertEquals("", RequestUtils.getUri(requestMock)); } public void testParseRFC1123() { diff --git a/core/src/test/java/org/apache/struts2/dispatcher/ActionExtensionEndToEndTest.java b/core/src/test/java/org/apache/struts2/dispatcher/ActionExtensionEndToEndTest.java index d5c64479a..a145cae2b 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/ActionExtensionEndToEndTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/ActionExtensionEndToEndTest.java @@ -161,6 +161,7 @@ public class ActionExtensionEndToEndTest extends StrutsInternalTestCase { private void doFilter(String uri) throws ServletException, IOException { request = new MockHttpServletRequest(servletContext); request.setRequestURI(uri); + request.setServletPath(uri); response = new MockHttpServletResponse(); filterChain = new MockFilterChain(); filter.doFilter(request, response, filterChain); diff --git a/core/src/test/java/org/apache/struts2/dispatcher/StrutsPrepareAndExecuteFilterIntegrationTest.java b/core/src/test/java/org/apache/struts2/dispatcher/StrutsPrepareAndExecuteFilterIntegrationTest.java index 1aa022714..42093bcde 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/StrutsPrepareAndExecuteFilterIntegrationTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/StrutsPrepareAndExecuteFilterIntegrationTest.java @@ -55,6 +55,7 @@ public class StrutsPrepareAndExecuteFilterIntegrationTest { }; request.setRequestURI("/foo.action"); + request.setServletPath("/foo.action"); StrutsPrepareAndExecuteFilter filter = new StrutsPrepareAndExecuteFilter(); filter.init(filterConfig); filter.doFilter(request, response, filterChain); @@ -76,6 +77,7 @@ public class StrutsPrepareAndExecuteFilterIntegrationTest { }; request.setRequestURI("/hello.action"); + request.setServletPath("/hello.action"); StrutsPrepareAndExecuteFilter filter = new StrutsPrepareAndExecuteFilter(); filter.init(filterConfig); filter.doFilter(request, response, filterChain); @@ -97,6 +99,7 @@ public class StrutsPrepareAndExecuteFilterIntegrationTest { }; request.setRequestURI("/hello.action"); + request.setServletPath("/hello.action"); StrutsPrepareAndExecuteFilter filter = new StrutsPrepareAndExecuteFilter(); filter.init(filterConfig); filter.doFilter(request, response, filterChain); @@ -112,6 +115,7 @@ public class StrutsPrepareAndExecuteFilterIntegrationTest { } }; request.setRequestURI("hello.jsp"); + request.setServletPath("hello.jsp"); filter.doFilter(request, response, filterChain2); assertEquals(200, response.getStatus()); assertNull(ActionContext.getContext()); @@ -133,6 +137,7 @@ public class StrutsPrepareAndExecuteFilterIntegrationTest { }; request.setRequestURI("/hello.action"); + request.setServletPath("/hello.action"); StrutsPrepareAndExecuteFilter filter = new StrutsPrepareAndExecuteFilter(); filter.init(filterConfig); filter.doFilter(request, response, filterChain); @@ -159,6 +164,7 @@ public class StrutsPrepareAndExecuteFilterIntegrationTest { }; request.setRequestURI("/foo.txt"); + request.setServletPath("/foo.txt"); StrutsPrepareAndExecuteFilter filter = new StrutsPrepareAndExecuteFilter(); filter.init(filterConfig); filter.doFilter(request, response, filterChain); @@ -181,6 +187,7 @@ public class StrutsPrepareAndExecuteFilterIntegrationTest { }; request.setRequestURI("/static/utils.js"); + request.setServletPath("/static/utils.js"); StrutsPrepareAndExecuteFilter filter = new StrutsPrepareAndExecuteFilter(); filter.init(filterConfig); filter.doFilter(request, response, filterChain); diff --git a/core/src/test/java/org/apache/struts2/dispatcher/TwoFilterIntegrationTest.java b/core/src/test/java/org/apache/struts2/dispatcher/TwoFilterIntegrationTest.java index a824cd787..33a4bbde8 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/TwoFilterIntegrationTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/TwoFilterIntegrationTest.java @@ -129,9 +129,12 @@ public class TwoFilterIntegrationTest { Filter dummyForward = newFilter((req, res, chain) -> { MockHttpServletRequest castReq = (MockHttpServletRequest) req; String oldUri = castReq.getRequestURI(); + String oldServletPath = castReq.getServletPath(); castReq.setRequestURI("/hello.action"); + castReq.setServletPath("/hello.action"); chain.doFilter(castReq, res); castReq.setRequestURI(oldUri); + castReq.setServletPath(oldServletPath); }); MockHttpServletResponse response = run( "/excluded/hello.action", @@ -169,6 +172,7 @@ public class TwoFilterIntegrationTest { }; filterInitParams.forEach(filterConfig::addInitParameter); request.setRequestURI(uri); + request.setServletPath(uri); for (Filter filter : filters) { filter.init(filterConfig); } diff --git a/core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.java b/core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.java index 10aa6ad2f..b1e315203 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.java @@ -284,18 +284,26 @@ public class DefaultActionMapperTest extends StrutsInternalTestCase { } @SuppressWarnings("rawtypes") - public void testGetUriWithSemicolonPresent() { + public void testGetUriWithPathParametersStrippedByTheContainer() { req.setParameters(new HashMap()); req.setRequestURI("/my/namespace/actionName.action;abc=123rty56"); - req.setServletPath("/my/namespace/actionName.action;abc=123rty56"); - - + req.setServletPath("/my/namespace/actionName.action"); DefaultActionMapper mapper = new DefaultActionMapper(); ActionMapping mapping = mapper.getMapping(req, configManager); assertEquals("/my/namespace/actionName.action", mapper.getUriFromActionMapping(mapping)); } + @SuppressWarnings("rawtypes") + public void testSemicolonInTheServletPathIsNotSpecial() { + req.setParameters(new HashMap()); + req.setRequestURI("/my/namespace/actionName.action;abc=123rty56"); + req.setServletPath("/my/namespace/actionName.action;abc=123rty56"); + + DefaultActionMapper mapper = new DefaultActionMapper(); + assertNull(mapper.getMapping(req, configManager)); + } + @SuppressWarnings("rawtypes") public void testGetUriWithMethod() { req.setParameters(new HashMap()); diff --git a/core/src/test/java/org/apache/struts2/result/ServletRedirectResultTest.java b/core/src/test/java/org/apache/struts2/result/ServletRedirectResultTest.java index fb276d7e1..5a71910f7 100644 --- a/core/src/test/java/org/apache/struts2/result/ServletRedirectResultTest.java +++ b/core/src/test/java/org/apache/struts2/result/ServletRedirectResultTest.java @@ -207,7 +207,7 @@ public class ServletRedirectResultTest extends StrutsInternalTestCase implements view.setLocation("foo.jsp"); requestMock.expectAndReturn("getParameterMap", new HashMap()); requestMock.expectAndReturn("getServletPath", "/namespace/some.action"); - requestMock.expectAndReturn("getRequestURI", "/namespace/some.action"); + requestMock.expectAndReturn("getPathInfo", null); requestMock.expectAndReturn("getAttribute", C.ANY_ARGS, null); responseMock.expectAndReturn("encodeRedirectURL", "/context/namespace/foo.jsp", "/context/namespace/foo.jsp"); responseMock.expect("sendRedirect", C.args(C.eq("/context/namespace/foo.jsp"))); @@ -222,7 +222,7 @@ public class ServletRedirectResultTest extends StrutsInternalTestCase implements view.setLocation("foo.jsp?foo=bar&baz=jim"); requestMock.expectAndReturn("getParameterMap", new HashMap()); requestMock.expectAndReturn("getServletPath", "/namespace/some.action"); - requestMock.expectAndReturn("getRequestURI", "/namespace/some.action"); + requestMock.expectAndReturn("getPathInfo", null); requestMock.expectAndReturn("getAttribute", C.ANY_ARGS, null); responseMock.expectAndReturn("encodeRedirectURL", "/context/namespace/foo.jsp?foo=bar&baz=jim", "/context/namespace/foo.jsp?foo=bar&baz=jim"); responseMock.expect("sendRedirect", C.args(C.eq("/context/namespace/foo.jsp?foo=bar&baz=jim"))); diff --git a/core/src/test/java/org/apache/struts2/views/freemarker/FreemarkerResultMockedTest.java b/core/src/test/java/org/apache/struts2/views/freemarker/FreemarkerResultMockedTest.java index 70a3493c7..df0d905f2 100644 --- a/core/src/test/java/org/apache/struts2/views/freemarker/FreemarkerResultMockedTest.java +++ b/core/src/test/java/org/apache/struts2/views/freemarker/FreemarkerResultMockedTest.java @@ -68,6 +68,7 @@ public class FreemarkerResultMockedTest extends StrutsInternalTestCase { init(); request.setRequestURI("/tutorial/test2.action"); + request.setServletPath("/tutorial/test2.action"); ActionMapping mapping = container.getInstance(ActionMapper.class).getMapping(request, configurationManager); dispatcher.serviceAction(request, response, mapping); assertEquals("beforenestedafter", stringWriter.toString()); @@ -83,6 +84,7 @@ public class FreemarkerResultMockedTest extends StrutsInternalTestCase { init(); request.setRequestURI("/tutorial/test5.action"); + request.setServletPath("/tutorial/test5.action"); ActionMapping mapping = container.getInstance(ActionMapper.class).getMapping(request, configurationManager); dispatcher.serviceAction(request, response, mapping); assertEquals("beforenestedafter", stringWriter.toString()); @@ -116,6 +118,7 @@ public class FreemarkerResultMockedTest extends StrutsInternalTestCase { init(); request.setRequestURI("/tutorial/test6.action"); + request.setServletPath("/tutorial/test6.action"); ActionMapping mapping = container.getInstance(ActionMapper.class).getMapping(request, configurationManager); dispatcher.serviceAction(request, response, mapping); @@ -159,6 +162,7 @@ public class FreemarkerResultMockedTest extends StrutsInternalTestCase { init(); request.setRequestURI("/tutorial/test7.action"); + request.setServletPath("/tutorial/test7.action"); ActionMapping mapping = container.getInstance(ActionMapper.class).getMapping(request, configurationManager); dispatcher.serviceAction(request, response, mapping); String expected = "<input type=\"radio\" name=\"client\" id=\"client_foo\" value=\"foo\"/><label for=\"client_foo\">foo</label>\n" @@ -181,6 +185,7 @@ public class FreemarkerResultMockedTest extends StrutsInternalTestCase { init(); request.setRequestURI("/tutorial/test8.action"); + request.setServletPath("/tutorial/test8.action"); ActionMapping mapping = container.getInstance(ActionMapper.class).getMapping(request, configurationManager); dispatcher.serviceAction(request, response, mapping); String expected = "<input type=\"text\"autofocus=\"autofocus\"/>"; @@ -225,6 +230,7 @@ public class FreemarkerResultMockedTest extends StrutsInternalTestCase { init(); request.setRequestURI("/tutorial/test9.action"); + request.setServletPath("/tutorial/test9.action"); ActionMapping mapping = container.getInstance(ActionMapper.class).getMapping(request, configurationManager); dispatcher.serviceAction(request, response, mapping); String result = stringWriter.toString(); @@ -267,6 +273,7 @@ public class FreemarkerResultMockedTest extends StrutsInternalTestCase { request.getSession().setAttribute("nonce", "aNonce"); request.setRequestURI("/tutorial/test10.action"); + request.setServletPath("/tutorial/test10.action"); ActionMapping mapping = container.getInstance(ActionMapper.class).getMapping(request, configurationManager); dispatcher.serviceAction(request, response, mapping); @@ -321,6 +328,7 @@ public class FreemarkerResultMockedTest extends StrutsInternalTestCase { }); request.setRequestURI("/tutorial/test11.action"); + request.setServletPath("/tutorial/test11.action"); ActionMapping mapping = container.getInstance(ActionMapper.class).getMapping(request, configurationManager); dispatcher.serviceAction(request, response, mapping); String result = stringWriter.toString(); diff --git a/core/src/test/java/org/apache/struts2/views/util/ResourceUtilTest.java b/core/src/test/java/org/apache/struts2/views/util/ResourceUtilTest.java index 8a60b82b0..c8acb7595 100644 --- a/core/src/test/java/org/apache/struts2/views/util/ResourceUtilTest.java +++ b/core/src/test/java/org/apache/struts2/views/util/ResourceUtilTest.java @@ -33,7 +33,7 @@ public class ResourceUtilTest extends TestCase { public void testGetResourceBase() throws Exception { expect(requestMock.getServletPath()).andReturn("/mycontext/"); - expect(requestMock.getRequestURI()).andReturn("/mycontext/"); + expect(requestMock.getPathInfo()).andReturn(null); replay(requestMock); assertEquals("/mycontext", ResourceUtil.getResourceBase(requestMock)); verify(requestMock); @@ -41,7 +41,7 @@ public class ResourceUtilTest extends TestCase { reset(requestMock); expect(requestMock.getServletPath()).andReturn("/mycontext/test.jsp"); - expect(requestMock.getRequestURI()).andReturn("/mycontext/test.jsp"); + expect(requestMock.getPathInfo()).andReturn(null); replay(requestMock); assertEquals("/mycontext", ResourceUtil.getResourceBase(requestMock)); diff --git a/plugins/convention/src/test/java/org/apache/struts2/convention/ConventionHttpMethodTest.java b/plugins/convention/src/test/java/org/apache/struts2/convention/ConventionHttpMethodTest.java index 9abac90db..15c4d2904 100644 --- a/plugins/convention/src/test/java/org/apache/struts2/convention/ConventionHttpMethodTest.java +++ b/plugins/convention/src/test/java/org/apache/struts2/convention/ConventionHttpMethodTest.java @@ -129,6 +129,7 @@ public class ConventionHttpMethodTest extends StrutsJUnit4TestCase<Object> { private void dispatch(String httpMethod, String uri) throws Exception { request.setMethod(httpMethod); request.setRequestURI(uri); + request.setServletPath(uri); ActionMapping mapping = getActionMapping(request); assertNotNull(mapping); Dispatcher.getInstance().serviceAction(request, response, mapping); diff --git a/plugins/junit/src/main/java/org/apache/struts2/junit/StrutsJUnit4TestCase.java b/plugins/junit/src/main/java/org/apache/struts2/junit/StrutsJUnit4TestCase.java index c67ba77ed..d14988ae8 100644 --- a/plugins/junit/src/main/java/org/apache/struts2/junit/StrutsJUnit4TestCase.java +++ b/plugins/junit/src/main/java/org/apache/struts2/junit/StrutsJUnit4TestCase.java @@ -94,6 +94,7 @@ public abstract class StrutsJUnit4TestCase<T> extends XWorkJUnit4TestCase { */ protected String executeAction(String uri) throws ServletException, UnsupportedEncodingException { request.setRequestURI(uri); + request.setServletPath(uri); ActionMapping mapping = getActionMapping(request); assertNotNull(mapping); @@ -112,6 +113,7 @@ public abstract class StrutsJUnit4TestCase<T> extends XWorkJUnit4TestCase { */ protected ActionProxy getActionProxy(String uri) { request.setRequestURI(uri); + request.setServletPath(uri); ActionMapping mapping = getActionMapping(request); String namespace = mapping.getNamespace(); String name = mapping.getName(); @@ -162,6 +164,7 @@ public abstract class StrutsJUnit4TestCase<T> extends XWorkJUnit4TestCase { protected ActionMapping getActionMapping(String url) { MockHttpServletRequest req = new MockHttpServletRequest(); req.setRequestURI(url); + req.setServletPath(url); return getActionMapping(req); } diff --git a/plugins/junit/src/main/java/org/apache/struts2/junit/StrutsRestTestCase.java b/plugins/junit/src/main/java/org/apache/struts2/junit/StrutsRestTestCase.java index 0112e6b8d..1a2c92034 100644 --- a/plugins/junit/src/main/java/org/apache/struts2/junit/StrutsRestTestCase.java +++ b/plugins/junit/src/main/java/org/apache/struts2/junit/StrutsRestTestCase.java @@ -69,6 +69,7 @@ public class StrutsRestTestCase<T> extends StrutsJUnit4TestCase<T> { */ protected String executeAction(String httpMethod, String uri) throws ServletException, UnsupportedEncodingException { request.setRequestURI(uri); + request.setServletPath(uri); request.setMethod(httpMethod); ActionMapping mapping = getActionMapping(request); @@ -106,6 +107,7 @@ public class StrutsRestTestCase<T> extends StrutsJUnit4TestCase<T> { */ protected ActionProxy getActionProxy(String httpMethod, String uri) { request.setRequestURI(uri); + request.setServletPath(uri); request.setMethod(httpMethod); ActionMapping mapping = getActionMapping(request); diff --git a/plugins/junit/src/main/java/org/apache/struts2/junit/StrutsTestCase.java b/plugins/junit/src/main/java/org/apache/struts2/junit/StrutsTestCase.java index 11918b432..6a5a8b3e3 100644 --- a/plugins/junit/src/main/java/org/apache/struts2/junit/StrutsTestCase.java +++ b/plugins/junit/src/main/java/org/apache/struts2/junit/StrutsTestCase.java @@ -71,6 +71,7 @@ public abstract class StrutsTestCase extends XWorkTestCase { */ protected String executeAction(String uri) throws ServletException, UnsupportedEncodingException { request.setRequestURI(uri); + request.setServletPath(uri); ActionMapping mapping = getActionMapping(request); assertNotNull(mapping); @@ -88,6 +89,7 @@ public abstract class StrutsTestCase extends XWorkTestCase { */ protected ActionProxy getActionProxy(String uri) { request.setRequestURI(uri); + request.setServletPath(uri); ActionMapping mapping = getActionMapping(request); String namespace = mapping.getNamespace(); String name = mapping.getName(); @@ -155,6 +157,7 @@ public abstract class StrutsTestCase extends XWorkTestCase { protected ActionMapping getActionMapping(String url) { MockHttpServletRequest req = new MockHttpServletRequest(); req.setRequestURI(url); + req.setServletPath(url); return getActionMapping(req); } diff --git a/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java b/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java index b8ecdaf85..202caeb77 100644 --- a/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java +++ b/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java @@ -66,9 +66,6 @@ import java.util.HashMap; * <li><code>struts.mapper.deleteMethodName</code> - The method name to call for a DELETE * request with an id parameter. Defaults to 'destroy'. * </li> - * <li><code>struts.mapper.editMethodName</code> - The method name to call for a GET - * request with an id parameter and the 'edit' view specified. Defaults to 'edit'. - * </li> * <li><code>struts.mapper.newMethodName</code> - The method name to call for a GET * request with no id parameter and the 'new' view specified. Defaults to 'editNew'. * </li> @@ -79,7 +76,6 @@ import java.util.HashMap; * <ul> * <li><code>GET: /movies => method="index"</code></li> * <li><code>GET: /movies/Thrillers => method="show", id="Thrillers"</code></li> - * <li><code>GET: /movies/Thrillers;edit => method="edit", id="Thrillers"</code></li> * <li><code>GET: /movies/Thrillers/edit => method="edit", id="Thrillers"</code></li> * <li><code>GET: /movies/new => method="editNew"</code></li> * <li><code>POST: /movies => method="create"</code></li> @@ -108,7 +104,6 @@ public class RestActionMapper extends DefaultActionMapper { private String indexMethodName = "index"; private String getMethodName = "show"; private String postMethodName = "create"; - private String editMethodName = "edit"; private String newMethodName = "editNew"; private String deleteMethodName = "destroy"; private String putMethodName = "update"; @@ -145,9 +140,15 @@ public class RestActionMapper extends DefaultActionMapper { this.postMethodName = postMethodName; } + /** + * @deprecated since 7.5.0, has no effect on the mapper: the {@code /edit} form names the method itself and + * the {@code ;edit} form is no longer recognised. {@code RestWorkflowInterceptor} still reads the setting. + * To be removed in 8.0.0. + */ + @Deprecated(since = "7.5.0", forRemoval = true) @Inject(required = false, value = RestConstants.REST_MAPPER_EDIT_METHOD_NAME) public void setEditMethodName(String editMethodName) { - this.editMethodName = editMethodName; + // no-op } @Inject(required = false, value = RestConstants.REST_MAPPER_NEW_METHOD_NAME) @@ -215,12 +216,6 @@ public class RestActionMapper extends DefaultActionMapper { // Only try something if the action name is specified if (StringUtils.isNotEmpty(fullName)) { - // cut off any ;jsessionid= type appendix but allow the rails-like ;edit - int scPos = fullName.indexOf(';'); - if (scPos > -1 && !"edit".equals(fullName.substring(scPos + 1))) { - fullName = fullName.substring(0, scPos); - } - int lastSlashPos = fullName.lastIndexOf('/'); String id = null; if (lastSlashPos > -1) { @@ -263,13 +258,8 @@ public class RestActionMapper extends DefaultActionMapper { // Handle uris with an id at the end } else if (id != null) { - // Viewing the form to edit an item e.g. foo/1;edit - if (isGet(request) && id.endsWith(";edit")) { - id = id.substring(0, id.length() - ";edit".length()); - mapping.setMethod(editMethodName); - // Viewing the form to create a new item e.g. foo/new - } else if (isGet(request) && "new".equals(id)) { + if (isGet(request) && "new".equals(id)) { mapping.setMethod(newMethodName); // Removing an item e.g. foo/1 @@ -313,14 +303,6 @@ public class RestActionMapper extends DefaultActionMapper { String actionName = name.substring(0, exclamation); String actionMethod = name.substring(exclamation + 1); - // WW-4585 - // add any ; appendix to name, it will be handled later in getMapping method - int scPos = actionMethod.indexOf(';'); - if (scPos != -1) { - actionName = actionName + actionMethod.substring(scPos); - actionMethod = actionMethod.substring(0, scPos); - } - mapping.setName(actionName); if (allowDynamicMethodCalls) { mapping.setMethod(cleanupMethodName(actionMethod)); diff --git a/plugins/rest/src/test/java/org/apache/struts2/rest/RestActionMapperTest.java b/plugins/rest/src/test/java/org/apache/struts2/rest/RestActionMapperTest.java index 9c8c2c6cb..37cde3cf0 100644 --- a/plugins/rest/src/test/java/org/apache/struts2/rest/RestActionMapperTest.java +++ b/plugins/rest/src/test/java/org/apache/struts2/rest/RestActionMapperTest.java @@ -155,7 +155,7 @@ public class RestActionMapperTest extends TestCase { assertEquals("edit", mapping.getMethod()); } - public void testEditSemicolonMapping() throws Exception { + public void testSemicolonEditIsNotSpecial() throws Exception { req.setRequestURI("/myapp/animals/dog/fido;edit"); req.setServletPath("/animals/dog/fido;edit"); req.setMethod("GET"); @@ -164,8 +164,8 @@ public class RestActionMapperTest extends TestCase { assertEquals("/animals", mapping.getNamespace()); assertEquals("dog", mapping.getName()); - assertEquals("fido", ((String[]) mapping.getParams().get("id"))[0]); - assertEquals("edit", mapping.getMethod()); + assertEquals("fido;edit", ((String[]) mapping.getParams().get("id"))[0]); + assertEquals("show", mapping.getMethod()); } public void testGetJsessionIdSemicolonMapping() throws Exception { @@ -183,7 +183,7 @@ public class RestActionMapperTest extends TestCase { public void testGetJsessionIdSemicolonMappingWithMethod() throws Exception { req.setRequestURI("/myapp/animals/dog/fido!update;jsessionid=29fefpv23do1g"); - req.setServletPath("/animals/dog/fido"); + req.setServletPath("/animals/dog/fido!update"); req.setMethod("GET"); ActionMapping mapping = mapper.getMapping(req, configManager); @@ -196,7 +196,7 @@ public class RestActionMapperTest extends TestCase { public void testGetJsessionIdSemicolonMappingWithMethodAllowDMI() throws Exception { req.setRequestURI("/myapp/animals/dog/fido!update;jsessionid=29fefpv23do1g"); - req.setServletPath("/animals/dog/fido"); + req.setServletPath("/animals/dog/fido!update"); req.setMethod("GET"); // allow DMI @@ -212,7 +212,7 @@ public class RestActionMapperTest extends TestCase { public void testMappingWithMethodAndId() throws Exception { req.setRequestURI("/myapp/animals/dog/fido/test/some-id!create;jsessionid=29fefpv23do1g"); - req.setServletPath("/animals/dog/fido/test/some-id"); + req.setServletPath("/animals/dog/fido/test/some-id!create"); req.setMethod("GET"); mapper.setAllowDynamicMethodCalls("true"); ActionMapping mapping = mapper.getMapping(req, configManager); @@ -225,7 +225,7 @@ public class RestActionMapperTest extends TestCase { public void testMappingForStaticFiles() throws Exception { req.setRequestURI("/myApp/custom/menu/Yosemite/Vernal_Fall/Vernal_Fall_Image!iframe"); - req.setServletPath("/custom/menu/Yosemite/Vernal_Fall/Vernal_Fall_Image"); + req.setServletPath("/custom/menu/Yosemite/Vernal_Fall/Vernal_Fall_Image!iframe"); req.setMethod("GET"); mapper.setAllowDynamicMethodCalls("true"); final ActionMapping mapping = mapper.getMapping(req, configManager); @@ -238,7 +238,7 @@ public class RestActionMapperTest extends TestCase { public void testMappingForStaticFilesWithJsessionId() throws Exception { req.setRequestURI("/myApp/custom/menu/Yosemite/Vernal_Fall/Vernal_Fall_Image!iframe;jsessionid=29fefpv23do1g"); - req.setServletPath("/custom/menu/Yosemite/Vernal_Fall/Vernal_Fall_Image"); + req.setServletPath("/custom/menu/Yosemite/Vernal_Fall/Vernal_Fall_Image!iframe"); req.setMethod("GET"); mapper.setAllowDynamicMethodCalls("true"); final ActionMapping mapping = mapper.getMapping(req, configManager); @@ -261,10 +261,6 @@ public class RestActionMapperTest extends TestCase { tryUri("/my/foo/", "/my", "foo/"); } - public void testParseNameAndNamespaceWithEdit() { - tryUri("/my/foo/23;edit", "/my", "foo/23;edit"); - } - /** * WW-5688: when a package declares the root namespace, a uri that matches no more specific * namespace belongs to that root rather than to the default namespace, so that "/foo" and
