This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch feat/WW-5593-convention-plugin-noclass-exception in repository https://gitbox.apache.org/repos/asf/struts.git
commit 0cb91deb4da55f41095fb34926a83b9e8dd8accd Author: Lukasz Lenart <[email protected]> AuthorDate: Tue Dec 2 18:49:26 2025 +0100 fix(convention): WW-5593 handle NoClassDefFoundError in action class scanning The PackageBasedActionConfigBuilder now catches NoClassDefFoundError in addition to ClassNotFoundException when scanning for action classes. This prevents application startup failures when classes have missing optional dependencies (e.g., test classes depending on JUnit). Changes: - Add NoClassDefFoundError to catch block in getActionClassTest() - Improve error message to suggest missing dependencies - Add unit tests for both exception types 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> --- .../PackageBasedActionConfigBuilder.java | 5 +- .../PackageBasedActionConfigBuilderTest.java | 182 ++++++++-- ...-WW-5593-convention-plugin-noclass-exception.md | 385 +++++++++++++++++++++ 3 files changed, 538 insertions(+), 34 deletions(-) diff --git a/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java b/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java index 45eb3f8b8..f40f3e94c 100644 --- a/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java +++ b/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java @@ -661,8 +661,9 @@ public class PackageBasedActionConfigBuilder implements ActionConfigBuilder { try { return inPackage && (nameMatches || (checkImplementsAction && org.apache.struts2.action.Action.class.isAssignableFrom(classInfo.get()))); - } catch (ClassNotFoundException ex) { - LOG.error("Unable to load class [{}]", classInfo.getName(), ex); + } catch (ClassNotFoundException | NoClassDefFoundError ex) { + LOG.error("Unable to load class [{}]. Perhaps it exists but certain dependencies are not available?", + classInfo.getName(), ex); return false; } } diff --git a/plugins/convention/src/test/java/org/apache/struts2/convention/PackageBasedActionConfigBuilderTest.java b/plugins/convention/src/test/java/org/apache/struts2/convention/PackageBasedActionConfigBuilderTest.java index dcb0b4352..edffbdebc 100644 --- a/plugins/convention/src/test/java/org/apache/struts2/convention/PackageBasedActionConfigBuilderTest.java +++ b/plugins/convention/src/test/java/org/apache/struts2/convention/PackageBasedActionConfigBuilderTest.java @@ -19,6 +19,9 @@ package org.apache.struts2.convention; import org.apache.struts2.result.ActionChainResult; +import org.apache.struts2.util.finder.ClassFinder; +import org.apache.struts2.util.finder.Test; + import jakarta.servlet.ServletContext; import junit.framework.TestCase; import org.apache.commons.lang3.StringUtils; @@ -127,8 +130,122 @@ public class PackageBasedActionConfigBuilderTest extends TestCase { public void setUp() throws Exception { super.setUp(); ActionContext.of() - .withContainer(new DummyContainer()) - .bind(); + .withContainer(new DummyContainer()) + .bind(); + } + + /** + * Tests that NoClassDefFoundError is properly caught and handled + * when scanning action classes with missing dependencies. + * <p> + * This is a regression test for WW-5593: Convention plugin fails with + * NoClassDefFoundError when classes have missing optional dependencies. + * + * @see <a href="https://issues.apache.org/jira/browse/WW-5593";>WW-5593</a> + */ + public void testNoClassDefFoundErrorHandling() throws Exception { + // Setup minimal configuration + ResultTypeConfig defaultResult = new ResultTypeConfig.Builder("dispatcher", + ServletDispatcherResult.class.getName()).defaultResultParam("location").build(); + PackageConfig strutsDefault = makePackageConfig("struts-default", null, null, "dispatcher", + new ResultTypeConfig[]{defaultResult}, null, null, null, true); + + final DummyContainer mockContainer = new DummyContainer(); + Configuration configuration = new DefaultConfiguration() { + @Override + public Container getContainer() { + return mockContainer; + } + }; + configuration.addPackageConfig("struts-default", strutsDefault); + + ActionNameBuilder actionNameBuilder = new SEOActionNameBuilder("true", "-"); + ObjectFactory of = new ObjectFactory(); + of.setContainer(mockContainer); + + mockContainer.setActionNameBuilder(actionNameBuilder); + mockContainer.setConventionsService(new ConventionsServiceImpl("")); + + // Create the builder with a package that will trigger NoClassDefFoundError simulation + PackageBasedActionConfigBuilder builder = new PackageBasedActionConfigBuilder( + configuration, mockContainer, of, "false", "struts-default", "false"); + builder.setActionPackages("org.apache.struts2.convention.actions.noclass"); + builder.setActionSuffix("Action"); + + DefaultFileManagerFactory fileManagerFactory = new DefaultFileManagerFactory(); + fileManagerFactory.setContainer(ActionContext.getContext().getContainer()); + fileManagerFactory.setFileManager(new DefaultFileManager()); + builder.setFileManagerFactory(fileManagerFactory); + builder.setProviderAllowlist(new ProviderAllowlist()); + + // The getActionClassTest() method returns a Test that should catch NoClassDefFoundError + Test<ClassFinder.ClassInfo> actionClassTest = builder.getActionClassTest(); + + // Create a mock ClassInfo that throws NoClassDefFoundError when get() is called + // Note: Class name must NOT end with "Action" suffix to ensure classInfo.get() is actually called + // (otherwise nameMatches=true and the condition short-circuits without calling get()) + ClassFinder.ClassInfo mockClassInfo = EasyMock.createMock(ClassFinder.ClassInfo.class); + EasyMock.expect(mockClassInfo.getName()).andReturn("org.apache.struts2.convention.actions.noclass.MissingDependency").anyTimes(); + EasyMock.expect(mockClassInfo.get()).andThrow(new NoClassDefFoundError("junit/framework/TestCase")); + EasyMock.replay(mockClassInfo); + + // This should return false (class excluded) instead of throwing NoClassDefFoundError + boolean result = actionClassTest.test(mockClassInfo); + assertFalse("NoClassDefFoundError should be caught and class should be excluded", result); + } + + /** + * Tests that ClassNotFoundException is still properly handled. + * This is a companion test to testNoClassDefFoundErrorHandling() to ensure + * the existing ClassNotFoundException handling still works. + */ + public void testClassNotFoundExceptionHandling() throws Exception { + // Setup minimal configuration + ResultTypeConfig defaultResult = new ResultTypeConfig.Builder("dispatcher", + ServletDispatcherResult.class.getName()).defaultResultParam("location").build(); + PackageConfig strutsDefault = makePackageConfig("struts-default", null, null, "dispatcher", + new ResultTypeConfig[]{defaultResult}, null, null, null, true); + + final DummyContainer mockContainer = new DummyContainer(); + Configuration configuration = new DefaultConfiguration() { + @Override + public Container getContainer() { + return mockContainer; + } + }; + configuration.addPackageConfig("struts-default", strutsDefault); + + ActionNameBuilder actionNameBuilder = new SEOActionNameBuilder("true", "-"); + ObjectFactory of = new ObjectFactory(); + of.setContainer(mockContainer); + + mockContainer.setActionNameBuilder(actionNameBuilder); + mockContainer.setConventionsService(new ConventionsServiceImpl("")); + + PackageBasedActionConfigBuilder builder = new PackageBasedActionConfigBuilder( + configuration, mockContainer, of, "false", "struts-default", "false"); + builder.setActionPackages("org.apache.struts2.convention.actions.noclass"); + builder.setActionSuffix("Action"); + + DefaultFileManagerFactory fileManagerFactory = new DefaultFileManagerFactory(); + fileManagerFactory.setContainer(ActionContext.getContext().getContainer()); + fileManagerFactory.setFileManager(new DefaultFileManager()); + builder.setFileManagerFactory(fileManagerFactory); + builder.setProviderAllowlist(new ProviderAllowlist()); + + Test<ClassFinder.ClassInfo> actionClassTest = builder.getActionClassTest(); + + // Create a mock ClassInfo that throws ClassNotFoundException when get() is called + // Note: Class name must NOT end with "Action" suffix to ensure classInfo.get() is actually called + // (otherwise nameMatches=true and the condition short-circuits without calling get()) + ClassFinder.ClassInfo mockClassInfo = EasyMock.createMock(ClassFinder.ClassInfo.class); + EasyMock.expect(mockClassInfo.getName()).andReturn("org.apache.struts2.convention.actions.noclass.MissingClass").anyTimes(); + EasyMock.expect(mockClassInfo.get()).andThrow(new ClassNotFoundException("org.example.MissingClass")); + EasyMock.replay(mockClassInfo); + + // This should return false (class excluded) instead of throwing ClassNotFoundException + boolean result = actionClassTest.test(mockClassInfo); + assertFalse("ClassNotFoundException should be caught and class should be excluded", result); } public void testActionPackages() throws MalformedURLException { @@ -158,6 +275,7 @@ public class PackageBasedActionConfigBuilderTest extends TestCase { private void run(String actionPackages, String packageLocators, String excludePackages) throws MalformedURLException { run(actionPackages, packageLocators, excludePackages, ""); } + private void run(String actionPackages, String packageLocators, String excludePackages, String enableSmiInheritance) throws MalformedURLException { //setup interceptors List<InterceptorConfig> defaultInterceptors = new ArrayList<>(); @@ -198,7 +316,7 @@ public class PackageBasedActionConfigBuilderTest extends TestCase { PackageConfig classLevelParentPkg = makePackageConfig("class-level", null, null, null); PackageConfig rootPkg = makePackageConfig("org.apache.struts2.convention.actions#struts-default#", - "", strutsDefault, null); + "", strutsDefault, null); PackageConfig paramsPkg = makePackageConfig("org.apache.struts2.convention.actions.params#struts-default#/params", "/params", strutsDefault, null); PackageConfig defaultInterceptorPkg = makePackageConfig("org.apache.struts2.convention.actions.defaultinterceptor#struts-default#/defaultinterceptor", @@ -206,17 +324,17 @@ public class PackageBasedActionConfigBuilderTest extends TestCase { PackageConfig exceptionPkg = makePackageConfig("org.apache.struts2.convention.actions.exception#struts-default#/exception", "/exception", strutsDefault, null); PackageConfig actionPkg = makePackageConfig("org.apache.struts2.convention.actions.action#struts-default#/action", - "/action", strutsDefault, null); + "/action", strutsDefault, null); PackageConfig idxPkg = makePackageConfig("org.apache.struts2.convention.actions.idx#struts-default#/idx", - "/idx", strutsDefault, null); + "/idx", strutsDefault, null); PackageConfig idx2Pkg = makePackageConfig("org.apache.struts2.convention.actions.idx.idx2#struts-default#/idx/idx2", - "/idx/idx2", strutsDefault, null); + "/idx/idx2", strutsDefault, null); PackageConfig interceptorRefsPkg = makePackageConfig("org.apache.struts2.convention.actions.interceptor#struts-default#/interceptor", "/interceptor", strutsDefault, null); PackageConfig packageLevelPkg = makePackageConfig("org.apache.struts2.convention.actions.parentpackage#package-level#/parentpackage", - "/parentpackage", packageLevelParentPkg, null); + "/parentpackage", packageLevelParentPkg, null); PackageConfig packageLevelSubPkg = makePackageConfig("org.apache.struts2.convention.actions.parentpackage.sub#package-level#/parentpackage/sub", - "/parentpackage/sub", packageLevelParentPkg, null); + "/parentpackage/sub", packageLevelParentPkg, null); // Unexpected method call build(class org.apache.struts2.convention.actions.allowedmethods.PackageLevelAllowedMethodsAction, null, "package-level-allowed-methods", PackageConfig: [org.apache.struts2.convention.actions.allowedmethods#struts-default#/allowedmethods] for namespace [/allowedmethods] with parents [[PackageConfig: [struts-default] for namespace [] with parents [[]]]]): PackageConfig packageLevelAllowedMethodsPkg = makePackageConfig("org.apache.struts2.convention.actions.allowedmethods#struts-default#/allowedmethods", @@ -228,17 +346,17 @@ public class PackageBasedActionConfigBuilderTest extends TestCase { "/allowedmethods", strutsDefault, null); PackageConfig differentPkg = makePackageConfig("org.apache.struts2.convention.actions.parentpackage#class-level#/parentpackage", - "/parentpackage", classLevelParentPkg, null); + "/parentpackage", classLevelParentPkg, null); PackageConfig differentSubPkg = makePackageConfig("org.apache.struts2.convention.actions.parentpackage.sub#class-level#/parentpackage/sub", - "/parentpackage/sub", classLevelParentPkg, null); + "/parentpackage/sub", classLevelParentPkg, null); PackageConfig pkgLevelNamespacePkg = makePackageConfig("org.apache.struts2.convention.actions.namespace#struts-default#/package-level", - "/package-level", strutsDefault, null); + "/package-level", strutsDefault, null); PackageConfig classLevelNamespacePkg = makePackageConfig("org.apache.struts2.convention.actions.namespace#struts-default#/class-level", - "/class-level", strutsDefault, null); + "/class-level", strutsDefault, null); PackageConfig actionLevelNamespacePkg = makePackageConfig("org.apache.struts2.convention.actions.namespace#struts-default#/action-level", - "/action-level", strutsDefault, null); + "/action-level", strutsDefault, null); PackageConfig defaultNamespacePkg = makePackageConfig("org.apache.struts2.convention.actions.namespace2#struts-default#/namespace2", - "/namespace2", strutsDefault, null); + "/namespace2", strutsDefault, null); PackageConfig namespaces1Pkg = makePackageConfig("org.apache.struts2.convention.actions.namespace3#struts-default#/namespaces1", "/namespaces1", strutsDefault, null); PackageConfig namespaces2Pkg = makePackageConfig("org.apache.struts2.convention.actions.namespace3#struts-default#/namespaces2", @@ -248,19 +366,19 @@ public class PackageBasedActionConfigBuilderTest extends TestCase { PackageConfig namespaces4Pkg = makePackageConfig("org.apache.struts2.convention.actions.namespace4#struts-default#/namespaces4", "/namespaces4", strutsDefault, null); PackageConfig resultPkg = makePackageConfig("org.apache.struts2.convention.actions.result#struts-default#/result", - "/result", strutsDefault, null); + "/result", strutsDefault, null); PackageConfig globalResultPkg = makePackageConfig("org.apache.struts2.convention.actions.result#class-level#/result", "/result", classLevelParentPkg, null); PackageConfig resultPathPkg = makePackageConfig("org.apache.struts2.convention.actions.resultpath#struts-default#/resultpath", - "/resultpath", strutsDefault, null); + "/resultpath", strutsDefault, null); PackageConfig skipPkg = makePackageConfig("org.apache.struts2.convention.actions.skip#struts-default#/skip", - "/skip", strutsDefault, null); + "/skip", strutsDefault, null); PackageConfig chainPkg = makePackageConfig("org.apache.struts2.convention.actions.chain#struts-default#/chain", - "/chain", strutsDefault, null); + "/chain", strutsDefault, null); PackageConfig transPkg = makePackageConfig("org.apache.struts2.convention.actions.transactions#struts-default#/transactions", - "/transactions", strutsDefault, null); + "/transactions", strutsDefault, null); PackageConfig excludePkg = makePackageConfig("org.apache.struts2.convention.actions.exclude#struts-default#/exclude", - "/exclude", strutsDefault, null); + "/exclude", strutsDefault, null); ResultMapBuilder resultMapBuilder = createStrictMock(ResultMapBuilder.class); checkOrder(resultMapBuilder, false); @@ -409,7 +527,7 @@ public class PackageBasedActionConfigBuilderTest extends TestCase { mockContainer.setResultMapBuilder(resultMapBuilder); mockContainer.setConventionsService(new ConventionsServiceImpl("")); - PackageBasedActionConfigBuilder builder = new PackageBasedActionConfigBuilder(configuration, mockContainer , of, "false", "struts-default", enableSmiInheritance); + PackageBasedActionConfigBuilder builder = new PackageBasedActionConfigBuilder(configuration, mockContainer, of, "false", "struts-default", enableSmiInheritance); builder.setFileProtocols("jar"); if (actionPackages != null) { builder.setActionPackages(actionPackages); @@ -706,7 +824,7 @@ public class PackageBasedActionConfigBuilderTest extends TestCase { verifyActionConfig(pkgConfig, "default-result-path", DefaultResultPathAction.class, "execute", pkgConfig.getName()); verifyActionConfig(pkgConfig, "skip", Skip.class, "execute", pkgConfig.getName()); verifyActionConfig(pkgConfig, "idx", org.apache.struts2.convention.actions.idx.Index.class, "execute", - "org.apache.struts2.convention.actions.idx#struts-default#/idx"); + "org.apache.struts2.convention.actions.idx#struts-default#/idx"); /* org.apache.struts2.convention.actions.transactions */ pkgConfig = configuration.getPackageConfig("org.apache.struts2.convention.actions.transactions#struts-default#/transactions"); @@ -741,7 +859,7 @@ public class PackageBasedActionConfigBuilderTest extends TestCase { } private void verifyActionConfig(PackageConfig pkgConfig, String actionName, Class<?> actionClass, - String methodName, String packageName) { + String methodName, String packageName) { ActionConfig ac = pkgConfig.getAllActionConfigs().get(actionName); assertNotNull(ac); assertEquals(actionClass.getName(), ac.getClassName()); @@ -756,7 +874,7 @@ public class PackageBasedActionConfigBuilderTest extends TestCase { } private void verifyMissingActionConfig(PackageConfig pkgConfig, String actionName, Class<?> actionClass, - String methodName, String packageName) { + String methodName, String packageName) { ActionConfig ac = pkgConfig.getAllActionConfigs().get(actionName); assertNull(ac); } @@ -769,10 +887,10 @@ public class PackageBasedActionConfigBuilderTest extends TestCase { assertEquals(packageName, ac.getPackageName()); } - private void checkSmiValue(PackageConfig pkgConfig, PackageConfig parentConfig, boolean isSmiInheritanceEnabled) { + private void checkSmiValue(PackageConfig pkgConfig, PackageConfig parentConfig, boolean isSmiInheritanceEnabled) { if (isSmiInheritanceEnabled) { assertEquals(parentConfig.isStrictMethodInvocation(), pkgConfig.isStrictMethodInvocation()); - } else if (!isSmiInheritanceEnabled && !parentConfig.isStrictMethodInvocation()){ + } else if (!isSmiInheritanceEnabled && !parentConfig.isStrictMethodInvocation()) { assertTrue(pkgConfig.isStrictMethodInvocation()); } } @@ -788,13 +906,13 @@ public class PackageBasedActionConfigBuilderTest extends TestCase { } private PackageConfig makePackageConfig(String name, String namespace, PackageConfig parent, - String defaultResultType, ResultTypeConfig... results) { + String defaultResultType, ResultTypeConfig... results) { return makePackageConfig(name, namespace, parent, defaultResultType, results, null, null, null, true); } private PackageConfig makePackageConfig(String name, String namespace, PackageConfig parent, - String defaultResultType, ResultTypeConfig[] results, List<InterceptorConfig> interceptors, - List<InterceptorStackConfig> interceptorStacks, Set<String> globalAllowedMethods, boolean strictMethodInvocation) { + String defaultResultType, ResultTypeConfig[] results, List<InterceptorConfig> interceptors, + List<InterceptorStackConfig> interceptorStacks, Set<String> globalAllowedMethods, boolean strictMethodInvocation) { PackageConfig.Builder builder = new PackageConfig.Builder(name); if (namespace != null) { builder.namespace(namespace); @@ -852,7 +970,7 @@ public class PackageBasedActionConfigBuilderTest extends TestCase { public boolean equals(Object obj) { PackageConfig other = (PackageConfig) obj; return getName().equals(other.getName()) && getNamespace().equals(other.getNamespace()) && - getParents().get(0) == other.getParents().get(0) && getParents().size() == other.getParents().size(); + getParents().get(0) == other.getParents().get(0) && getParents().size() == other.getParents().size(); } } @@ -893,7 +1011,7 @@ public class PackageBasedActionConfigBuilderTest extends TestCase { T obj; if (type == ObjectFactory.class) { obj = type.getConstructor().newInstance(); - ((ObjectFactory)obj).setContainer(this); + ((ObjectFactory) obj).setContainer(this); OgnlReflectionProvider rp = new OgnlReflectionProvider() { @@ -929,7 +1047,7 @@ public class PackageBasedActionConfigBuilderTest extends TestCase { } return obj; } catch (Exception e) { - throw new RuntimeException(e); + throw new RuntimeException(e); } } diff --git a/thoughts/shared/research/2025-12-02-WW-5593-convention-plugin-noclass-exception.md b/thoughts/shared/research/2025-12-02-WW-5593-convention-plugin-noclass-exception.md new file mode 100644 index 000000000..b4c7b4d14 --- /dev/null +++ b/thoughts/shared/research/2025-12-02-WW-5593-convention-plugin-noclass-exception.md @@ -0,0 +1,385 @@ +--- +date: 2025-12-02T18:33:08+01:00 +topic: "WW-5593: Convention plugin fails with NoClassDefFoundError" +tags: [research, codebase, convention-plugin, exception-handling, bug-analysis, WW-5593] +status: complete +jira_ticket: WW-5593 +related_tickets: [WW-5594] +--- + +# Research: WW-5593 - Convention Plugin NoClassDefFoundError Handling + +**Date**: 2025-12-02T18:33:08+01:00 +**JIRA**: [WW-5593](https://issues.apache.org/jira/browse/WW-5593) +**Related**: [WW-5594](https://issues.apache.org/jira/browse/WW-5594) + +## Research Question + +Why does the convention plugin fail completely with `NoClassDefFoundError` when scanning classes with missing dependencies, instead of gracefully logging and continuing? + +## Summary + +The `PackageBasedActionConfigBuilder` class (line 664) only catches `ClassNotFoundException` but not `NoClassDefFoundError` when testing action class candidates during classpath scanning. This causes complete application startup failures instead of graceful degradation when classes have missing optional dependencies. + +The issue is triggered when `struts.convention.action.includeJars` is configured, causing the plugin to scan struts2-core.jar which contains `XWorkTestCase` - a test utility class depending on JUnit (an optional dependency). + +## Detailed Findings + +### 1. The Exception Handling Bug + +**Location**: `plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java:662-667` + +**Current Code**: +```java +try { + return inPackage && (nameMatches || (checkImplementsAction && org.apache.struts2.action.Action.class.isAssignableFrom(classInfo.get()))); +} catch (ClassNotFoundException ex) { + LOG.error("Unable to load class [{}]", classInfo.getName(), ex); + return false; +} +``` + +**Problem**: Only catches `ClassNotFoundException`, missing `NoClassDefFoundError`. + +### 2. Exception Hierarchy + +``` +java.lang.Throwable + ├─ java.lang.Exception + │ └─ java.lang.ClassNotFoundException + └─ java.lang.Error + └─ java.lang.LinkageError + └─ java.lang.NoClassDefFoundError +``` + +**Key Point**: `NoClassDefFoundError` and `ClassNotFoundException` are siblings under `Throwable`, not parent-child. You cannot catch `NoClassDefFoundError` by catching `ClassNotFoundException`. + +**Difference**: +- **ClassNotFoundException**: Thrown when `Class.forName()` or `ClassLoader.loadClass()` cannot find the class definition +- **NoClassDefFoundError**: Thrown when a class was found at compile time but cannot be loaded at runtime due to: + - Missing dependencies (JAR files) + - Class initialization failures + - Static initializer blocks throw exceptions + - Incompatible class versions + - Classloader isolation issues + +### 3. Where NoClassDefFoundError Originates + +**Location**: `core/src/main/java/org/apache/struts2/util/finder/ClassFinder.java:224-235` + +```java +public Class<?> get() throws ClassNotFoundException { + if (clazz != null) return clazz; + if (notFound != null) throw notFound; + try { + this.clazz = classFinder.getClassLoaderInterface().loadClass(name); + return clazz; + } catch (ClassNotFoundException notFound) { + classFinder.getClassesNotLoaded().add(name); + this.notFound = notFound; + throw notFound; + } +} +``` + +**Analysis**: +- The method signature declares `throws ClassNotFoundException` +- The catch block only handles `ClassNotFoundException` +- However, `ClassLoader.loadClass()` can also throw `NoClassDefFoundError` when class dependencies are missing +- This `NoClassDefFoundError` propagates uncaught through `ClassInfo.get()` + +### 4. Call Stack During Failure + +From the email thread error log: + +``` +ERROR [org.apache.struts2.convention.DefaultClassFinder] (default task-1) +Error loading class [org.apache.struts2.XWorkTestCase]: +java.lang.NoClassDefFoundError: Failed to link org/apache/struts2/XWorkTestCase +(Module "deployment.coreweb.war" from Service Module Loader): junit/framework/TestCase + at java.base/java.lang.ClassLoader.defineClass1(Native Method) + at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1017) + ... + at deployment.coreweb.war//org.apache.struts2.util.finder.ClassFinder$ClassInfo.get(ClassFinder.java:228) + at deployment.coreweb.war//org.apache.struts2.convention.PackageBasedActionConfigBuilder$1.test(PackageBasedActionConfigBuilder.java:6XX) + at deployment.coreweb.war//org.apache.struts2.convention.DefaultClassFinder.findClasses(DefaultClassFinder.java:280) +``` + +The error occurs when: +1. Convention plugin scans for action classes +2. Finds `org.apache.struts2.XWorkTestCase` as a candidate +3. Calls `classInfo.get()` to load the class +4. ClassLoader attempts to link the class and load `junit.framework.TestCase` +5. JUnit is not available at runtime (optional dependency) +6. `NoClassDefFoundError` is thrown +7. **Not caught** by `PackageBasedActionConfigBuilder` catch block +8. Application startup fails completely + +### 5. Correct Exception Handling Patterns in Struts + +The Struts codebase already uses correct patterns in other locations: + +#### Pattern A: Catch Throwable (Most Defensive) + +**Location**: `plugins/convention/src/main/java/org/apache/struts2/convention/DefaultClassFinder.java:283-286` + +```java +} catch (Throwable e) { + LOG.error("Error loading class [{}]", classInfo.getName(), e); + classesNotLoaded.add(classInfo.getName()); +} +``` + +**Also used at**: Lines 248-251, 267-270, 297-300 + +**Advantage**: Catches all exceptions and errors, including: +- `ClassNotFoundException` +- `NoClassDefFoundError` +- `LinkageError` +- `ExceptionInInitializerError` +- Any other unexpected errors + +#### Pattern B: Multi-Catch with Specific Exceptions + +**Location**: `core/src/main/java/org/apache/struts2/config/providers/XmlDocConfigurationProvider.java:580-582` + +```java +} catch (ClassNotFoundException | NoClassDefFoundError e) { + throw new ConfigurationException("Result class [" + className + "] not found", e, loc); +} +``` + +**Location**: `core/src/main/java/org/apache/struts2/factory/DefaultInterceptorFactory.java:91-93` + +```java +} catch (NoClassDefFoundError e) { + cause = e; + message = "Could not load class " + interceptorClassName + ". Perhaps it exists but certain dependencies are not available?"; +} +``` + +**Advantage**: Explicit about what exceptions are expected, provides better error messages + +#### Pattern C: Insufficient (Current Bug) + +**Location**: `plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java:664` + +```java +} catch (ClassNotFoundException ex) { + LOG.error("Unable to load class [{}]", classInfo.getName(), ex); + return false; +} +``` + +**Problem**: Misses `NoClassDefFoundError` and other linkage errors + +### 6. When NoClassDefFoundError Occurs + +**Common Scenarios**: +1. **Application redeployment** (as mentioned in related research) +2. **Optional dependencies missing** (like JUnit for test classes) +3. **Classloader isolation** in application servers (JBoss, WebSphere, etc.) +4. **Static initializer failures** in class being loaded +5. **Split package issues** in modular systems +6. **Incompatible class versions** between dependencies + +### 7. Trigger Condition + +**From Email Thread**: Setting this constant triggers the bug: + +```xml +<constant name="struts.convention.action.includeJars" + value=".*?/myjar.*?jar(!/)?" /> +``` + +**Why**: +- By default, convention plugin only scans application's own classes +- Setting `struts.convention.action.includeJars` causes scanning of JAR files +- This includes struts2-core.jar itself +- struts2-core.jar contains `org.apache.struts2.XWorkTestCase` +- `XWorkTestCase` depends on `junit.framework.TestCase` +- JUnit is optional (scope: test) +- At runtime, JUnit not available → `NoClassDefFoundError` + +## Code References + +- `plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java:664` - Bug location +- `core/src/main/java/org/apache/struts2/util/finder/ClassFinder.java:228` - Where NoClassDefFoundError originates +- `plugins/convention/src/main/java/org/apache/struts2/convention/DefaultClassFinder.java:283-286` - Correct pattern (catches Throwable) +- `core/src/main/java/org/apache/struts2/factory/DefaultInterceptorFactory.java:91-93` - Precedent for NoClassDefFoundError handling +- `core/src/main/java/org/apache/struts2/config/providers/XmlDocConfigurationProvider.java:580-582` - Multi-catch pattern + +## Recommended Fix + +### Option A: Multi-Catch (Recommended) + +**File**: `plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java:664` + +```java +} catch (ClassNotFoundException | NoClassDefFoundError ex) { + LOG.error("Unable to load class [{}]. Perhaps it exists but certain dependencies are not available?", + classInfo.getName(), ex); + return false; +} +``` + +**Advantages**: +- Explicit about expected exceptions +- Follows pattern from `XmlDocConfigurationProvider` +- Provides helpful error message +- Java 7+ multi-catch syntax + +### Option B: Catch Throwable (More Defensive) + +```java +} catch (Throwable ex) { + LOG.error("Unable to load class [{}]", classInfo.getName(), ex); + return false; +} +``` + +**Advantages**: +- Matches pattern in `DefaultClassFinder` +- Handles any unexpected errors +- Most defensive approach + +**Disadvantages**: +- Less specific +- Could mask programming errors + +### Recommendation + +Use **Option A** (multi-catch) because: +1. It's explicit about the expected error conditions +2. Follows established pattern in `XmlDocConfigurationProvider` +3. Provides better error message for users +4. Still maintains good defensive programming + +### Additional Improvements + +Consider adding to the log message: +```java +LOG.error("Unable to load class [{}]. Perhaps it exists but certain dependencies are not available? " + + "If this is a test class or from a library, consider adding it to struts.convention.exclude.packages", + classInfo.getName(), ex); +``` + +## Impact Assessment + +### Current Risk + +**Severity**: HIGH + +**Scenarios Affected**: +1. Applications using `struts.convention.action.includeJars` +2. Applications with optional dependencies on scanned classes +3. Redeployment scenarios with classloader issues +4. Application server environments with module isolation (JBoss, WebSphere) + +**Current Behavior**: +- ❌ Complete application startup failure +- ❌ No graceful degradation +- ❌ Misleading error (appears critical, not missing optional dependency) +- ❌ Users must add workarounds to proceed + +### After Fix + +**Expected Behavior**: +- ✅ Graceful degradation - problematic classes logged and skipped +- ✅ Application continues to function +- ✅ Only unavailable actions are affected, not entire app +- ✅ Clear error messages indicating dependency issues +- ✅ Consistent with `DefaultClassFinder` behavior + +## Testing Strategy + +### Unit Tests to Add + +1. **Test NoClassDefFoundError Handling**: +```java +@Test +public void testHandlesNoClassDefFoundError() { + // Mock a ClassInfo that throws NoClassDefFoundError + ClassInfo classInfo = mock(ClassInfo.class); + when(classInfo.get()).thenThrow(new NoClassDefFoundError("junit/framework/TestCase")); + + // Should return false (exclude class) instead of propagating error + assertFalse(builder.includeClassNameInActionScan("org.apache.struts2.XWorkTestCase")); + + // Should log error + verify(mockLogger).error(contains("Unable to load class"), any(NoClassDefFoundError.class)); +} +``` + +2. **Test ClassNotFoundException Handling** (existing behavior): +```java +@Test +public void testHandlesClassNotFoundException() { + ClassInfo classInfo = mock(ClassInfo.class); + when(classInfo.get()).thenThrow(new ClassNotFoundException("com.example.MissingClass")); + + assertFalse(builder.includeClassNameInActionScan("com.example.MissingClass")); + verify(mockLogger).error(contains("Unable to load class"), any(ClassNotFoundException.class)); +} +``` + +3. **Test ExceptionInInitializerError** (edge case): +```java +@Test +public void testHandlesExceptionInInitializerError() { + ClassInfo classInfo = mock(ClassInfo.class); + when(classInfo.get()).thenThrow(new ExceptionInInitializerError("Static init failed")); + + assertFalse(builder.includeClassNameInActionScan("com.example.BadStaticInit")); +} +``` + +### Integration Tests + +1. Create a test JAR with a class that has missing dependencies +2. Configure `struts.convention.action.includeJars` to scan that JAR +3. Verify application starts successfully despite the problematic class +4. Verify error is logged appropriately + +## Workaround for Users + +Until the fix is released, users can: + +**Option 1**: Exclude problematic packages: +```xml +<constant name="struts.convention.exclude.packages" + value="org.apache.struts2,org.apache.struts2.*" /> +``` + +**Option 2**: Include JUnit at runtime (not recommended): +```xml +<dependency> + <groupId>junit</groupId> + <artifactId>junit</artifactId> + <version>4.13.2</version> + <scope>runtime</scope> +</dependency> +``` + +**Option 3**: Don't use `struts.convention.action.includeJars` if not needed + +## Related Issues + +- **WW-5594**: Wildcard pattern matching issue preventing proper exclusion of org.apache.struts2 package +- Related research: `thoughts/shared/research/2025-12-02-memory-leak-redeployment.md` (redeployment issues) + +## Email Thread Reference + +**From**: Florian Schlittgen ([email protected]) +**Date**: December 19, 2024 - January 19, 2025 +**Subject**: Struts 7: action class finder +**List**: [email protected] + +## Next Steps + +1. ✅ Create JIRA ticket WW-5593 +2. ⏳ Implement fix in `PackageBasedActionConfigBuilder.java:664` +3. ⏳ Add unit tests for NoClassDefFoundError handling +4. ⏳ Add integration tests with missing dependencies +5. ⏳ Update any other catch blocks with same issue +6. ⏳ Review release notes to document the fix \ No newline at end of file
