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 0ced1f2d58406660a1e551a08cdb4e04085b8c56 Author: Lukasz Lenart <[email protected]> AuthorDate: Mon Nov 2 07:44:34 2026 +0100 WW-4164 feat(core): parse extension attribute on package and action Co-Authored-By: Claude Sonnet 5.5 <[email protected]> --- .../struts2/config/entities/ActionConfig.java | 28 +++++++- .../struts2/config/entities/PackageConfig.java | 37 +++++++++++ .../struts2/config/impl/ActionConfigMatcher.java | 1 + .../struts2/config/impl/DefaultConfiguration.java | 5 ++ .../providers/XmlDocConfigurationProvider.java | 22 +++++++ core/src/main/resources/struts-7.5.dtd | 2 + .../XmlConfigurationProviderExtensionTest.java | 76 ++++++++++++++++++++++ .../config/providers/struts-extension-invalid.xml | 29 +++++++++ .../struts2/config/providers/struts-extension.xml | 41 ++++++++++++ 9 files changed, 240 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/struts2/config/entities/ActionConfig.java b/core/src/main/java/org/apache/struts2/config/entities/ActionConfig.java index b2088a269..cd0df08a5 100644 --- a/core/src/main/java/org/apache/struts2/config/entities/ActionConfig.java +++ b/core/src/main/java/org/apache/struts2/config/entities/ActionConfig.java @@ -69,6 +69,7 @@ public class ActionConfig extends Located implements Serializable { protected boolean strictMethodInvocation = true; protected AllowedMethods allowedMethods; private Set<HttpMethod> httpMethods; + private List<String> extensions; protected ActionConfig(String packageName, String name, String className) { this.packageName = packageName; @@ -98,6 +99,7 @@ public class ActionConfig extends Located implements Serializable { this.allowedMethods = orig.allowedMethods; this.httpMethods = EnumSet.noneOf(HttpMethod.class); this.httpMethods.addAll(orig.httpMethods); + this.extensions = orig.extensions != null ? List.copyOf(orig.extensions) : null; this.location = orig.location; this.strictMethodInvocation = orig.strictMethodInvocation; } @@ -130,6 +132,14 @@ public class ActionConfig extends Located implements Serializable { return httpMethods; } + /** + * @return action extensions in declared order, null when neither the action nor its package declares any + * @since 7.5.0 + */ + public List<String> getExtensions() { + return extensions; + } + /** * Returns name of the action method * @@ -200,7 +210,11 @@ public class ActionConfig extends Located implements Serializable { return false; } - return Objects.equals(httpMethods, actionConfig.httpMethods); + if (!Objects.equals(httpMethods, actionConfig.httpMethods)) { + return false; + } + + return Objects.equals(extensions, actionConfig.extensions); } @Override @@ -216,6 +230,7 @@ public class ActionConfig extends Located implements Serializable { result = 31 * result + (name != null ? name.hashCode() : 0); result = 31 * result + (allowedMethods != null ? allowedMethods.hashCode() : 0); result = 31 * result + (httpMethods != null ? httpMethods.hashCode() : 0); + result = 31 * result + (extensions != null ? extensions.hashCode() : 0); return result; } @@ -234,6 +249,9 @@ public class ActionConfig extends Located implements Serializable { if (!httpMethods.isEmpty()) { sb.append(" - ").append(httpMethods); } + if (extensions != null) { + sb.append(" - extensions ").append(extensions); + } sb.append("}"); return sb.toString(); } @@ -368,6 +386,14 @@ public class ActionConfig extends Located implements Serializable { return this; } + /** + * @since 7.5.0 + */ + public Builder extensions(List<String> extensions) { + target.extensions = extensions != null ? List.copyOf(extensions) : null; + return this; + } + public Builder location(Location loc) { target.location = loc; return this; diff --git a/core/src/main/java/org/apache/struts2/config/entities/PackageConfig.java b/core/src/main/java/org/apache/struts2/config/entities/PackageConfig.java index 88e196192..60a58d481 100644 --- a/core/src/main/java/org/apache/struts2/config/entities/PackageConfig.java +++ b/core/src/main/java/org/apache/struts2/config/entities/PackageConfig.java @@ -59,6 +59,7 @@ public class PackageConfig extends Located implements Comparable<PackageConfig>, protected boolean isFinal = false; // a final package is unextendable protected boolean needsRefresh; protected boolean strictMethodInvocation = true; + private List<String> extensions; private Map<String, String> scopedConstants; private Map<String, String> allScopedConstants; @@ -94,6 +95,7 @@ public class PackageConfig extends Located implements Comparable<PackageConfig>, this.parents = new ArrayList<>(orig.parents); this.location = orig.location; this.strictMethodInvocation = orig.strictMethodInvocation; + this.extensions = orig.extensions != null ? List.copyOf(orig.extensions) : null; this.scopedConstants = new LinkedHashMap<>(orig.scopedConstants); this.allScopedConstants = orig.allScopedConstants; } @@ -321,6 +323,31 @@ public class PackageConfig extends Located implements Comparable<PackageConfig>, return defaultResultType; } + /** + * @return extensions declared on this package, or null when it declares none + * @since 7.5.0 + */ + public List<String> getExtensions() { + return extensions; + } + + /** + * @return extensions declared on this package, else on the first parent that declares them, else null + * @since 7.5.0 + */ + public List<String> getFullExtensions() { + if (extensions != null) { + return extensions; + } + for (PackageConfig parent : parents) { + List<String> parentExtensions = parent.getFullExtensions(); + if (parentExtensions != null) { + return parentExtensions; + } + } + return null; + } + /** * gets the global ResultConfigs local to this package * @@ -422,6 +449,7 @@ public class PackageConfig extends Located implements Comparable<PackageConfig>, return false; if (!Objects.equals(defaultClassRef, that.defaultClassRef)) return false; + if (!Objects.equals(extensions, that.extensions)) return false; if (!name.equals(that.name)) return false; return Objects.equals(namespace, that.namespace); } @@ -446,6 +474,7 @@ public class PackageConfig extends Located implements Comparable<PackageConfig>, result = 31 * result + (isFinal ? 1 : 0); result = 31 * result + (needsRefresh ? 1 : 0); result = 31 * result + (strictMethodInvocation ? 1 : 0); + result = 31 * result + (extensions != null ? extensions.hashCode() : 0); return result; } @@ -676,6 +705,14 @@ public class PackageConfig extends Located implements Comparable<PackageConfig>, return this; } + /** + * @since 7.5.0 + */ + public Builder extensions(List<String> extensions) { + target.extensions = extensions != null ? List.copyOf(extensions) : null; + return this; + } + public boolean isStrictMethodInvocation() { return target.strictMethodInvocation; } diff --git a/core/src/main/java/org/apache/struts2/config/impl/ActionConfigMatcher.java b/core/src/main/java/org/apache/struts2/config/impl/ActionConfigMatcher.java index cdd7e7fa2..ce8914655 100644 --- a/core/src/main/java/org/apache/struts2/config/impl/ActionConfigMatcher.java +++ b/core/src/main/java/org/apache/struts2/config/impl/ActionConfigMatcher.java @@ -146,6 +146,7 @@ public class ActionConfigMatcher extends AbstractMatcher<ActionConfig> implement .setStrictMethodInvocation(orig.isStrictMethodInvocation()) .addAllowedMethod(orig.getAllowedMethods()) .addHttpMethods(orig.getHttpMethods()) + .extensions(orig.getExtensions()) .addInterceptors(orig.getInterceptors()) .addExceptionMappings(exs) .location(orig.getLocation()) diff --git a/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java b/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java index 7d940275f..8ee6999ba 100644 --- a/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java +++ b/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java @@ -569,7 +569,12 @@ public class DefaultConfiguration implements Configuration { LOG.debug("Using pattern [{}] to match allowed methods when SMI is disabled!", methodRegex); + List<String> extensions = baseConfig.getExtensions() != null + ? baseConfig.getExtensions() + : packageContext.getFullExtensions(); + return new ActionConfig.Builder(baseConfig) + .extensions(extensions) .addParams(params) .addResultConfigs(results) .defaultClassName(packageContext.getDefaultClassRef()) // fill in default if non class has been provided diff --git a/core/src/main/java/org/apache/struts2/config/providers/XmlDocConfigurationProvider.java b/core/src/main/java/org/apache/struts2/config/providers/XmlDocConfigurationProvider.java index 0080d97f3..34cd92ab7 100644 --- a/core/src/main/java/org/apache/struts2/config/providers/XmlDocConfigurationProvider.java +++ b/core/src/main/java/org/apache/struts2/config/providers/XmlDocConfigurationProvider.java @@ -26,6 +26,7 @@ import org.apache.struts2.config.ConfigurationException; import org.apache.struts2.config.ConfigurationProvider; import org.apache.struts2.config.ConfigurationUtil; import org.apache.struts2.config.entities.ActionConfig; +import org.apache.struts2.dispatcher.mapper.ActionExtensions; import org.apache.struts2.config.entities.ExceptionMappingConfig; import org.apache.struts2.config.entities.InterceptorConfig; import org.apache.struts2.config.entities.InterceptorMapping; @@ -503,6 +504,7 @@ public abstract class XmlDocConfigurationProvider implements ConfigurationProvid List<ExceptionMappingConfig> exceptionMappings = buildExceptionMappings(actionElement); Set<String> allowedMethods = buildAllowedMethods(actionElement, packageContext); Set<HttpMethod> httpMethods = buildHttpMethods(actionElement, location); + List<String> extensions = buildExtensions(actionElement, location); return new ActionConfig.Builder(packageContext.getName(), actionName, className) .methodName(methodName) @@ -513,6 +515,7 @@ public abstract class XmlDocConfigurationProvider implements ConfigurationProvid .setStrictMethodInvocation(packageContext.isStrictMethodInvocation()) .addAllowedMethod(allowedMethods) .addHttpMethods(httpMethods) + .extensions(extensions) .location(location) .build(); } @@ -533,6 +536,24 @@ public abstract class XmlDocConfigurationProvider implements ConfigurationProvid return httpMethods; } + /** + * @return parsed extension list, or null when the element has no extension attribute + * @since 7.5.0 + */ + protected List<String> buildExtensions(Element element, Location location) { + if (!element.hasAttribute("extension")) { + return null; + } + List<String> extensions = ActionExtensions.parse(element.getAttribute("extension")); + for (String extension : extensions) { + if (extension.startsWith(".") || extension.contains("/")) { + throw new ConfigurationException(format("Invalid extension [%s] on [%s], it must not start with '.' or contain '/'", + extension, element.getAttribute("name")), location); + } + } + return extensions; + } + protected void verifyAction(String className, Location loc) { if (className.contains("{")) { LOG.debug("Action class [{}] contains a wildcard replacement value, so it can't be verified", className); @@ -633,6 +654,7 @@ public abstract class XmlDocConfigurationProvider implements ConfigurationProvid .isAbstract(isAbstract) .isFinal(isFinal) .strictMethodInvocation(strictDMI) + .extensions(buildExtensions(packageElement, DomHelper.getLocationObject(packageElement))) .location(DomHelper.getLocationObject(packageElement)); if (parent.isEmpty()) { diff --git a/core/src/main/resources/struts-7.5.dtd b/core/src/main/resources/struts-7.5.dtd index b534fd5c8..4abfe4627 100644 --- a/core/src/main/resources/struts-7.5.dtd +++ b/core/src/main/resources/struts-7.5.dtd @@ -43,6 +43,7 @@ abstract CDATA #IMPLIED final CDATA #IMPLIED strict-method-invocation (true|false) "true" + extension CDATA #IMPLIED > <!ELEMENT scoped-constant EMPTY> @@ -106,6 +107,7 @@ method CDATA #IMPLIED converter CDATA #IMPLIED http-method CDATA #IMPLIED + extension CDATA #IMPLIED > <!ELEMENT param (#PCDATA)> diff --git a/core/src/test/java/org/apache/struts2/config/providers/XmlConfigurationProviderExtensionTest.java b/core/src/test/java/org/apache/struts2/config/providers/XmlConfigurationProviderExtensionTest.java new file mode 100644 index 000000000..4e318e57c --- /dev/null +++ b/core/src/test/java/org/apache/struts2/config/providers/XmlConfigurationProviderExtensionTest.java @@ -0,0 +1,76 @@ +/* + * 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. + */ +package org.apache.struts2.config.providers; + +import org.apache.struts2.StrutsInternalTestCase; +import org.apache.struts2.config.ConfigurationException; +import org.apache.struts2.config.StrutsXmlConfigurationProvider; +import org.apache.struts2.config.entities.ActionConfig; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +public class XmlConfigurationProviderExtensionTest extends StrutsInternalTestCase { + + private static final String CONFIG = "org/apache/struts2/config/providers/struts-extension.xml"; + private static final String INVALID_CONFIG = "org/apache/struts2/config/providers/struts-extension-invalid.xml"; + + public void testActionInheritsPackageChainExtension() { + loadConfigurationProviders(new StrutsXmlConfigurationProvider(CONFIG)); + + assertThat(actionConfig("/child", "inherits").getExtensions()).isEqualTo(List.of("html")); + } + + public void testActionOverridesPackage() { + loadConfigurationProviders(new StrutsXmlConfigurationProvider(CONFIG)); + + assertThat(actionConfig("/child", "own").getExtensions()).isEqualTo(List.of("json", "")); + } + + public void testUndeclaredIsNull() { + loadConfigurationProviders(new StrutsXmlConfigurationProvider(CONFIG)); + + assertThat(actionConfig("/plain", "undeclared").getExtensions()).isNull(); + } + + public void testEmptyAttributeMeansNoExtensionOnly() { + loadConfigurationProviders(new StrutsXmlConfigurationProvider(CONFIG)); + + assertThat(actionConfig("/plain", "none").getExtensions()).isEqualTo(List.of("")); + } + + public void testWildcardMatchCopiesExtensions() { + loadConfigurationProviders(new StrutsXmlConfigurationProvider(CONFIG)); + + assertThat(actionConfig("/wild", "edit_user").getExtensions()).isEqualTo(List.of("html")); + } + + public void testLeadingDotRejected() { + StrutsXmlConfigurationProvider provider = new StrutsXmlConfigurationProvider(INVALID_CONFIG); + assertThatThrownBy(() -> loadConfigurationProviders(provider)) + .isInstanceOf(ConfigurationException.class) + .hasStackTraceContaining(".html"); + } + + private ActionConfig actionConfig(String namespace, String name) { + return configuration.getRuntimeConfiguration().getActionConfig(namespace, name); + } +} diff --git a/core/src/test/resources/org/apache/struts2/config/providers/struts-extension-invalid.xml b/core/src/test/resources/org/apache/struts2/config/providers/struts-extension-invalid.xml new file mode 100644 index 000000000..284b026ad --- /dev/null +++ b/core/src/test/resources/org/apache/struts2/config/providers/struts-extension-invalid.xml @@ -0,0 +1,29 @@ +<?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="extension-invalid"> + <action name="Dotted" class="org.apache.struts2.TestAction" extension=".html"/> + </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 new file mode 100644 index 000000000..15b3e7fd4 --- /dev/null +++ b/core/src/test/resources/org/apache/struts2/config/providers/struts-extension.xml @@ -0,0 +1,41 @@ +<?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="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> +</struts>
