This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/struts.git
commit 4d061a464b6df4a0df49be847ff92ca602b25610 Author: Lukasz Lenart <[email protected]> AuthorDate: Mon Nov 2 07:56:03 2026 +0100 WW-4164 feat(core): map declared extensions to their actions Co-Authored-By: Claude Opus 5.5 <[email protected]> --- .../dispatcher/mapper/DefaultActionMapper.java | 80 ++++++++++++++- .../dispatcher/mapper/DefaultActionMapperTest.java | 114 +++++++++++++++++++++ ...s-extension.xml => struts-extension-reload.xml} | 16 +-- .../struts2/config/providers/struts-extension.xml | 4 + 4 files changed, 199 insertions(+), 15 deletions(-) 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 ffdda8bea..b5a38921b 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 @@ -21,6 +21,7 @@ package org.apache.struts2.dispatcher.mapper; import org.apache.struts2.ActionContext; import org.apache.struts2.config.Configuration; import org.apache.struts2.config.ConfigurationManager; +import org.apache.struts2.config.RuntimeConfiguration; import org.apache.struts2.config.entities.ActionConfig; import org.apache.struts2.config.entities.PackageConfig; import org.apache.struts2.inject.Container; @@ -34,6 +35,7 @@ import org.apache.struts2.RequestUtils; import org.apache.struts2.StrutsConstants; import org.apache.struts2.util.PrefixTrie; +import java.util.Comparator; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -135,6 +137,8 @@ public class DefaultActionMapper implements ActionMapper { protected List<String> extensions = List.of("action", ""); + private volatile DeclaredExtensions declaredExtensions; + protected Container container; public DefaultActionMapper() { @@ -249,6 +253,7 @@ public class DefaultActionMapper implements ActionMapper { } else { this.extensions = null; } + this.declaredExtensions = null; } @Override @@ -275,7 +280,8 @@ public class DefaultActionMapper implements ActionMapper { int indexOfSemicolon = uri.indexOf(';'); uri = (indexOfSemicolon > -1) ? uri.substring(0, indexOfSemicolon) : uri; - uri = dropExtension(uri, mapping); + String declaredUri = stripDeclaredExtension(uri, mapping, configManager); + uri = declaredUri != null ? declaredUri : dropExtension(uri, mapping); if (uri == null) { return null; } @@ -468,6 +474,78 @@ public class DefaultActionMapper implements ActionMapper { mapping.setMethod(methodName); } + /** + * Strips an extension that only package or action {@code extension} attributes declare, but only when the + * remaining URI resolves to an action that itself declares extensions. + * + * @param uri the request URI + * @param mapping the action mapping to store the extension in + * @param configManager configuration manager + * @return the URI without the extension, or {@code null} when no declared-only extension claims it + * @since 7.5.0 + */ + protected String stripDeclaredExtension(String uri, ActionMapping mapping, ConfigurationManager configManager) { + RuntimeConfiguration runtimeConfiguration = configManager.getConfiguration().getRuntimeConfiguration(); + if (runtimeConfiguration == null) { + return null; + } + for (String ext : declaredOnlyExtensions(runtimeConfiguration)) { + String suffix = "." + ext; + if (uri.endsWith(suffix)) { + String candidate = uri.substring(0, uri.length() - suffix.length()); + if (declaresExtensions(candidate, runtimeConfiguration, configManager)) { + mapping.setExtension(ext); + return candidate; + } + } + } + return null; + } + + private boolean declaresExtensions(String candidate, RuntimeConfiguration runtimeConfiguration, ConfigurationManager configManager) { + ActionMapping probe = new ActionMapping(); + parseNameAndNamespace(candidate, probe, configManager); + String name = probe.getName(); + int bang = name.indexOf('!'); + String actionName = bang == -1 ? name : name.substring(0, bang); + ActionConfig config = runtimeConfiguration.getActionConfig(probe.getNamespace(), actionName); + return config != null && config.getExtensions() != null; + } + + private List<String> declaredOnlyExtensions(RuntimeConfiguration runtimeConfiguration) { + DeclaredExtensions cached = declaredExtensions; + if (cached != null && cached.runtimeConfiguration == runtimeConfiguration) { + return cached.declaredOnly; + } + Set<String> declared = new HashSet<>(); + for (Map<String, ActionConfig> actionConfigs : runtimeConfiguration.getActionConfigs().values()) { + for (ActionConfig actionConfig : actionConfigs.values()) { + if (actionConfig.getExtensions() != null) { + declared.addAll(actionConfig.getExtensions()); + } + } + } + declared.remove(""); + if (extensions != null) { + extensions.forEach(declared::remove); + } + List<String> declaredOnly = declared.stream() + .sorted(Comparator.comparingInt(String::length).reversed().thenComparing(Comparator.naturalOrder())) + .toList(); + declaredExtensions = new DeclaredExtensions(runtimeConfiguration, declaredOnly); + return declaredOnly; + } + + private static final class DeclaredExtensions { + private final RuntimeConfiguration runtimeConfiguration; + private final List<String> declaredOnly; + + private DeclaredExtensions(RuntimeConfiguration runtimeConfiguration, List<String> declaredOnly) { + this.runtimeConfiguration = runtimeConfiguration; + this.declaredOnly = declaredOnly; + } + } + /** * Drops the extension from the action name, storing it in the mapping for later use * 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 a03319c3a..75683b488 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 @@ -36,13 +36,19 @@ import org.apache.struts2.config.entities.ActionConfig; import org.apache.struts2.config.entities.PackageConfig; import org.apache.struts2.config.entities.ResultConfig; import org.apache.struts2.config.impl.DefaultConfiguration; +import org.apache.struts2.config.StrutsXmlConfigurationProvider; import org.apache.struts2.inject.Container; +import static org.assertj.core.api.Assertions.assertThat; + /** * DefaultActionMapper test case. */ public class DefaultActionMapperTest extends StrutsInternalTestCase { + private static final String EXTENSION_CONFIG = "org/apache/struts2/config/providers/struts-extension.xml"; + private static final String EXTENSION_RELOAD_CONFIG = "org/apache/struts2/config/providers/struts-extension-reload.xml"; + private MockHttpServletRequest req; private ConfigurationManager configManager; private Configuration config; @@ -997,4 +1003,112 @@ public class DefaultActionMapperTest extends StrutsInternalTestCase { assertEquals("error", result); } + public void testDeclaredExtensionClaimedForResolvingAction() { + loadConfigurationProviders(new StrutsXmlConfigurationProvider(EXTENSION_CONFIG)); + + ActionMapping mapping = mapDeclared(declaredMapper(), "/child/inherits.html"); + + assertThat(mapping.getName()).isEqualTo("inherits"); + assertThat(mapping.getNamespace()).isEqualTo("/child"); + assertThat(mapping.getExtension()).isEqualTo("html"); + } + + public void testDeclaredExtensionNotClaimedForStaticFile() { + loadConfigurationProviders(new StrutsXmlConfigurationProvider(EXTENSION_CONFIG)); + + assertThat(mapDeclared(declaredMapper(), "/index.html")).isNull(); + } + + public void testDeclaredOnlyExtensionNotClaimedForUndeclaredAction() { + loadConfigurationProviders(new StrutsXmlConfigurationProvider(EXTENSION_CONFIG)); + + assertThat(mapDeclared(declaredMapper(), "/plain/undeclared.html")).isNull(); + } + + public void testDeclaredActionClaimsOtherDeclaredExtension() { + loadConfigurationProviders(new StrutsXmlConfigurationProvider(EXTENSION_CONFIG)); + + ActionMapping mapping = mapDeclared(declaredMapper(), "/child/inherits.json"); + + assertThat(mapping.getName()).isEqualTo("inherits"); + assertThat(mapping.getExtension()).isEqualTo("json"); + } + + public void testGlobalExtensionUnchanged() { + loadConfigurationProviders(new StrutsXmlConfigurationProvider(EXTENSION_CONFIG)); + + ActionMapping mapping = mapDeclared(declaredMapper(), "/plain/undeclared.action"); + + assertThat(mapping.getName()).isEqualTo("undeclared"); + assertThat(mapping.getExtension()).isEqualTo("action"); + } + + public void testDmiNameWithDeclaredExtension() { + loadConfigurationProviders(new StrutsXmlConfigurationProvider(EXTENSION_CONFIG)); + DefaultActionMapper mapper = declaredMapper(); + mapper.setAllowDynamicMethodCalls("true"); + + ActionMapping mapping = mapDeclared(mapper, "/child/inherits!foo.html"); + + assertThat(mapping.getName()).isEqualTo("inherits"); + assertThat(mapping.getMethod()).isEqualTo("foo"); + assertThat(mapping.getExtension()).isEqualTo("html"); + } + + public void testNamespaceFallbackWithDeclaredExtension() { + loadConfigurationProviders(new StrutsXmlConfigurationProvider(EXTENSION_CONFIG)); + assertThat(configurationManager.getConfiguration().getRuntimeConfiguration().getActionConfig("/other", "thing")).isNotNull(); + + ActionMapping mapping = mapDeclared(declaredMapper(), "/other/thing.html"); + + assertThat(mapping.getName()).isEqualTo("thing"); + assertThat(mapping.getExtension()).isEqualTo("html"); + } + + public void testWildcardWithDeclaredExtension() { + loadConfigurationProviders(new StrutsXmlConfigurationProvider(EXTENSION_CONFIG)); + + ActionMapping mapping = mapDeclared(declaredMapper(), "/wild/edit_user.html"); + + assertThat(mapping.getName()).isEqualTo("edit_user"); + assertThat(mapping.getExtension()).isEqualTo("html"); + } + + public void testGlobalNullStillStripsDeclared() { + loadConfigurationProviders(new StrutsXmlConfigurationProvider(EXTENSION_CONFIG)); + DefaultActionMapper mapper = new DefaultActionMapper(); + mapper.setExtensions(""); + + ActionMapping declared = mapDeclared(mapper, "/child/inherits.html"); + assertThat(declared.getName()).isEqualTo("inherits"); + assertThat(declared.getExtension()).isEqualTo("html"); + + ActionMapping literal = mapDeclared(mapper, "/plain/foo.bar"); + assertThat(literal.getName()).isEqualTo("foo.bar"); + assertThat(literal.getExtension()).isNull(); + } + + public void testCacheFollowsReload() { + loadConfigurationProviders(new StrutsXmlConfigurationProvider(EXTENSION_CONFIG)); + DefaultActionMapper mapper = declaredMapper(); + assertThat(mapDeclared(mapper, "/child/inherits.html")).isNotNull(); + + loadConfigurationProviders(new StrutsXmlConfigurationProvider(EXTENSION_RELOAD_CONFIG)); + + assertThat(mapDeclared(mapper, "/child/inherits.html")).isNull(); + assertThat(mapDeclared(mapper, "/child/inherits.json").getExtension()).isEqualTo("json"); + } + + private DefaultActionMapper declaredMapper() { + DefaultActionMapper mapper = new DefaultActionMapper(); + mapper.setExtensions("action,"); + return mapper; + } + + private ActionMapping mapDeclared(DefaultActionMapper mapper, String uri) { + req.setRequestURI(uri); + req.setServletPath(uri); + return mapper.getMapping(req, configurationManager); + } + } diff --git a/core/src/test/resources/org/apache/struts2/config/providers/struts-extension.xml b/core/src/test/resources/org/apache/struts2/config/providers/struts-extension-reload.xml similarity index 65% copy from core/src/test/resources/org/apache/struts2/config/providers/struts-extension.xml copy to core/src/test/resources/org/apache/struts2/config/providers/struts-extension-reload.xml index 15b3e7fd4..502dc8775 100644 --- a/core/src/test/resources/org/apache/struts2/config/providers/struts-extension.xml +++ b/core/src/test/resources/org/apache/struts2/config/providers/struts-extension-reload.xml @@ -23,19 +23,7 @@ "-//Apache Software Foundation//DTD Struts Configuration 7.5//EN" "https://struts.apache.org/dtds/struts-7.5.dtd";> <struts> - <package name="base" abstract="true" extension="html"/> - - <package name="child" extends="base" namespace="/child"> - <action name="inherits" class="org.apache.struts2.TestAction"/> - <action name="own" class="org.apache.struts2.TestAction" extension="json,"/> - </package> - - <package name="plain" namespace="/plain"> - <action name="undeclared" class="org.apache.struts2.TestAction"/> - <action name="none" class="org.apache.struts2.TestAction" extension=""/> - </package> - - <package name="wild" namespace="/wild"> - <action name="edit_*" class="org.apache.struts2.TestAction" extension="html"/> + <package name="child" namespace="/child"> + <action name="inherits" class="org.apache.struts2.TestAction" extension="json"/> </package> </struts> diff --git a/core/src/test/resources/org/apache/struts2/config/providers/struts-extension.xml b/core/src/test/resources/org/apache/struts2/config/providers/struts-extension.xml index 15b3e7fd4..2dd2fd1a3 100644 --- a/core/src/test/resources/org/apache/struts2/config/providers/struts-extension.xml +++ b/core/src/test/resources/org/apache/struts2/config/providers/struts-extension.xml @@ -38,4 +38,8 @@ <package name="wild" namespace="/wild"> <action name="edit_*" class="org.apache.struts2.TestAction" extension="html"/> </package> + + <package name="fallback" namespace=""> + <action name="thing" class="org.apache.struts2.TestAction" extension="html"/> + </package> </struts>
