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 0cb27d0786b62071e6ade713e88d24e5ed3a2fee Author: Lukasz Lenart <[email protected]> AuthorDate: Mon Nov 2 09:50:04 2026 +0100 WW-4164 fix(core): parse probe names like the mapper and keep allowed extensionless links - Resolver cuts a target name at the last ! only when DMI is enabled, so a literal report!daily resolves its own action - Resolver treats an extensionless current request as "" for declared targets, keeping it when allowed - Mapper declaration probe refuses names cleanupActionName replaced with the default action name - Mapper declaration probe derives the name through parseActionName, matching the final mapping for DMI on and off Co-Authored-By: Claude Opus 5.5 <[email protected]> --- .../dispatcher/mapper/ActionExtensionResolver.java | 51 +++++++++++++++------- .../dispatcher/mapper/DefaultActionMapper.java | 14 ++++-- .../mapper/ActionExtensionResolverTest.java | 37 ++++++++++++++++ .../dispatcher/mapper/DefaultActionMapperTest.java | 37 ++++++++++++++++ .../config/providers/struts-extension-bang.xml | 38 ++++++++++++++++ 5 files changed, 157 insertions(+), 20 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/dispatcher/mapper/ActionExtensionResolver.java b/core/src/main/java/org/apache/struts2/dispatcher/mapper/ActionExtensionResolver.java index 6849bdb56..f322b10f0 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/mapper/ActionExtensionResolver.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/mapper/ActionExtensionResolver.java @@ -18,6 +18,7 @@ */ package org.apache.struts2.dispatcher.mapper; +import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.StringUtils; import org.apache.struts2.ActionContext; import org.apache.struts2.StrutsConstants; @@ -38,6 +39,7 @@ public class ActionExtensionResolver { private Configuration configuration; private List<String> extensions; + private boolean allowDynamicMethodCalls; private volatile DeclarationState declarationState; /** @@ -58,13 +60,24 @@ public class ActionExtensionResolver { this.extensions = StringUtils.isNotEmpty(extensions) ? ActionExtensions.parse(extensions) : null; } + /** + * @param enableDynamicMethodCalls whether {@code name!method} names are split at their last {@code !} + * @since 7.5.0 + */ + @Inject(StrutsConstants.STRUTS_ENABLE_DYNAMIC_METHOD_INVOCATION) + public void setAllowDynamicMethodCalls(String enableDynamicMethodCalls) { + this.allowDynamicMethodCalls = BooleanUtils.toBoolean(enableDynamicMethodCalls); + } + /** * Sets the extension of {@code target} to the current request's extension when the target accepts it, otherwise - * to the target's first accepted extension, which is also used when the current request has no extension, as in - * {@link DefaultActionMapper#lookupExtension(String)}. Leaves {@code target} unchanged when it already has an extension, - * when no action in the configuration declares extensions, when the target action is unknown (a name reached only - * through the namespace's {@code default-action-ref} counts as unknown), or when neither the - * target nor the global constant declares extensions. + * to the target's first accepted extension. For a target declaring extensions an extensionless request counts as + * the empty extension; the first accepted extension is used when there is no current request, or for an + * extensionless request to an undeclared target, as in {@link DefaultActionMapper#lookupExtension(String)}. + * Leaves {@code target} unchanged when it already has an extension, when no action in the configuration declares + * extensions, when the target action is unknown (a name reached only through the namespace's + * {@code default-action-ref} counts as unknown), or when neither the target nor the global constant declares + * extensions. * * @param target the mapping of the action the URL points at * @since 7.5.0 @@ -77,17 +90,26 @@ public class ActionExtensionResolver { if (!anyActionDeclaresExtensions(runtimeConfiguration)) { return; } - String actionName = StringUtils.substringBefore(StringUtils.substringBefore(target.getName(), "?"), "!"); - ActionConfig actionConfig = runtimeConfiguration.getMatchedActionConfig(target.getNamespace(), actionName); + ActionConfig actionConfig = runtimeConfiguration.getMatchedActionConfig(target.getNamespace(), actionName(target.getName())); if (actionConfig == null) { return; } - List<String> effective = !actionConfig.getExtensions().isEmpty() ? actionConfig.getExtensions() : extensions; - if (effective == null) { + ActionMapping currentMapping = currentMapping(); + List<String> declared = actionConfig.getExtensions(); + if (!declared.isEmpty()) { + target.setExtension(currentMapping == null ? declared.get(0) : ActionExtensions.select(declared, currentMapping.getExtension())); return; } - String current = currentExtension(); - target.setExtension(current == null ? effective.get(0) : ActionExtensions.select(effective, current)); + if (extensions == null) { + return; + } + String current = currentMapping == null ? null : currentMapping.getExtension(); + target.setExtension(current == null ? extensions.get(0) : ActionExtensions.select(extensions, current)); + } + + private String actionName(String name) { + String withoutQuery = StringUtils.substringBefore(name, "?"); + return allowDynamicMethodCalls ? StringUtils.substringBeforeLast(withoutQuery, "!") : withoutQuery; } private boolean anyActionDeclaresExtensions(RuntimeConfiguration runtimeConfiguration) { @@ -102,12 +124,9 @@ public class ActionExtensionResolver { return anyDeclared; } - private String currentExtension() { + private ActionMapping currentMapping() { ActionContext context = ActionContext.getContext(); - if (context == null || context.getActionMapping() == null) { - return null; - } - return context.getActionMapping().getExtension(); + return context == null ? null : context.getActionMapping(); } private static final class DeclarationState { 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 bb624688b..63208956f 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 @@ -511,13 +511,19 @@ public class DefaultActionMapper implements ActionMapper { 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.getMatchedActionConfig(probe.getNamespace(), actionName); + if (!keepsRequestedName(candidate, probe.getName())) { + return false; + } + parseActionName(probe); + ActionConfig config = runtimeConfiguration.getMatchedActionConfig(probe.getNamespace(), probe.getName()); return config != null && !config.getExtensions().isEmpty(); } + private static boolean keepsRequestedName(String uri, String name) { + String lastSegment = uri.substring(uri.lastIndexOf('/') + 1); + return name.equals(lastSegment) || name.endsWith("/" + lastSegment); + } + private DeclaredExtensions declaredExtensions(RuntimeConfiguration runtimeConfiguration) { DeclaredExtensions cached = declaredExtensions; if (cached != null && cached.runtimeConfiguration == runtimeConfiguration) { diff --git a/core/src/test/java/org/apache/struts2/dispatcher/mapper/ActionExtensionResolverTest.java b/core/src/test/java/org/apache/struts2/dispatcher/mapper/ActionExtensionResolverTest.java index 0713e9a88..a1e98405c 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/mapper/ActionExtensionResolverTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/mapper/ActionExtensionResolverTest.java @@ -80,6 +80,7 @@ public class ActionExtensionResolverTest extends StrutsInternalTestCase { } public void testDmiNameResolvesActionConfig() { + resolver.setAllowDynamicMethodCalls("true"); withCurrentExtension("action"); assertThat(resolve("/child", "inherits!foo")).isEqualTo("html"); @@ -125,6 +126,42 @@ public class ActionExtensionResolverTest extends StrutsInternalTestCase { assertThat(resolve("", "Bar")).isNull(); } + public void testLiteralBangNameWithoutDmi() { + useBangConfig(); + withCurrentExtension("action"); + + assertThat(resolve("/bang", "report!daily")).isEqualTo("html"); + } + + public void testBangNameWithDmiResolvesActionBeforeLastBang() { + useBangConfig(); + resolver.setAllowDynamicMethodCalls("true"); + withCurrentExtension("action"); + + assertThat(resolve("/bang", "report!daily")).isEqualTo("json"); + } + + public void testExtensionlessPageKeepsAllowedEmptyExtension() { + withCurrentExtension(null); + + assertThat(resolve("/child", "own")).isEmpty(); + } + + public void testExtensionlessPageUsesFirstWhenEmptyNotAllowed() { + withCurrentExtension(null); + + assertThat(resolve("/child", "inherits")).isEqualTo("html"); + } + + public void testWithoutCurrentMappingDeclaredTargetUsesFirst() { + assertThat(resolve("/child", "own")).isEqualTo("json"); + } + + private void useBangConfig() { + loadConfigurationProviders(new StrutsXmlConfigurationProvider("org/apache/struts2/config/providers/struts-extension-bang.xml")); + resolver.setConfiguration(configuration); + } + private String resolve(String namespace, String name) { ActionMapping target = new ActionMapping(name, namespace, null, null); resolver.applyTo(target); 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 96810e955..7113f60d8 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 @@ -49,6 +49,7 @@ 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 static final String EXTENSION_DEFAULT_ACTION_CONFIG = "org/apache/struts2/config/providers/struts-extension-default-action.xml"; + private static final String EXTENSION_BANG_CONFIG = "org/apache/struts2/config/providers/struts-extension-bang.xml"; private MockHttpServletRequest req; private ConfigurationManager configManager; @@ -1146,6 +1147,42 @@ public class DefaultActionMapperTest extends StrutsInternalTestCase { assertThat(mapDeclared(mapper, "/plain/undeclared.action").getName()).isEqualTo("undeclared"); } + public void testDeclaringDefaultActionDoesNotClaimDisallowedName() { + loadConfigurationProviders(new StrutsXmlConfigurationProvider(EXTENSION_DEFAULT_ACTION_CONFIG)); + + assertThat(mapDeclared(declaredMapper(), "/manual(1).html")).isNull(); + } + + public void testDeclaredExtensionClaimedForLiteralBangNameWithoutDmi() { + loadConfigurationProviders(new StrutsXmlConfigurationProvider(EXTENSION_BANG_CONFIG)); + + ActionMapping mapping = mapDeclared(declaredMapper(), "/bang-only/report!daily.html"); + + assertThat(mapping.getName()).isEqualTo("report!daily"); + assertThat(mapping.getMethod()).isNull(); + assertThat(mapping.getExtension()).isEqualTo("html"); + } + + public void testDeclaredExtensionClaimedForDmiName() { + loadConfigurationProviders(new StrutsXmlConfigurationProvider(EXTENSION_BANG_CONFIG)); + DefaultActionMapper mapper = declaredMapper(); + mapper.setAllowDynamicMethodCalls("true"); + + ActionMapping mapping = mapDeclared(mapper, "/dmi/report!daily.html"); + + assertThat(mapping.getName()).isEqualTo("report"); + assertThat(mapping.getMethod()).isEqualTo("daily"); + assertThat(mapping.getExtension()).isEqualTo("html"); + } + + public void testDeclaredExtensionProbeSplitsDmiNameAtLastBang() { + loadConfigurationProviders(new StrutsXmlConfigurationProvider(EXTENSION_BANG_CONFIG)); + DefaultActionMapper mapper = declaredMapper(); + mapper.setAllowDynamicMethodCalls("true"); + + assertThat(mapDeclared(mapper, "/dmi/report!x!daily.html")).isNull(); + } + private DefaultActionMapper declaredMapper() { DefaultActionMapper mapper = new DefaultActionMapper(); mapper.setExtensions("action,"); diff --git a/core/src/test/resources/org/apache/struts2/config/providers/struts-extension-bang.xml b/core/src/test/resources/org/apache/struts2/config/providers/struts-extension-bang.xml new file mode 100644 index 000000000..bd2f7a011 --- /dev/null +++ b/core/src/test/resources/org/apache/struts2/config/providers/struts-extension-bang.xml @@ -0,0 +1,38 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +--> +<!DOCTYPE struts PUBLIC + "-//Apache Software Foundation//DTD Struts Configuration 7.5//EN" + "https://struts.apache.org/dtds/struts-7.5.dtd";> +<struts> + <package name="bang" namespace="/bang"> + <action name="report!daily" class="org.apache.struts2.TestAction" extension="html"/> + <action name="report" class="org.apache.struts2.TestAction" extension="json"/> + </package> + + <package name="bang-only" namespace="/bang-only"> + <action name="report!daily" class="org.apache.struts2.TestAction" extension="html"/> + </package> + + <package name="dmi" namespace="/dmi"> + <action name="report" class="org.apache.struts2.TestAction" extension="html"/> + </package> +</struts>
