This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch WW-5791-struts-boot in repository https://gitbox.apache.org/repos/asf/struts.git
commit 914d7e7bab228dbd10ee15a1b9311501d3c138c4 Author: Lukasz Lenart <[email protected]> AuthorDate: Fri Oct 23 13:16:24 2026 +0200 WW-5791 fix(boot): verify action classes and registry arguments at startup Mirror XmlDocConfigurationProvider#verifyAction: a non-public action class or one without a public no-arg constructor fails startup. A null action class or an empty redirectAction target is rejected with a ConfigurationException. Co-Authored-By: Claude Opus 5.5 <[email protected]> --- .../org/apache/struts2/boot/ActionDefinition.java | 4 ++ .../org/apache/struts2/boot/NamespaceActions.java | 3 ++ .../boot/RegistryConfigurationProvider.java | 18 ++++++++ .../apache/struts2/boot/ActionRegistryTest.java | 16 +++++++ .../boot/RegistryConfigurationProviderTest.java | 49 ++++++++++++++++++++++ 5 files changed, 90 insertions(+) diff --git a/boot/src/main/java/org/apache/struts2/boot/ActionDefinition.java b/boot/src/main/java/org/apache/struts2/boot/ActionDefinition.java index d287e5ca5..3b9c1830f 100644 --- a/boot/src/main/java/org/apache/struts2/boot/ActionDefinition.java +++ b/boot/src/main/java/org/apache/struts2/boot/ActionDefinition.java @@ -68,6 +68,10 @@ public final class ActionDefinition { } public ActionDefinition redirectAction(String resultName, String actionName) { + if (actionName == null || actionName.isEmpty()) { + throw new ConfigurationException("Action '" + name + "' result '" + resultName + + "' redirects to an action, so the target action name must not be empty"); + } results.put(resultName, new ResultDefinition(ResultDefinition.Kind.REDIRECT_ACTION, actionName)); return this; } diff --git a/boot/src/main/java/org/apache/struts2/boot/NamespaceActions.java b/boot/src/main/java/org/apache/struts2/boot/NamespaceActions.java index d18084d2a..39b418d92 100644 --- a/boot/src/main/java/org/apache/struts2/boot/NamespaceActions.java +++ b/boot/src/main/java/org/apache/struts2/boot/NamespaceActions.java @@ -42,6 +42,9 @@ public final class NamespaceActions { if (name.contains("/")) { throw new ConfigurationException("Action name '" + name + "' contains a slash; declare the path with namespace(...) instead"); } + if (actionClass == null) { + throw new ConfigurationException("Action '" + name + "' in namespace '" + namespace + "' has no action class"); + } for (ActionDefinition existing : actions) { if (existing.name().equals(name)) { throw new ConfigurationException("Action '" + name + "' is declared twice in namespace '" + namespace + "'"); diff --git a/boot/src/main/java/org/apache/struts2/boot/RegistryConfigurationProvider.java b/boot/src/main/java/org/apache/struts2/boot/RegistryConfigurationProvider.java index ccfb87ff1..699525210 100644 --- a/boot/src/main/java/org/apache/struts2/boot/RegistryConfigurationProvider.java +++ b/boot/src/main/java/org/apache/struts2/boot/RegistryConfigurationProvider.java @@ -33,6 +33,7 @@ import org.apache.struts2.ognl.ProviderAllowlist; import org.apache.struts2.util.ClassLoaderUtil; import org.apache.struts2.util.location.LocatableProperties; +import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.HashSet; import java.util.List; @@ -104,6 +105,9 @@ public class RegistryConfigurationProvider implements ServletContextAwareConfigu for (ActionDefinition definition : namespace.actions()) { assertNoClash(existing, namespace.namespace(), definition.name()); + if (objectFactory.isNoArgConstructorRequired()) { + verifyActionClass(namespace.namespace(), definition); + } allowlist.addAll(ConfigurationUtil.getAllClassTypes(definition.actionClass())); pkg.addActionConfig(definition.name(), actionConfig(packageName, namespace, definition, pkg, objectFactory, freemarker, redirectAction)); @@ -149,6 +153,20 @@ public class RegistryConfigurationProvider implements ServletContextAwareConfigu }; } + private static void verifyActionClass(String namespace, ActionDefinition definition) { + Class<?> actionClass = definition.actionClass(); + String subject = "Action class [" + actionClass.getName() + "] of action '" + definition.name() + + "' in namespace '" + namespace + "'"; + if (!Modifier.isPublic(actionClass.getModifiers())) { + throw new ConfigurationException(subject + " is not public"); + } + try { + actionClass.getConstructor(); + } catch (NoSuchMethodException e) { + throw new ConfigurationException(subject + " does not have a public no-arg constructor", e); + } + } + private void assertNoClash(List<PackageConfig> existing, String namespace, String actionName) { for (PackageConfig other : existing) { if (!other.isAbstract() && namespace.equals(other.getNamespace()) diff --git a/boot/src/test/java/org/apache/struts2/boot/ActionRegistryTest.java b/boot/src/test/java/org/apache/struts2/boot/ActionRegistryTest.java index 6e552ac1b..06e7d067f 100644 --- a/boot/src/test/java/org/apache/struts2/boot/ActionRegistryTest.java +++ b/boot/src/test/java/org/apache/struts2/boot/ActionRegistryTest.java @@ -78,6 +78,22 @@ public class ActionRegistryTest { .isInstanceOf(ConfigurationException.class).hasMessageContaining("FreeMarker"); } + @Test + public void testNullActionClassThrows() { + ActionRegistry r = new ActionRegistry(); + assertThatThrownBy(() -> r.action("hello", null)) + .isInstanceOf(ConfigurationException.class).hasMessageContaining("hello").hasMessageContaining("class"); + } + + @Test + public void testRedirectActionWithoutTargetThrows() { + ActionDefinition a = new ActionRegistry().action("save", Object.class); + assertThatThrownBy(() -> a.redirectAction("success", null)) + .isInstanceOf(ConfigurationException.class).hasMessageContaining("save").hasMessageContaining("success"); + assertThatThrownBy(() -> a.redirectAction("success", "")) + .isInstanceOf(ConfigurationException.class).hasMessageContaining("save").hasMessageContaining("success"); + } + @Test public void testFluentCallsAreRecorded() { ActionDefinition a = new ActionRegistry().action("save", Object.class) diff --git a/boot/src/test/java/org/apache/struts2/boot/RegistryConfigurationProviderTest.java b/boot/src/test/java/org/apache/struts2/boot/RegistryConfigurationProviderTest.java index f0683b173..c19342c44 100644 --- a/boot/src/test/java/org/apache/struts2/boot/RegistryConfigurationProviderTest.java +++ b/boot/src/test/java/org/apache/struts2/boot/RegistryConfigurationProviderTest.java @@ -34,6 +34,9 @@ import org.apache.struts2.ActionContext; import org.junit.After; import org.junit.Test; +import java.util.ArrayList; +import java.util.List; + import static org.apache.struts2.boot.HttpMethods.POST; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -195,6 +198,52 @@ public class RegistryConfigurationProviderTest { .rootCause().hasMessageContaining("templates/missing.ftl"); } + @Test + public void testNonPublicActionClassThrows() { + ActionRegistry registry = new ActionRegistry(); + registry.namespace("/admin", ns -> ns.action("hidden", NonPublicAction.class).success("admin/users.ftl")); + + assertThatThrownBy(() -> load(registry)) + .isInstanceOf(ConfigurationException.class) + .rootCause().hasMessageContaining(NonPublicAction.class.getName()) + .hasMessageContaining("hidden").hasMessageContaining("not public"); + } + + @Test + public void testActionClassWithoutNoArgConstructorThrows() { + ActionRegistry registry = new ActionRegistry(); + registry.namespace("/admin", ns -> ns.action("needsArgs", NoDefaultConstructorAction.class).success("admin/users.ftl")); + + assertThatThrownBy(() -> load(registry)) + .isInstanceOf(ConfigurationException.class) + .satisfies(e -> assertThat(messagesOf(e)).anyMatch(message -> + message.contains(NoDefaultConstructorAction.class.getName()) + && message.contains("needsArgs") && message.contains("no-arg constructor"))); + } + + private static List<String> messagesOf(Throwable throwable) { + List<String> messages = new ArrayList<>(); + for (Throwable current = throwable; current != null; current = current.getCause()) { + messages.add(String.valueOf(current.getMessage())); + } + return messages; + } + + static class NonPublicAction { + public String execute() { + return "success"; + } + } + + public static class NoDefaultConstructorAction { + public NoDefaultConstructorAction(String name) { + } + + public String execute() { + return "success"; + } + } + @Test public void testClashWithXmlThrows() { assertThatThrownBy(() -> load(usersRegistry(), "struts-boot-clash.xml"))
