This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch WW-5842-container-servlet-path-6x in repository https://gitbox.apache.org/repos/asf/struts.git
commit 49c2990f69363a610866e30da7196f981669d1af Author: Lukasz Lenart <[email protected]> AuthorDate: Sat Nov 7 11:40:51 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, and the mockobjects-based mapper tests stub the path info. 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 | 37 ++---------- .../struts2/dispatcher/ExecuteOperations.java | 4 -- .../dispatcher/mapper/DefaultActionMapper.java | 3 - .../apache/struts2/views/util/ResourceUtil.java | 2 +- .../java/org/apache/struts2/RequestUtilsTest.java | 70 ++++++++++++---------- ...rutsPrepareAndExecuteFilterIntegrationTest.java | 7 +++ .../dispatcher/TwoFilterIntegrationTest.java | 4 ++ .../dispatcher/mapper/DefaultActionMapperTest.java | 5 +- .../mapper/Restful2ActionMapperTest.java | 1 + .../struts2/result/ServletRedirectResultTest.java | 4 +- .../freemarker/FreemarkerResultMockedTest.java | 8 +++ .../struts2/views/util/ResourceUtilTest.java | 4 +- .../apache/struts2/junit/StrutsJUnit4TestCase.java | 3 + .../apache/struts2/junit/StrutsRestTestCase.java | 2 + .../org/apache/struts2/junit/StrutsTestCase.java | 3 + .../apache/struts2/StrutsTestCasePortletTests.java | 3 + .../org/apache/struts2/rest/RestActionMapper.java | 34 +++-------- .../apache/struts2/rest/RestActionMapperTest.java | 20 +++---- 18 files changed, 99 insertions(+), 115 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/RequestUtils.java b/core/src/main/java/org/apache/struts2/RequestUtils.java index 40ed2ddb0..70c241c7a 100644 --- a/core/src/main/java/org/apache/struts2/RequestUtils.java +++ b/core/src/main/java/org/apache/struts2/RequestUtils.java @@ -49,36 +49,15 @@ public class RequestUtils { }; /** - * Retrieves the current request servlet path. - * Deals with differences between servlet specs (2.2 vs 2.3+) + * 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().equals("") ? 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()); } /** @@ -94,13 +73,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 769ca0a39..d6724edc2 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 96dae07c4..6b3da5a47 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 @@ -279,9 +279,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; - uri = dropExtension(uri, mapping); if (uri == null) { return 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 52cb3ba31..3fa7fc6b5 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 javax.servlet.http.HttpServletRequest; public class ResourceUtil { public static String getResourceBase(HttpServletRequest req) { String path = RequestUtils.getServletPath(req); - if (path == null || "".equals(path)) { + 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 e7d7437bc..74970a5cd 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 javax.servlet.RequestDispatcher; import javax.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/StrutsPrepareAndExecuteFilterIntegrationTest.java b/core/src/test/java/org/apache/struts2/dispatcher/StrutsPrepareAndExecuteFilterIntegrationTest.java index c7d525722..72db33bbd 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 65d2dfaea..8e7e99041 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 c1cca1a11..377d4f841 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 @@ -51,6 +51,7 @@ public class DefaultActionMapperTest extends StrutsInternalTestCase { req = new MockHttpServletRequest(); req.setupGetParameterMap(new HashMap()); req.setupGetContextPath("/my/namespace"); + req.setupPathInfo(null); config = new DefaultConfiguration(); PackageConfig pkg = new PackageConfig.Builder("myns") @@ -270,10 +271,10 @@ public class DefaultActionMapperTest extends StrutsInternalTestCase { } @SuppressWarnings("rawtypes") - public void testGetUriWithSemicolonPresent() { + public void testGetUriWithPathParametersStrippedByTheContainer() { req.setupGetParameterMap(new HashMap()); req.setupGetRequestURI("/my/namespace/actionName.action;abc=123rty56"); - req.setupGetServletPath("/my/namespace/actionName.action;abc=123rty56"); + req.setupGetServletPath("/my/namespace/actionName.action"); req.setupGetAttribute(null); req.addExpectedGetAttributeName("javax.servlet.include.servlet_path"); diff --git a/core/src/test/java/org/apache/struts2/dispatcher/mapper/Restful2ActionMapperTest.java b/core/src/test/java/org/apache/struts2/dispatcher/mapper/Restful2ActionMapperTest.java index d792b227e..2e9c9e2bc 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/mapper/Restful2ActionMapperTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/mapper/Restful2ActionMapperTest.java @@ -45,6 +45,7 @@ public class Restful2ActionMapperTest extends StrutsInternalTestCase { req = new MockHttpServletRequest(); req.setupGetParameterMap(new HashMap()); req.setupGetContextPath("/my/namespace"); + req.setupPathInfo(null); config = new DefaultConfiguration(); PackageConfig pkg = new PackageConfig.Builder("myns") 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 e8746aa8d..64f67e6a9 100644 --- a/core/src/test/java/org/apache/struts2/result/ServletRedirectResultTest.java +++ b/core/src/test/java/org/apache/struts2/result/ServletRedirectResultTest.java @@ -180,7 +180,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"))); @@ -195,7 +195,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 7801c50b0..e6339cfd9 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); @@ -155,6 +158,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" @@ -177,6 +181,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\"/>"; @@ -221,6 +226,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(); @@ -263,6 +269,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); @@ -317,6 +324,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 6512fb8dc..8fbe0bcea 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/junit/src/main/java/org/apache/struts2/junit/StrutsJUnit4TestCase.java b/plugins/junit/src/main/java/org/apache/struts2/junit/StrutsJUnit4TestCase.java index 93a08e399..4533ac7b5 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 82656edae..a2413fd3a 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 6280de8a8..e56f75d05 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 @@ -70,6 +70,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); @@ -87,6 +88,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(); @@ -154,6 +156,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/portlet/src/test/java/org/apache/struts2/StrutsTestCasePortletTests.java b/plugins/portlet/src/test/java/org/apache/struts2/StrutsTestCasePortletTests.java index 0da8d1043..e20d53b95 100644 --- a/plugins/portlet/src/test/java/org/apache/struts2/StrutsTestCasePortletTests.java +++ b/plugins/portlet/src/test/java/org/apache/struts2/StrutsTestCasePortletTests.java @@ -83,6 +83,7 @@ public abstract class StrutsTestCasePortletTests extends XWorkTestCase { */ protected String executeAction(String uri) throws ServletException, UnsupportedEncodingException { request.setRequestURI(uri); + request.setServletPath(uri); ActionMapping mapping = getActionMapping(request); assertNotNull(mapping); @@ -100,6 +101,7 @@ public abstract class StrutsTestCasePortletTests 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(); @@ -167,6 +169,7 @@ public abstract class StrutsTestCasePortletTests 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 1678fd9a6..088230511 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 6.13.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 @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) @@ -210,12 +211,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) { @@ -258,13 +253,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 @@ -308,14 +298,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 dd5262a81..8adc8ca93 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 @@ -151,7 +151,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"); @@ -160,8 +160,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 { @@ -179,7 +179,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); @@ -192,7 +192,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 @@ -208,7 +208,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); @@ -221,7 +221,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); @@ -234,7 +234,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); @@ -257,10 +257,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
