From bf5f29132be6f4be9d3570a3fb2873a44d5f48f3 Mon Sep 17 00:00:00 2001 From: Jeffery Xie Date: Thu, 11 Apr 2024 12:44:53 +1000 Subject: [PATCH 1/3] WW-5409 introduce final attribute to package element which make them unextendable --- .../xwork2/config/entities/PackageConfig.java | 13 ++ .../XmlDocConfigurationProvider.java | 25 ++- .../StrutsXmlConfigurationProvider.java | 1 + core/src/main/resources/struts-6.4.0.dtd | 158 ++++++++++++++++++ .../XmlConfigurationProviderPackagesTest.java | 68 ++++++-- .../xwork-test-package-extends-final.xml | 37 ++++ .../providers/xwork-test-package-final.xml | 37 ++++ 7 files changed, 318 insertions(+), 21 deletions(-) create mode 100644 core/src/main/resources/struts-6.4.0.dtd create mode 100644 core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-package-extends-final.xml create mode 100644 core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-package-final.xml diff --git a/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java b/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java index 9174e651b5..eafff831da 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java @@ -47,6 +47,7 @@ public class PackageConfig extends Located implements Comparable, protected String name; protected String namespace = ""; protected boolean isAbstract = false; + protected boolean isFinal = false; // a final package is unextendable protected boolean needsRefresh; protected boolean strictMethodInvocation = true; @@ -69,6 +70,7 @@ protected PackageConfig(PackageConfig orig) { this.name = orig.name; this.namespace = orig.namespace; this.isAbstract = orig.isAbstract; + this.isFinal = orig.isFinal; this.needsRefresh = orig.needsRefresh; this.actionConfigs = new LinkedHashMap<>(orig.actionConfigs); this.globalResultConfigs = new LinkedHashMap<>(orig.globalResultConfigs); @@ -85,6 +87,10 @@ public boolean isAbstract() { return isAbstract; } + public boolean isFinal() { + return isFinal; + } + public Map getActionConfigs() { return actionConfigs; } @@ -360,6 +366,7 @@ public boolean equals(Object o) { PackageConfig that = (PackageConfig) o; if (isAbstract != that.isAbstract) return false; + if (isFinal != that.isFinal) return false; if (needsRefresh != that.needsRefresh) return false; if (strictMethodInvocation != that.strictMethodInvocation) return false; if (actionConfigs != null ? !actionConfigs.equals(that.actionConfigs) : that.actionConfigs != null) @@ -404,6 +411,7 @@ public int hashCode() { result = 31 * result + name.hashCode(); result = 31 * result + (namespace != null ? namespace.hashCode() : 0); result = 31 * result + (isAbstract ? 1 : 0); + result = 31 * result + (isFinal ? 1 : 0); result = 31 * result + (needsRefresh ? 1 : 0); result = 31 * result + (strictMethodInvocation ? 1 : 0); return result; @@ -453,6 +461,11 @@ public Builder isAbstract(boolean isAbstract) { return this; } + public Builder isFinal(boolean isFinal) { + target.isFinal = isFinal; + return this; + } + public Builder defaultInterceptorRef(String name) { target.defaultInterceptorRef = name; return this; diff --git a/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlDocConfigurationProvider.java b/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlDocConfigurationProvider.java index cad52fb79f..6de2024609 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlDocConfigurationProvider.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlDocConfigurationProvider.java @@ -603,8 +603,8 @@ protected Class verifyResultType(String className, Location loc) { */ protected PackageConfig.Builder buildPackageContext(Element packageElement) { String parent = packageElement.getAttribute("extends"); - String abstractVal = packageElement.getAttribute("abstract"); - boolean isAbstract = parseBoolean(abstractVal); + boolean isAbstract = parseBoolean(packageElement.getAttribute("abstract")); + boolean isFinal = parseBoolean(packageElement.getAttribute("final")); String name = defaultString(packageElement.getAttribute("name")); String namespace = defaultString(packageElement.getAttribute("namespace")); @@ -617,6 +617,7 @@ protected PackageConfig.Builder buildPackageContext(Element packageElement) { PackageConfig.Builder cfg = new PackageConfig.Builder(name) .namespace(namespace) .isAbstract(isAbstract) + .isFinal(isFinal) .strictMethodInvocation(strictDMI) .location(DomHelper.getLocationObject(packageElement)); @@ -627,17 +628,23 @@ protected PackageConfig.Builder buildPackageContext(Element packageElement) { // has parents, let's look it up List parents = new ArrayList<>(); for (String parentPackageName : ConfigurationUtil.buildParentListFromString(parent)) { - if (configuration.getPackageConfigNames().contains(parentPackageName)) { - parents.add(configuration.getPackageConfig(parentPackageName)); - } else if (declaredPackages.containsKey(parentPackageName)) { - if (configuration.getPackageConfig(parentPackageName) == null) { - addPackage(declaredPackages.get(parentPackageName)); + boolean isParentPackageConfigDefined = false; + if (configuration.getPackageConfigNames().contains(parentPackageName)) { // parent package already added to configuration + isParentPackageConfigDefined = true; + } else if (declaredPackages.containsKey(parentPackageName)) { // parent package declared but yet added to configuration + addPackage(declaredPackages.get(parentPackageName)); + isParentPackageConfigDefined = true; + } + + if (isParentPackageConfigDefined) { + PackageConfig parentPackageConfig = configuration.getPackageConfig(parentPackageName); + if (parentPackageConfig.isFinal()) { + throw new ConfigurationException("Parent package is final and unextendable: " + parentPackageName); } - parents.add(configuration.getPackageConfig(parentPackageName)); + parents.add(parentPackageConfig); } else { throw new ConfigurationException("Parent package is not defined: " + parentPackageName); } - } if (parents.isEmpty()) { diff --git a/core/src/main/java/org/apache/struts2/config/StrutsXmlConfigurationProvider.java b/core/src/main/java/org/apache/struts2/config/StrutsXmlConfigurationProvider.java index 9b62b3d59e..d38b36cadd 100644 --- a/core/src/main/java/org/apache/struts2/config/StrutsXmlConfigurationProvider.java +++ b/core/src/main/java/org/apache/struts2/config/StrutsXmlConfigurationProvider.java @@ -54,6 +54,7 @@ public class StrutsXmlConfigurationProvider extends XmlConfigurationProvider { put("-//Apache Software Foundation//DTD Struts Configuration 2.3//EN", "struts-2.3.dtd"); put("-//Apache Software Foundation//DTD Struts Configuration 2.5//EN", "struts-2.5.dtd"); put("-//Apache Software Foundation//DTD Struts Configuration 6.0//EN", "struts-6.0.dtd"); + put("-//Apache Software Foundation//DTD Struts Configuration 6.4.0//EN", "struts-6.4.0.dtd"); }}); private File baseDir = null; private final String filename; diff --git a/core/src/main/resources/struts-6.4.0.dtd b/core/src/main/resources/struts-6.4.0.dtd new file mode 100644 index 0000000000..222c29d242 --- /dev/null +++ b/core/src/main/resources/struts-6.4.0.dtd @@ -0,0 +1,158 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderPackagesTest.java b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderPackagesTest.java index b39baecdaa..ed8910bfab 100644 --- a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderPackagesTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderPackagesTest.java @@ -36,10 +36,9 @@ public class XmlConfigurationProviderPackagesTest extends ConfigurationTestBase { public void testBadInheritance() throws ConfigurationException { - final String filename = "com/opensymphony/xwork2/config/providers/xwork-test-bad-inheritance.xml"; ConfigurationProvider provider = null; try { - provider = buildConfigurationProvider(filename); + provider = buildConfigurationProvider(getXmlConfigFilePath("xwork-test-bad-inheritance.xml")); fail("Should have thrown a ConfigurationException"); provider.init(configuration); provider.loadPackages(); @@ -49,8 +48,7 @@ public void testBadInheritance() throws ConfigurationException { } public void testBasicPackages() throws ConfigurationException { - final String filename = "com/opensymphony/xwork2/config/providers/xwork-test-basic-packages.xml"; - ConfigurationProvider provider = buildConfigurationProvider(filename); + ConfigurationProvider provider = buildConfigurationProvider(getXmlConfigFilePath("xwork-test-basic-packages.xml")); provider.init(configuration); provider.loadPackages(); @@ -70,8 +68,7 @@ public void testBasicPackages() throws ConfigurationException { } public void testDefaultPackage() throws ConfigurationException { - final String filename = "com/opensymphony/xwork2/config/providers/xwork-test-default-package.xml"; - ConfigurationProvider provider = buildConfigurationProvider(filename); + ConfigurationProvider provider = buildConfigurationProvider(getXmlConfigFilePath("xwork-test-default-package.xml")); provider.init(configuration); provider.loadPackages(); @@ -84,8 +81,7 @@ public void testDefaultPackage() throws ConfigurationException { } public void testPackageInheritance() throws ConfigurationException { - final String filename = "com/opensymphony/xwork2/config/providers/xwork-test-package-inheritance.xml"; - ConfigurationProvider provider = buildConfigurationProvider(filename); + ConfigurationProvider provider = buildConfigurationProvider(getXmlConfigFilePath("xwork-test-package-inheritance.xml")); provider.init(configuration); provider.loadPackages(); @@ -111,7 +107,7 @@ public void testPackageInheritance() throws ConfigurationException { assertTrue(multipleParents.contains(defaultPackage)); assertTrue(multipleParents.contains(abstractPackage)); assertTrue(multipleParents.contains(singlePackage)); - + PackageConfig parentBelow = configuration.getPackageConfig("testParentBelow"); assertEquals(1, parentBelow.getParents().size()); List parentBelowParents = parentBelow.getParents(); @@ -129,7 +125,7 @@ public void testPackageInheritance() throws ConfigurationException { assertNull(runtimeConfiguration.getActionConfig("/single", "abstract")); assertNotNull(runtimeConfiguration.getActionConfig("/single", "single")); assertNull(runtimeConfiguration.getActionConfig("/single", "multiple")); - + assertNotNull(runtimeConfiguration.getActionConfig("/parentBelow", "default")); assertNotNull(runtimeConfiguration.getActionConfig("/parentBelow", "abstract")); assertNotNull(runtimeConfiguration.getActionConfig("/parentBelow", "single")); @@ -138,13 +134,57 @@ public void testPackageInheritance() throws ConfigurationException { } + public void testPackageWithFinalAttributeLoads() throws ConfigurationException { + ConfigurationProvider provider = buildConfigurationProvider(getXmlConfigFilePath("xwork-test-package-final.xml")); + + provider.init(configuration); + provider.loadPackages(); + + // test expectations + assertEquals(3, configuration.getPackageConfigs().size()); + PackageConfig defaultPackage = configuration.getPackageConfig("default"); + assertNotNull(defaultPackage); + assertEquals("default", defaultPackage.getName()); + + // final package extends default + PackageConfig finalPackage = configuration.getPackageConfig("finalPackage"); + assertNotNull(finalPackage); + assertEquals("finalPackage", finalPackage.getName()); + assertEquals(1, finalPackage.getParents().size()); + assertEquals(defaultPackage, finalPackage.getParents().get(0)); + + // normal package extends default + PackageConfig normalPackage = configuration.getPackageConfig("normalPackage"); + assertNotNull(normalPackage); + assertEquals("normalPackage", normalPackage.getName()); + assertEquals(1, normalPackage.getParents().size()); + assertEquals(defaultPackage, normalPackage.getParents().get(0)); + + configurationManager.addContainerProvider(provider); + configurationManager.reload(); + + RuntimeConfiguration runtimeConfiguration = configurationManager.getConfiguration().getRuntimeConfiguration(); + assertNotNull(runtimeConfiguration.getActionConfig("/final", "default")); + assertNotNull(runtimeConfiguration.getActionConfig("/final", "actionFinal")); + + assertNotNull(runtimeConfiguration.getActionConfig("/normal", "default")); + assertNotNull(runtimeConfiguration.getActionConfig("/normal", "actionNormal")); + } + + public void testExtendsFinalPackageThrowsConfigurationException() throws ConfigurationException { + try { + buildConfigurationProvider(getXmlConfigFilePath("xwork-test-package-extends-final.xml")); + } catch (ConfigurationException e) { + assertEquals("Parent package is final and unextendable: parentLevelTwo", e.getMessage()); + } + } + public void testDefaultClassRef() throws ConfigurationException { - final String filename = "com/opensymphony/xwork2/config/providers/xwork-test-defaultclassref-package.xml"; final String hasDefaultClassRefPkgName = "hasDefaultClassRef"; final String noDefaultClassRefPkgName = "noDefaultClassRef"; final String testDefaultClassRef = "com.opensymphony.xwork2.ActionSupport"; - ConfigurationProvider provider = buildConfigurationProvider(filename); + ConfigurationProvider provider = buildConfigurationProvider(getXmlConfigFilePath("xwork-test-defaultclassref-package.xml")); provider.init(configuration); // setup our expectations @@ -157,4 +197,8 @@ public void testDefaultClassRef() throws ConfigurationException { assertEquals(expectedDefaultClassRefPackage, configuration.getPackageConfig(hasDefaultClassRefPkgName)); assertEquals(expectedNoDefaultClassRefPackage, configuration.getPackageConfig(noDefaultClassRefPkgName)); } + + private String getXmlConfigFilePath(String fileName) { + return "com/opensymphony/xwork2/config/providers/" + fileName; + } } diff --git a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-package-extends-final.xml b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-package-extends-final.xml new file mode 100644 index 0000000000..6f01c8d154 --- /dev/null +++ b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-package-extends-final.xml @@ -0,0 +1,37 @@ + + + + + + + + + + + + + + + + diff --git a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-package-final.xml b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-package-final.xml new file mode 100644 index 0000000000..8fd96bcc19 --- /dev/null +++ b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-package-final.xml @@ -0,0 +1,37 @@ + + + + + + + + + + + + + + + + From 4088f2ee2b9d39bf7e61f88f4cd6c2919ccf066a Mon Sep 17 00:00:00 2001 From: Jeffery Xie Date: Thu, 11 Apr 2024 15:07:25 +1000 Subject: [PATCH 2/3] WW-5409 update new dtd from 6.4.0 to 6.5.0 --- .../apache/struts2/config/StrutsXmlConfigurationProvider.java | 2 +- .../src/main/resources/{struts-6.4.0.dtd => struts-6.5.0.dtd} | 4 ++-- .../config/providers/xwork-test-package-extends-final.xml | 4 ++-- .../xwork2/config/providers/xwork-test-package-final.xml | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) rename core/src/main/resources/{struts-6.4.0.dtd => struts-6.5.0.dtd} (96%) diff --git a/core/src/main/java/org/apache/struts2/config/StrutsXmlConfigurationProvider.java b/core/src/main/java/org/apache/struts2/config/StrutsXmlConfigurationProvider.java index d38b36cadd..f5a516dfaa 100644 --- a/core/src/main/java/org/apache/struts2/config/StrutsXmlConfigurationProvider.java +++ b/core/src/main/java/org/apache/struts2/config/StrutsXmlConfigurationProvider.java @@ -54,7 +54,7 @@ public class StrutsXmlConfigurationProvider extends XmlConfigurationProvider { put("-//Apache Software Foundation//DTD Struts Configuration 2.3//EN", "struts-2.3.dtd"); put("-//Apache Software Foundation//DTD Struts Configuration 2.5//EN", "struts-2.5.dtd"); put("-//Apache Software Foundation//DTD Struts Configuration 6.0//EN", "struts-6.0.dtd"); - put("-//Apache Software Foundation//DTD Struts Configuration 6.4.0//EN", "struts-6.4.0.dtd"); + put("-//Apache Software Foundation//DTD Struts Configuration 6.5.0//EN", "struts-6.5.0.dtd"); }}); private File baseDir = null; private final String filename; diff --git a/core/src/main/resources/struts-6.4.0.dtd b/core/src/main/resources/struts-6.5.0.dtd similarity index 96% rename from core/src/main/resources/struts-6.4.0.dtd rename to core/src/main/resources/struts-6.5.0.dtd index 222c29d242..bf1b534a91 100644 --- a/core/src/main/resources/struts-6.4.0.dtd +++ b/core/src/main/resources/struts-6.5.0.dtd @@ -26,8 +26,8 @@ Use the following DOCTYPE + "-//Apache Software Foundation//DTD Struts Configuration 6.5.0//EN" + "https://struts.apache.org/dtds/struts-6.5.0.dtd"> --> diff --git a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-package-extends-final.xml b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-package-extends-final.xml index 6f01c8d154..28b9526b9d 100644 --- a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-package-extends-final.xml +++ b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-package-extends-final.xml @@ -20,8 +20,8 @@ */ --> + "-//Apache Software Foundation//DTD Struts Configuration 6.5.0//EN" + "struts-6.5.0.dtd"> diff --git a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-package-final.xml b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-package-final.xml index 8fd96bcc19..12ac2b4fa2 100644 --- a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-package-final.xml +++ b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-package-final.xml @@ -20,8 +20,8 @@ */ --> + "-//Apache Software Foundation//DTD Struts Configuration 6.5.0//EN" + "struts-6.5.0.dtd"> From 85783a0cc757ae0c526e0b5ba7cc31f3a5b5a87e Mon Sep 17 00:00:00 2001 From: Jeffery Xie Date: Fri, 12 Apr 2024 16:04:34 +1000 Subject: [PATCH 3/3] WW-5409 rename 6.5.0.dtd to 6.5.dtd to follow the naming pattern --- .../apache/struts2/config/StrutsXmlConfigurationProvider.java | 2 +- core/src/main/resources/{struts-6.5.0.dtd => struts-6.5.dtd} | 4 ++-- .../config/providers/xwork-test-package-extends-final.xml | 4 ++-- .../xwork2/config/providers/xwork-test-package-final.xml | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) rename core/src/main/resources/{struts-6.5.0.dtd => struts-6.5.dtd} (98%) diff --git a/core/src/main/java/org/apache/struts2/config/StrutsXmlConfigurationProvider.java b/core/src/main/java/org/apache/struts2/config/StrutsXmlConfigurationProvider.java index f5a516dfaa..9c1b3948c2 100644 --- a/core/src/main/java/org/apache/struts2/config/StrutsXmlConfigurationProvider.java +++ b/core/src/main/java/org/apache/struts2/config/StrutsXmlConfigurationProvider.java @@ -54,7 +54,7 @@ public class StrutsXmlConfigurationProvider extends XmlConfigurationProvider { put("-//Apache Software Foundation//DTD Struts Configuration 2.3//EN", "struts-2.3.dtd"); put("-//Apache Software Foundation//DTD Struts Configuration 2.5//EN", "struts-2.5.dtd"); put("-//Apache Software Foundation//DTD Struts Configuration 6.0//EN", "struts-6.0.dtd"); - put("-//Apache Software Foundation//DTD Struts Configuration 6.5.0//EN", "struts-6.5.0.dtd"); + put("-//Apache Software Foundation//DTD Struts Configuration 6.5//EN", "struts-6.5.dtd"); }}); private File baseDir = null; private final String filename; diff --git a/core/src/main/resources/struts-6.5.0.dtd b/core/src/main/resources/struts-6.5.dtd similarity index 98% rename from core/src/main/resources/struts-6.5.0.dtd rename to core/src/main/resources/struts-6.5.dtd index bf1b534a91..6c2ef0d05b 100644 --- a/core/src/main/resources/struts-6.5.0.dtd +++ b/core/src/main/resources/struts-6.5.dtd @@ -26,8 +26,8 @@ Use the following DOCTYPE + "-//Apache Software Foundation//DTD Struts Configuration 6.5//EN" + "https://struts.apache.org/dtds/struts-6.5.dtd"> --> diff --git a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-package-extends-final.xml b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-package-extends-final.xml index 28b9526b9d..4e42422350 100644 --- a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-package-extends-final.xml +++ b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-package-extends-final.xml @@ -20,8 +20,8 @@ */ --> + "-//Apache Software Foundation//DTD Struts Configuration 6.5//EN" + "struts-6.5.dtd"> diff --git a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-package-final.xml b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-package-final.xml index 12ac2b4fa2..2a49a153b1 100644 --- a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-package-final.xml +++ b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-package-final.xml @@ -20,8 +20,8 @@ */ --> + "-//Apache Software Foundation//DTD Struts Configuration 6.5//EN" + "struts-6.5.dtd">