From 3fa9d9b7985403809c3e4c8baf0a015145a144bf Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sun, 30 Aug 2026 08:37:51 +0200 Subject: [PATCH] WW-5706 fix(core): align RestfulActionMapper action name handling with DefaultActionMapper RestfulActionMapper derived the action name straight from the request URI, unlike DefaultActionMapper which validates it via cleanupActionName against the allowedActionNames pattern. Apply the same check (and the struts.allowed.action.names / struts.default.action.name settings) so both mappers handle action names consistently. Fixes: https://issues.apache.org/jira/browse/WW-5706 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_016XMyQ1CRuYZkqygmD4aGHv --- .../mapper/RestfulActionMapper.java | 40 ++++++++++++++++++- .../mapper/RestfulActionMapperTest.java | 16 ++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/struts2/dispatcher/mapper/RestfulActionMapper.java b/core/src/main/java/org/apache/struts2/dispatcher/mapper/RestfulActionMapper.java index d93d45e98f..92307d6916 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/mapper/RestfulActionMapper.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/mapper/RestfulActionMapper.java @@ -24,11 +24,13 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.struts2.RequestUtils; +import org.apache.struts2.StrutsConstants; import org.apache.struts2.url.UrlDecoder; import java.util.HashMap; import java.util.Map; import java.util.StringTokenizer; +import java.util.regex.Pattern; /** * Simple Restfull Action Mapper to support REST application @@ -41,11 +43,31 @@ public class RestfulActionMapper implements ActionMapper { private UrlDecoder decoder; + /** + * Matches action names allowed in the request URI, aligned with {@link DefaultActionMapper}. + */ + private Pattern allowedActionNames = Pattern.compile("[a-zA-Z0-9._!/\\-]*"); + + /** + * Action name used when the name extracted from the URI is not allowed, aligned with {@link DefaultActionMapper}. + */ + private String defaultActionName = "index"; + @Inject public void setDecoder(UrlDecoder decoder) { this.decoder = decoder; } + @Inject(value = StrutsConstants.STRUTS_ALLOWED_ACTION_NAMES, required = false) + public void setAllowedActionNames(String allowedActionNames) { + this.allowedActionNames = Pattern.compile(allowedActionNames); + } + + @Inject(value = StrutsConstants.STRUTS_DEFAULT_ACTION_NAME, required = false) + public void setDefaultActionName(String defaultActionName) { + this.defaultActionName = defaultActionName; + } + /* (non-Javadoc) * @see org.apache.struts2.dispatcher.mapper.ActionMapper#getMapping(jakarta.servlet.http.HttpServletRequest) */ @@ -57,7 +79,7 @@ public ActionMapping getMapping(HttpServletRequest request, ConfigurationManager return null; } - String actionName = uri.substring(1, nextSlash); + String actionName = cleanupActionName(uri.substring(1, nextSlash)); Map parameters = new HashMap<>(); try { StringTokenizer st = new StringTokenizer(uri.substring(nextSlash), "/"); @@ -96,6 +118,22 @@ public ActionMapping getMappingFromActionName(String actionName) { return new ActionMapping(actionName, null, null, null); } + /** + * Checks action name against the allowed pattern; if it does not match, returns the default action name. + * Mirrors {@link DefaultActionMapper#cleanupActionName(String)}. + * + * @param rawActionName action name extracted from the URI + * @return safe action name + */ + protected String cleanupActionName(final String rawActionName) { + if (allowedActionNames.matcher(rawActionName).matches()) { + return rawActionName; + } else { + LOG.warn("{} did not match allowed action names {} - default action {} will be used!", rawActionName, allowedActionNames, defaultActionName); + return defaultActionName; + } + } + /* (non-Javadoc) * @see org.apache.struts2.dispatcher.mapper.ActionMapper#getUriFromActionMapping(org.apache.struts2.dispatcher.mapper.ActionMapping) */ diff --git a/core/src/test/java/org/apache/struts2/dispatcher/mapper/RestfulActionMapperTest.java b/core/src/test/java/org/apache/struts2/dispatcher/mapper/RestfulActionMapperTest.java index d8bc9d5534..79efccfa09 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/mapper/RestfulActionMapperTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/mapper/RestfulActionMapperTest.java @@ -105,6 +105,22 @@ public void testGetMapping3() { assertEquals("europe", am.getParams().get("region")); } + public void testGetMappingRejectsActionNameWithDisallowedCharacters() { + StrutsMockHttpServletRequest request = new StrutsMockHttpServletRequest(); + request.setServletPath("/%{1+1}/x"); + + ActionMapping am = mapper.getMapping(request, null); + assertEquals("index", am.getName()); + } + + public void testGetMappingAcceptsRegularActionName() { + StrutsMockHttpServletRequest request = new StrutsMockHttpServletRequest(); + request.setServletPath("/my-app.action/x"); + + ActionMapping am = mapper.getMapping(request, null); + assertEquals("my-app.action", am.getName()); + } + protected void setUp() throws Exception { super.setUp(); mapper = new RestfulActionMapper();