This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch feat/WW-5594-wildcard-exclusion-pattern in repository https://gitbox.apache.org/repos/asf/struts.git
commit d24b5206a88919622bbee40e495627f79dbe03e3 Author: Lukasz Lenart <[email protected]> AuthorDate: Tue Dec 2 18:49:51 2025 +0100 fix(convention): WW-5594 exclude root package classes with wildcard patterns The exclusion pattern "org.apache.struts2.*" was not properly excluding classes directly in the root package (like XWorkTestCase) because: 1. PackageBasedActionConfigBuilder extracts package names using substringBeforeLast(className, ".") which produces "org.apache.struts2" (no trailing dot) 2. The wildcard pattern requires a literal "." before "*" 3. Result: Pattern doesn't match root package classes Fix: Enhanced checkExcludePackages() to automatically handle patterns ending with ".*" by also checking if the package name equals the base pattern (without ".*"). Now "org.apache.struts2.*" properly excludes both: - Classes in root package: org.apache.struts2.XWorkTestCase - Classes in subpackages: org.apache.struts2.dispatcher.SomeClass Closes [WW-5594](https://issues.apache.org/jira/browse/WW-5594) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> --- .../apache/struts2/util/WildcardHelperTest.java | 53 +++ .../PackageBasedActionConfigBuilder.java | 31 +- .../src/main/resources/struts-plugin.xml | 86 ++-- .../PackageBasedActionConfigBuilderTest.java | 140 +++++-- ...025-12-02-WW-5594-wildcard-exclusion-pattern.md | 460 +++++++++++++++++++++ 5 files changed, 693 insertions(+), 77 deletions(-) diff --git a/core/src/test/java/org/apache/struts2/util/WildcardHelperTest.java b/core/src/test/java/org/apache/struts2/util/WildcardHelperTest.java index e856bfce8..6312b31a2 100644 --- a/core/src/test/java/org/apache/struts2/util/WildcardHelperTest.java +++ b/core/src/test/java/org/apache/struts2/util/WildcardHelperTest.java @@ -80,4 +80,57 @@ public class WildcardHelperTest extends XWorkTestCase { assertEquals("", matchedPatterns.get("1")); } + /** + * WW-5594: Tests that pattern "org.apache.struts2.*" does NOT match "org.apache.struts2" + * (package name without trailing dot). + * <p> + * This is important because when checking exclusion patterns, the convention plugin + * extracts package names from class names using StringUtils.substringBeforeLast(className, "."), + * which produces package names without trailing dots. + * <p> + * For example: + * - Class "org.apache.struts2.XWorkTestCase" -> package "org.apache.struts2" (no trailing dot) + * - Pattern "org.apache.struts2.*" requires a literal "." before the wildcard + * - Result: Pattern doesn't match, class is not excluded + * <p> + * The fix is to include both "org.apache.struts2" and "org.apache.struts2.*" in exclusion patterns. + */ + public void testWW5594_WildcardPatternRequiresTrailingDot() { + // given + HashMap<String, String> matchedPatterns = new HashMap<>(); + int[] wildcardPattern = wildcardHelper.compilePattern("org.apache.struts2.*"); + + // when & then - Pattern with wildcard does NOT match package name without trailing dot + // This is the root cause of WW-5594 + assertFalse("Pattern 'org.apache.struts2.*' should NOT match 'org.apache.struts2' (no trailing dot)", + wildcardHelper.match(matchedPatterns, "org.apache.struts2", wildcardPattern)); + + // But it DOES match with trailing dot + assertTrue("Pattern 'org.apache.struts2.*' should match 'org.apache.struts2.' (with trailing dot)", + wildcardHelper.match(matchedPatterns, "org.apache.struts2.", wildcardPattern)); + + // And it DOES match full class names + assertTrue("Pattern 'org.apache.struts2.*' should match 'org.apache.struts2.SomeClass'", + wildcardHelper.match(matchedPatterns, "org.apache.struts2.SomeClass", wildcardPattern)); + } + + /** + * WW-5594: Tests that exact package pattern matches package names correctly. + * To properly exclude classes in a root package, use both the exact package name + * and the wildcard pattern. + */ + public void testWW5594_ExactPackagePatternMatchesPackageName() { + // given + HashMap<String, String> matchedPatterns = new HashMap<>(); + int[] exactPattern = wildcardHelper.compilePattern("org.apache.struts2"); + + // when & then - Exact pattern matches exactly + assertTrue("Exact pattern 'org.apache.struts2' should match 'org.apache.struts2'", + wildcardHelper.match(matchedPatterns, "org.apache.struts2", exactPattern)); + + // But exact pattern does NOT match subpackages + assertFalse("Exact pattern 'org.apache.struts2' should NOT match 'org.apache.struts2.core'", + wildcardHelper.match(matchedPatterns, "org.apache.struts2.core", exactPattern)); + } + } 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..ea9850568 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 @@ -18,6 +18,7 @@ */ package org.apache.struts2.convention; +import org.apache.commons.lang3.Strings; import org.apache.struts2.ActionContext; import org.apache.struts2.FileManager; import org.apache.struts2.FileManagerFactory; @@ -406,7 +407,7 @@ public class PackageBasedActionConfigBuilder implements ActionConfigBuilder { if (ctx != null) { classLoaderInterface = (ClassLoaderInterface) ctx.get(ClassLoaderInterface.CLASS_LOADER_INTERFACE); } - return ObjectUtils.defaultIfNull(classLoaderInterface, new ClassLoaderInterfaceDelegate(getClassLoader())); + return ObjectUtils.getIfNull(classLoaderInterface, new ClassLoaderInterfaceDelegate(getClassLoader())); } } @@ -561,7 +562,17 @@ public class PackageBasedActionConfigBuilder implements ActionConfigBuilder { } /** - * Checks if provided class package is on the exclude list + * Checks if provided class package is on the exclude list. + * <p> + * WW-5594: For patterns ending with ".*", this method also checks if the package name + * equals the base pattern (without ".*"). This ensures that classes directly in the + * root package are excluded, not just classes in subpackages. + * <p> + * For example, pattern "org.apache.struts2.*" will exclude both: + * <ul> + * <li>Classes in subpackages like "org.apache.struts2.dispatcher.SomeClass"</li> + * <li>Classes directly in the root package like "org.apache.struts2.XWorkTestCase"</li> + * </ul> * * @param classPackageName name of class package * @return false if class package is on the {@link #excludePackages} list @@ -574,6 +585,16 @@ public class PackageBasedActionConfigBuilder implements ActionConfigBuilder { Map<String, String> matchMap = new HashMap<>(); for (String packageExclude : excludePackages) { + // WW-5594: For patterns ending with ".*", also check if package equals the base + // This handles root package exclusion (e.g., pattern "org.apache.struts2.*" + // should also exclude classes in package "org.apache.struts2") + if (packageExclude.endsWith(".*")) { + String basePackage = packageExclude.substring(0, packageExclude.length() - 2); + if (classPackageName.equals(basePackage)) { + return false; + } + } + int[] packagePattern = wildcardHelper.compilePattern(packageExclude); if (wildcardHelper.match(matchMap, classPackageName, packagePattern)) { return false; @@ -647,7 +668,7 @@ public class PackageBasedActionConfigBuilder implements ActionConfigBuilder { * should be included in the package scan */ protected Test<ClassFinder.ClassInfo> getActionClassTest() { - return new Test<ClassFinder.ClassInfo>() { + return new Test<>() { public boolean test(ClassFinder.ClassInfo classInfo) { // Why do we call includeClassNameInActionScan here, when it's @@ -973,7 +994,7 @@ public class PackageBasedActionConfigBuilder implements ActionConfigBuilder { String className = actionClass.getName(); if (annotation != null) { actionName = annotation.value().equals(Action.DEFAULT_VALUE) ? actionName : annotation.value(); - actionName = StringUtils.contains(actionName, "/") && !slashesInActionNames ? StringUtils.substringAfterLast(actionName, "/") : actionName; + actionName = Strings.CI.contains(actionName, "/") && !slashesInActionNames ? StringUtils.substringAfterLast(actionName, "/") : actionName; if (!Action.DEFAULT_VALUE.equals(annotation.className())) { className = annotation.className(); } @@ -1060,7 +1081,7 @@ public class PackageBasedActionConfigBuilder implements ActionConfigBuilder { if (action != null && !action.value().equals(Action.DEFAULT_VALUE)) { LOG.trace("Using non-default action namespace from the Action annotation of [{}]", action.value()); String actionName = action.value(); - actionNamespace = StringUtils.contains(actionName, "/") ? StringUtils.substringBeforeLast(actionName, "/") : StringUtils.EMPTY; + actionNamespace = Strings.CI.contains(actionName, "/") ? StringUtils.substringBeforeLast(actionName, "/") : StringUtils.EMPTY; } // Next grab the parent annotation from the class diff --git a/plugins/convention/src/main/resources/struts-plugin.xml b/plugins/convention/src/main/resources/struts-plugin.xml index 541c79f0c..afafe0f66 100644 --- a/plugins/convention/src/main/resources/struts-plugin.xml +++ b/plugins/convention/src/main/resources/struts-plugin.xml @@ -21,53 +21,61 @@ --> <!DOCTYPE struts PUBLIC - "-//Apache Software Foundation//DTD Struts Configuration 6.0//EN" - "https://struts.apache.org/dtds/struts-6.0.dtd";> + "-//Apache Software Foundation//DTD Struts Configuration 6.0//EN" + "https://struts.apache.org/dtds/struts-6.0.dtd";> <struts order="20"> - <bean type="org.apache.struts2.UnknownHandler" name="convention" class="org.apache.struts2.convention.ConventionUnknownHandler"/> + <bean type="org.apache.struts2.UnknownHandler" name="convention" + class="org.apache.struts2.convention.ConventionUnknownHandler"/> - <bean type="org.apache.struts2.convention.ActionConfigBuilder" name="convention" class="org.apache.struts2.convention.PackageBasedActionConfigBuilder"/> - <bean type="org.apache.struts2.convention.ActionNameBuilder" name="convention" class="org.apache.struts2.convention.SEOActionNameBuilder"/> - <bean type="org.apache.struts2.convention.ResultMapBuilder" name="convention" class="org.apache.struts2.convention.DefaultResultMapBuilder"/> - <bean type="org.apache.struts2.convention.InterceptorMapBuilder" name="convention" class="org.apache.struts2.convention.DefaultInterceptorMapBuilder"/> - <bean type="org.apache.struts2.convention.ConventionsService" name="convention" class="org.apache.struts2.convention.ConventionsServiceImpl"/> + <bean type="org.apache.struts2.convention.ActionConfigBuilder" name="convention" + class="org.apache.struts2.convention.PackageBasedActionConfigBuilder"/> + <bean type="org.apache.struts2.convention.ActionNameBuilder" name="convention" + class="org.apache.struts2.convention.SEOActionNameBuilder"/> + <bean type="org.apache.struts2.convention.ResultMapBuilder" name="convention" + class="org.apache.struts2.convention.DefaultResultMapBuilder"/> + <bean type="org.apache.struts2.convention.InterceptorMapBuilder" name="convention" + class="org.apache.struts2.convention.DefaultInterceptorMapBuilder"/> + <bean type="org.apache.struts2.convention.ConventionsService" name="convention" + class="org.apache.struts2.convention.ConventionsServiceImpl"/> - <bean type="org.apache.struts2.config.PackageProvider" name="convention.packageProvider" class="org.apache.struts2.convention.ClasspathPackageProvider"/> - <bean type="org.apache.struts2.config.PackageProvider" name="convention.containerProvider" class="org.apache.struts2.convention.ClasspathConfigurationProvider"/> + <bean type="org.apache.struts2.config.PackageProvider" name="convention.packageProvider" + class="org.apache.struts2.convention.ClasspathPackageProvider"/> + <bean type="org.apache.struts2.config.PackageProvider" name="convention.containerProvider" + class="org.apache.struts2.convention.ClasspathConfigurationProvider"/> - <constant name="struts.convention.actionConfigBuilder" value="convention"/> - <constant name="struts.convention.actionNameBuilder" value="convention"/> - <constant name="struts.convention.resultMapBuilder" value="convention"/> - <constant name="struts.convention.interceptorMapBuilder" value="convention"/> - <constant name="struts.convention.conventionsService" value="convention"/> + <constant name="struts.convention.actionConfigBuilder" value="convention"/> + <constant name="struts.convention.actionNameBuilder" value="convention"/> + <constant name="struts.convention.resultMapBuilder" value="convention"/> + <constant name="struts.convention.interceptorMapBuilder" value="convention"/> + <constant name="struts.convention.conventionsService" value="convention"/> - <constant name="struts.convention.result.path" value="/WEB-INF/content/"/> - <constant name="struts.convention.result.flatLayout" value="true"/> - <constant name="struts.convention.action.suffix" value="Action"/> - <constant name="struts.convention.action.disableScanning" value="false"/> - <constant name="struts.convention.action.mapAllMatches" value="false"/> - <constant name="struts.convention.action.checkImplementsAction" value="true"/> - <constant name="struts.convention.default.parent.package" value="convention-default"/> - <constant name="struts.convention.action.name.lowercase" value="true"/> - <constant name="struts.convention.action.name.separator" value="-"/> - <constant name="struts.convention.package.locators" value="action,actions,struts,struts2"/> - <constant name="struts.convention.package.locators.disable" value="false"/> - <constant name="struts.convention.package.locators.basePackage" value=""/> - <constant name="struts.convention.exclude.packages" value="org.apache.struts.*,org.apache.struts2.*,org.springframework.web.struts.*,org.springframework.web.struts2.*,org.hibernate.*"/> - <constant name="struts.convention.relative.result.types" value="dispatcher,velocity,freemarker"/> - <constant name="struts.convention.redirect.to.slash" value="true"/> - <constant name="struts.convention.action.alwaysMapExecute" value="true"/> - <constant name="struts.mapper.alwaysSelectFullNamespace" value="true"/> - <!-- <constant name="struts.convention.action.includeJars" /> --> - <constant name="struts.convention.action.fileProtocols" value="jar" /> + <constant name="struts.convention.result.path" value="/WEB-INF/content/"/> + <constant name="struts.convention.result.flatLayout" value="true"/> + <constant name="struts.convention.action.suffix" value="Action"/> + <constant name="struts.convention.action.disableScanning" value="false"/> + <constant name="struts.convention.action.mapAllMatches" value="false"/> + <constant name="struts.convention.action.checkImplementsAction" value="true"/> + <constant name="struts.convention.default.parent.package" value="convention-default"/> + <constant name="struts.convention.action.name.lowercase" value="true"/> + <constant name="struts.convention.action.name.separator" value="-"/> + <constant name="struts.convention.package.locators" value="action,actions,struts,struts2"/> + <constant name="struts.convention.package.locators.disable" value="false"/> + <constant name="struts.convention.package.locators.basePackage" value=""/> + <constant name="struts.convention.exclude.packages" value="org.apache.struts.*,org.apache.struts2.*,org.springframework.web.struts.*,org.springframework.web.struts2.*,org.hibernate.*"/> + <constant name="struts.convention.relative.result.types" value="dispatcher,velocity,freemarker"/> + <constant name="struts.convention.redirect.to.slash" value="true"/> + <constant name="struts.convention.action.alwaysMapExecute" value="true"/> + <constant name="struts.mapper.alwaysSelectFullNamespace" value="true"/> + <!-- <constant name="struts.convention.action.includeJars" /> --> + <constant name="struts.convention.action.fileProtocols" value="jar"/> - <constant name="struts.convention.classes.reload" value="false" /> + <constant name="struts.convention.classes.reload" value="false"/> - <constant name="struts.convention.exclude.parentClassLoader" value="true" /> + <constant name="struts.convention.exclude.parentClassLoader" value="true"/> - <constant name="struts.convention.enable.smi.inheritance" value="false" /> + <constant name="struts.convention.enable.smi.inheritance" value="false"/> - <package name="convention-default" extends="struts-default"> - </package> + <package name="convention-default" extends="struts-default"> + </package> </struts> 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..bb85dcac5 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 @@ -18,7 +18,6 @@ */ package org.apache.struts2.convention; -import org.apache.struts2.result.ActionChainResult; import jakarta.servlet.ServletContext; import junit.framework.TestCase; import org.apache.commons.lang3.StringUtils; @@ -94,6 +93,7 @@ import org.apache.struts2.inject.Container; import org.apache.struts2.inject.Scope.Strategy; import org.apache.struts2.ognl.OgnlReflectionProvider; import org.apache.struts2.ognl.ProviderAllowlist; +import org.apache.struts2.result.ActionChainResult; import org.apache.struts2.result.Result; import org.apache.struts2.result.ServletDispatcherResult; import org.apache.struts2.util.TextParseUtil; @@ -127,8 +127,8 @@ public class PackageBasedActionConfigBuilderTest extends TestCase { public void setUp() throws Exception { super.setUp(); ActionContext.of() - .withContainer(new DummyContainer()) - .bind(); + .withContainer(new DummyContainer()) + .bind(); } public void testActionPackages() throws MalformedURLException { @@ -155,9 +155,83 @@ public class PackageBasedActionConfigBuilderTest extends TestCase { run("org.apache.struts2.convention.actions", null, null, "false"); } + /** + * WW-5594: Tests that exclusion patterns properly exclude classes in root packages. + * <p> + * The issue was that pattern "org.apache.struts2.*" did NOT match package name + * "org.apache.struts2" (without trailing dot) because: + * 1. PackageBasedActionConfigBuilder extracts package name using substringBeforeLast(className, ".") + * 2. For class "org.apache.struts2.XWorkTestCase", this produces "org.apache.struts2" (no trailing dot) + * 3. The wildcard pattern requires a literal "." before the "*" + * <p> + * The fix is in checkExcludePackages() which now handles patterns ending with ".*" by also + * checking if the package name equals the base pattern (without ".*"). + */ + public void testWW5594_RootPackageExclusion() { + // Setup minimal configuration + final DummyContainer mockContainer = new DummyContainer(); + Configuration configuration = new DefaultConfiguration() { + @Override + public Container getContainer() { + return mockContainer; + } + }; + + ResultTypeConfig[] defaultResults = new ResultTypeConfig[]{ + new ResultTypeConfig.Builder("dispatcher", ServletDispatcherResult.class.getName()) + .defaultResultParam("location").build() + }; + PackageConfig strutsDefault = makePackageConfig("struts-default", null, null, "dispatcher", defaultResults); + 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"); + + // Test 1: Wildcard pattern now properly excludes root package classes (WW-5594 fix) + builder.setActionPackages("org.apache.struts2"); + builder.setExcludePackages("org.apache.struts2.*"); + + // Class in root package - package name is "org.apache.struts2" (no trailing dot) + // With the fix, pattern "org.apache.struts2.*" now also matches the base package + boolean includeRootPackageClass = builder.includeClassNameInActionScan("org.apache.struts2.XWorkTestCase"); + assertFalse("With wildcard pattern, root package class should be excluded (WW-5594 fix)", + includeRootPackageClass); + + // Test 2: Subpackage classes should also be excluded by wildcard pattern + boolean includeSubpackageClass = builder.includeClassNameInActionScan("org.apache.struts2.core.ActionSupport"); + assertFalse("Subpackage classes should be excluded by wildcard pattern", + includeSubpackageClass); + + // Test 3: Exact pattern still works for specific package exclusion + builder.setExcludePackages("org.apache.struts2"); + boolean includeWithExactPattern = builder.includeClassNameInActionScan("org.apache.struts2.XWorkTestCase"); + assertFalse("Exact pattern should exclude root package class", + includeWithExactPattern); + + // Test 4: Exact pattern should NOT exclude subpackage classes + boolean includeSubpackageWithExact = builder.includeClassNameInActionScan("org.apache.struts2.core.ActionSupport"); + assertTrue("Exact pattern should NOT exclude subpackage classes", + includeSubpackageWithExact); + + // Test 5: Classes in unrelated packages should NOT be excluded + builder.setActionPackages("com.example.actions"); + builder.setExcludePackages("org.apache.struts2.*"); + boolean includeUnrelated = builder.includeClassNameInActionScan("com.example.actions.MyAction"); + assertTrue("Classes in unrelated packages should not be excluded", + includeUnrelated); + } + 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 +272,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 +280,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 +302,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 +322,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 +483,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 +780,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 +815,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 +830,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 +843,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 +862,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 +926,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 +967,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 +1003,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-5594-wildcard-exclusion-pattern.md b/thoughts/shared/research/2025-12-02-WW-5594-wildcard-exclusion-pattern.md new file mode 100644 index 000000000..d0f05822a --- /dev/null +++ b/thoughts/shared/research/2025-12-02-WW-5594-wildcard-exclusion-pattern.md @@ -0,0 +1,460 @@ +--- +date: 2025-12-02T18:33:08+01:00 +topic: "WW-5594: Convention plugin exclusion pattern wildcard matching issue" +tags: [research, codebase, convention-plugin, wildcard-matching, configuration, bug-analysis, WW-5594] +status: complete +jira_ticket: WW-5594 +related_tickets: [WW-5593] +--- + +# Research: WW-5594 - Convention Plugin Wildcard Exclusion Pattern Issue + +**Date**: 2025-12-02T18:33:08+01:00 +**JIRA**: [WW-5594](https://issues.apache.org/jira/browse/WW-5594) +**Related**: [WW-5593](https://issues.apache.org/jira/browse/WW-5593) + +## Research Question + +Why doesn't the default exclusion pattern `org.apache.struts2.*` properly exclude classes directly in the `org.apache.struts2` package (like `XWorkTestCase`)? + +## Summary + +The convention plugin's `PackageBasedActionConfigBuilder` extracts package names from class names using `StringUtils.substringBeforeLast(className, ".")`, then matches these package names against exclusion patterns. For class `org.apache.struts2.XWorkTestCase`, this produces package name `org.apache.struts2` (no trailing dot). The pattern `org.apache.struts2.*` expects something after the dot for the `*` to match, causing a mismatch for classes directly in the root package. + +This is a conceptual mismatch between patterns designed for full class names vs. matching against extracted package names. + +## Detailed Findings + +### 1. Package Exclusion Logic + +**Location**: `plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java:558-584` + +#### Package Name Extraction (Line 558-560) + +```java +protected boolean includeClassNameInActionScan(String className) { + String classPackageName = StringUtils.substringBeforeLast(className, "."); + return (checkActionPackages(classPackageName) || checkPackageLocators(classPackageName)) && checkExcludePackages(classPackageName); +} +``` + +**Examples**: +- `"org.apache.struts2.core.ActionSupport"` → `"org.apache.struts2.core"` +- `"com.example.actions.UserAction"` → `"com.example.actions"` +- `"org.apache.struts2.XWorkTestCase"` → `"org.apache.struts2"` ⚠️ + +#### Exclusion Pattern Matching (Lines 569-584) + +```java +protected boolean checkExcludePackages(String classPackageName) { + if (excludePackages != null && excludePackages.length > 0) { + WildcardHelper wildcardHelper = new WildcardHelper(); + Map<String, String> matchMap = new HashMap<>(); + + for (String packageExclude : excludePackages) { + int[] packagePattern = wildcardHelper.compilePattern(packageExclude); + if (wildcardHelper.match(matchMap, classPackageName, packagePattern)) { + return false; + } + } + } + return true; +} +``` + +### 2. The Conceptual Mismatch + +**The Problem**: + +When checking if `org.apache.struts2.XWorkTestCase` should be excluded: + +1. **Step 1**: Extract package name + - Input: `"org.apache.struts2.XWorkTestCase"` + - `substringBeforeLast(className, ".")` → `"org.apache.struts2"` + - No trailing dot + +2. **Step 2**: Match against pattern `org.apache.struts2.*` + - Pattern expects: `org.apache.struts2.` + `*` (something) + - Actual input: `org.apache.struts2` (nothing after last segment) + - **Mismatch**: Pattern requires a literal `.` that doesn't exist in the package name + +**Root Cause**: +- Exclusion patterns are written for **fully qualified class names** (e.g., `org.apache.struts2.*` should match `org.apache.struts2.XWorkTestCase`) +- But matching is performed against **package names only** (e.g., `org.apache.struts2` after stripping class name) +- This creates edge cases for classes directly in a package + +### 3. WildcardHelper Pattern Matching + +**Location**: `core/src/main/java/org/apache/struts2/util/WildcardHelper.java` + +#### Pattern Compilation Constants + +From `WildcardHelper.java:42-48`: +```java +public static final int MATCH_FILE = -1; // Single * - matches zero or more chars (excluding /) +public static final int MATCH_PATH = -2; // Double ** - matches zero or more chars (including /) +public static final int MATCH_BEGIN = -4; // Pattern must match from beginning +public static final int MATCH_THEEND = -5; // Pattern must match to the end +``` + +#### How Pattern `org.apache.struts2.*` Compiles + +The pattern compiles to: +``` +[MATCH_BEGIN, 'o', 'r', 'g', '.', 'a', 'p', 'a', 'c', 'h', 'e', '.', 's', 't', 'r', 'u', 't', 's', '2', '.', MATCH_FILE, MATCH_THEEND] +``` + +**Matching Requirements**: +1. Must start with `org.apache.struts2.` (literal characters) +2. Must have `MATCH_FILE` (zero or more non-slash characters) +3. Must end exactly (`MATCH_THEEND`) + +#### Test Evidence + +From `core/src/test/java/org/apache/struts2/util/WildcardHelperTest.java:54-76`: + +```java +public void testMatchStrutsPackages() { + // given + HashMap<String, String> matchedPatterns = new HashMap<>(); + int[] pattern = wildcardHelper.compilePattern("org.apache.struts2.*"); + + // when & then + assertTrue(wildcardHelper.match(matchedPatterns, "org.apache.struts2.XWorkTestCase", pattern)); + assertEquals("org.apache.struts2.XWorkTestCase", matchedPatterns.get("0")); + assertEquals("XWorkTestCase", matchedPatterns.get("1")); + + assertTrue(wildcardHelper.match(matchedPatterns, "org.apache.struts2.core.SomeClass", pattern)); + assertEquals("org.apache.struts2.core.SomeClass", matchedPatterns.get("0")); + assertEquals("core.SomeClass", matchedPatterns.get("1")); + + // IMPORTANT: Pattern matches even with nothing after dot + assertTrue(wildcardHelper.match(matchedPatterns, "org.apache.struts2.", pattern)); + assertEquals("org.apache.struts2.", matchedPatterns.get("0")); + assertEquals("", matchedPatterns.get("1")); +} +``` + +**Key Insight**: The pattern `org.apache.struts2.*` **WILL match** `org.apache.struts2.` (with trailing dot) because `MATCH_FILE` can match zero characters. However, it **WON'T match** `org.apache.struts2` (without trailing dot) because the literal `.` character before `MATCH_FILE` is required. + +### 4. Why This Matters + +**Scenario**: Class `org.apache.struts2.XWorkTestCase` should be excluded + +**What Happens**: +1. Pattern in config: `org.apache.struts2.*` +2. Class name: `org.apache.struts2.XWorkTestCase` +3. Extracted package: `org.apache.struts2` (no trailing dot) +4. Pattern match: `"org.apache.struts2"` vs pattern `"org.apache.struts2.*"` +5. **Result**: NO MATCH (pattern expects literal `.` before the `*`) +6. Class is NOT excluded +7. Convention plugin tries to scan it +8. Triggers WW-5593 (NoClassDefFoundError) + +**Contrast with subpackage classes**: +1. Class name: `org.apache.struts2.core.ActionSupport` +2. Extracted package: `org.apache.struts2.core` +3. Pattern match: `"org.apache.struts2.core"` vs pattern `"org.apache.struts2.*"` +4. **Result**: NO MATCH (pattern expects `org.apache.struts2.` + something, but we have `org.apache.struts2.core` which is a different string) + +**Wait, that's also wrong!** Let me reconsider... + +Actually, looking at the test more carefully: + +The test shows that `"org.apache.struts2.*"` matches: +- `"org.apache.struts2.XWorkTestCase"` (full class name) +- `"org.apache.struts2.core.SomeClass"` (subpackage class name) + +But in the actual code, we're matching: +- `"org.apache.struts2"` (extracted package, no class name) + +So the pattern `"org.apache.struts2.*"` is designed to match **class names** like `"org.apache.struts2.XWorkTestCase"`, but it's being used to match **package names** like `"org.apache.struts2"`. + +### 5. Default Configuration + +**Location**: `plugins/convention/src/main/resources/struts-plugin.xml:57` + +```xml +<constant name="struts.convention.exclude.packages" + value="org.apache.struts.*,org.apache.struts2.*,org.springframework.web.struts.*,org.springframework.web.struts2.*,org.hibernate.*"/> +``` + +**Packages That Should Be Excluded**: +- `org.apache.struts.*` - Struts 1.x packages +- `org.apache.struts2.*` - Struts 2.x packages +- `org.springframework.web.struts.*` - Spring Struts integration +- `org.springframework.web.struts2.*` - Spring Struts 2 integration +- `org.hibernate.*` - Hibernate packages + +**Problem**: Classes directly in these root packages (without subpackages) may not be properly excluded: +- `org.apache.struts2.XWorkTestCase` ❌ Not excluded +- `org.apache.struts2.StrutsConstants` ❌ Not excluded +- But: `org.apache.struts2.core.ActionSupport` - needs investigation + +### 6. Email Thread Evidence + +From Florian Schlittgen's debugging (2024-12-19 21:41): + +```java +public static void main(String[] args) { + String packageExclude = "org.apache.struts2.*"; + String classPackageName = "org.apache.struts2"; + WildcardHelper wildcardHelper = new WildcardHelper(); + int[] packagePattern = wildcardHelper.compilePattern(packageExclude); + System.out.println(wildcardHelper.match(new HashMap<>(), classPackageName, packagePattern)); +} +``` + +**Output**: `false` + +The pattern `"org.apache.struts2.*"` does NOT match `"org.apache.struts2"` (without trailing dot). + +From Lukasz Lenart's response (2024-12-23 19:12): + +```java +// This WORKS (with trailing dot) +String classPackageName = "org.apache.struts2."; +// Output: true +``` + +So the pattern requires a trailing dot to match. + +## Code References + +- `plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java:558-584` - Package exclusion logic +- `core/src/main/java/org/apache/struts2/util/WildcardHelper.java:71-254` - Wildcard pattern matching implementation +- `core/src/test/java/org/apache/struts2/util/WildcardHelperTest.java:54-76` - Pattern matching tests +- `plugins/convention/src/main/resources/struts-plugin.xml:57` - Default exclusion configuration + +## Recommended Fixes + +### Option A: Update Default Configuration (Simplest) + +**File**: `plugins/convention/src/main/resources/struts-plugin.xml:57` + +Add both root packages and wildcard patterns to the configuration. + +**Disadvantages**: +- ❌ Users with custom exclusion patterns still need to know about this +- ❌ Makes the configuration string longer +- ❌ Requires pattern duplication + +### Option B: Match Against Full Class Name + +Match exclusion patterns against the full class name instead of just the package name. + +**Disadvantages**: +- ❌ Changes method logic significantly +- ❌ May affect performance (two wildcard checks instead of one) +- ❌ Could have unintended side effects with existing configurations + +### Option C: Enhance WildcardHelper for Package Matching + +Create a new method `matchPackagePattern()` that understands package-style matching. + +**Disadvantages**: +- ❌ Most complex solution +- ❌ Changes core utility class +- ❌ Requires extensive testing +- ❌ Overkill for the problem + +### Option D: Enhance checkExcludePackages to Handle Root Packages (IMPLEMENTED ✅) + +**File**: `plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java` + +Modify `checkExcludePackages()` to automatically handle patterns ending with `.*` by also checking if the package name equals the base pattern (without `.*`). + +**Implementation**: +```java +protected boolean checkExcludePackages(String classPackageName) { + if (excludePackages != null && excludePackages.length > 0) { + WildcardHelper wildcardHelper = new WildcardHelper(); + Map<String, String> matchMap = new HashMap<>(); + + for (String packageExclude : excludePackages) { + // WW-5594: For patterns ending with ".*", also check if package equals the base + if (packageExclude.endsWith(".*")) { + String basePackage = packageExclude.substring(0, packageExclude.length() - 2); + if (classPackageName.equals(basePackage)) { + return false; + } + } + + int[] packagePattern = wildcardHelper.compilePattern(packageExclude); + if (wildcardHelper.match(matchMap, classPackageName, packagePattern)) { + return false; + } + } + } + return true; +} +``` + +**Advantages**: +- ✅ No configuration duplication needed +- ✅ Backward compatible - existing configurations with `.*` patterns now work correctly +- ✅ Single point of fix in `checkExcludePackages()` method +- ✅ Intuitive behavior - `org.apache.struts2.*` now excludes both root and subpackages +- ✅ Minimal code change +- ✅ Users don't need to update their configurations + +**Disadvantages**: +- ❌ Slightly more processing per pattern (trivial performance impact) + +### Recommendation + +**Option D** was chosen and implemented because: +1. Most elegant solution - no configuration duplication +2. Backward compatible - existing configurations work better +3. Single point of fix +4. Intuitive behavior for users +5. Minimal code change with maximum benefit + +## Impact Assessment + +### Current Risk + +**Severity**: MEDIUM + +**Scenarios Affected**: +1. Classes directly in root Struts packages (like `XWorkTestCase`) +2. Users setting `struts.convention.action.includeJars` +3. Custom exclusion patterns not including root packages + +**Current Behavior**: +- ❌ Test classes in root packages not excluded +- ❌ Can trigger WW-5593 (NoClassDefFoundError) +- ❌ Users must manually add root packages to exclusions +- ❌ Non-intuitive pattern matching behavior + +### After Fix + +**Expected Behavior**: +- ✅ Root package classes properly excluded by default +- ✅ Reduces occurrence of WW-5593 +- ✅ More intuitive default configuration +- ✅ Better documentation for custom patterns + +## Testing Strategy + +### Unit Tests to Add + +1. **Test Root Package Exclusion**: +```java +@Test +public void testExcludeRootPackageClasses() { + String[] excludePackages = {"org.apache.struts2", "org.apache.struts2.*"}; + builder.setExcludePackages(excludePackages); + + // Should exclude classes directly in org.apache.struts2 + assertFalse(builder.includeClassNameInActionScan("org.apache.struts2.XWorkTestCase")); + assertFalse(builder.includeClassNameInActionScan("org.apache.struts2.StrutsConstants")); + + // Should also exclude subpackages + assertFalse(builder.includeClassNameInActionScan("org.apache.struts2.core.ActionSupport")); + assertFalse(builder.includeClassNameInActionScan("org.apache.struts2.dispatcher.Dispatcher")); +} +``` + +2. **Test Wildcard Pattern Behavior**: +```java +@Test +public void testWildcardPatternMatchingWithPackageNames() { + WildcardHelper helper = new WildcardHelper(); + int[] pattern = helper.compilePattern("org.apache.struts2.*"); + + // Package name without trailing dot should NOT match + assertFalse(helper.match(new HashMap<>(), "org.apache.struts2", pattern)); + + // Package name with trailing dot SHOULD match + assertTrue(helper.match(new HashMap<>(), "org.apache.struts2.", pattern)); + + // Full class name SHOULD match + assertTrue(helper.match(new HashMap<>(), "org.apache.struts2.XWorkTestCase", pattern)); +} +``` + +3. **Test Default Configuration**: +```java +@Test +public void testDefaultExclusionPatterns() { + // Load default patterns from struts-plugin.xml + builder.setExcludePackages(getDefaultExclusionPatterns()); + + // Should exclude Struts 2 root package classes + assertFalse(builder.includeClassNameInActionScan("org.apache.struts2.XWorkTestCase")); + + // Should exclude Struts 1 root package classes + assertFalse(builder.includeClassNameInActionScan("org.apache.struts.Action")); + + // Should exclude Hibernate root package classes + assertFalse(builder.includeClassNameInActionScan("org.hibernate.Session")); +} +``` + +### Integration Tests + +1. Test scanning with `struts.convention.action.includeJars` configured +2. Verify classes in root excluded packages are not scanned +3. Verify classes in subpackages are properly excluded +4. Verify user action classes are still included + +## Workaround for Users + +Until the fix is released, users should update their exclusion patterns: + +```xml +<constant name="struts.convention.exclude.packages" + value="org.apache.struts2,org.apache.struts2.*,com.opensymphony.xwork2,com.opensymphony.xwork2.*" /> +``` + +**Pattern**: For each package to exclude, include both: +- Root package without wildcard: `org.apache.struts2` +- Subpackages with wildcard: `org.apache.struts2.*` + +## Documentation Updates Needed + +1. **Convention Plugin Documentation**: + - Explain wildcard pattern matching behavior + - Provide examples of correct exclusion patterns + - Document the difference between `org.example` and `org.example.*` + +2. **Migration Guide**: + - Note the improved default exclusions in release notes + - Explain that custom exclusion patterns should include root packages + +3. **Configuration Reference**: + - Update `struts.convention.exclude.packages` documentation + - Add examples showing both patterns + +## Related Issues + +- **WW-5593**: NoClassDefFoundError bug that this exclusion issue can trigger +- Related research: `thoughts/shared/research/2025-12-02-WW-5593-convention-plugin-noclass-exception.md` + +## Email Thread Reference + +**From**: Florian Schlittgen ([email protected]) +**Date**: December 19, 2024 - January 19, 2025 +**Subject**: Struts 7: action class finder +**List**: [email protected] + +**Key Insight from Thread** (Florian, 2024-12-23 19:41): +> "Thanks for looking into it. Your examples/tests are alright, but I think this is not the way it is being called. Please take a look at org.apache.struts2.convention.PackageBasedActionConfigBuilder.includeClassNameInActionScan(String). While debugging I can see that this method is being called with parameter "className" = "org.apache.struts2.XWorkTestCase". In the method's first line the package name is derived from the className by using "StringUtils.substringBeforeLast(className, "." [...] + +## Implementation Status + +1. ✅ Create JIRA ticket WW-5594 +2. ✅ Modify `checkExcludePackages()` to handle `.*` patterns for root packages (Option D) +3. ✅ Default exclusion patterns in `struts-plugin.xml` unchanged (no duplication needed) +4. ✅ Add unit tests for root package exclusion (`PackageBasedActionConfigBuilderTest.testWW5594_RootPackageExclusion`) +5. ✅ Add unit tests documenting WildcardHelper behavior (`WildcardHelperTest.testWW5594_*`) +6. ⏳ Add integration tests with includeJars configured +7. ⏳ Update documentation for pattern matching behavior +8. ⏳ Update migration guide with new default patterns + +## Files Changed + +- `plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java` - Enhanced `checkExcludePackages()` method +- `plugins/convention/src/test/java/org/apache/struts2/convention/PackageBasedActionConfigBuilderTest.java` - Added `testWW5594_RootPackageExclusion()` +- `core/src/test/java/org/apache/struts2/util/WildcardHelperTest.java` - Added `testWW5594_WildcardPatternRequiresTrailingDot()` and `testWW5594_ExactPackagePatternMatchesPackageName()` \ No newline at end of file
