You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2026/10/09 09:56:59 UTC
(tomcat) 01/13: Fix storeconfig default-manager check always rejecting started managers
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 36020e7f936c37aab18caccaf0a0f266af054207
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 9 10:03:54 2026 +0200
Fix storeconfig default-manager check always rejecting started managers
ManagerBase.startInternal() always propagates the Engine's jvmRoute (null
by default) to the session ID generator, overwriting its empty-string
default. The isDefaultManager() check in ManagerSF compared the generator's
jvmRoute against "", so it returned false for every started StandardManager
and storeConfig wrote a redundant <Manager> element with a nested
<SessionIdGenerator> for every context using the default configuration.
Treat a generator jvmRoute that is empty or equal to the value the manager
propagates as runtime state rather than configuration, and treat a not yet
created generator as default. Since making the skip path reachable would
otherwise silently drop configuration, require the exact manager and
generator classes and check the persistable throwOnFailure property.
---
.../org/apache/catalina/storeconfig/ManagerSF.java | 54 +++--
.../catalina/storeconfig/TestStoreConfig.java | 249 +++++++++++++++++++++
webapps/docs/changelog.xml | 14 ++
3 files changed, 300 insertions(+), 17 deletions(-)
diff --git a/java/org/apache/catalina/storeconfig/ManagerSF.java b/java/org/apache/catalina/storeconfig/ManagerSF.java
index ee29281d94..ad38c89afd 100644
--- a/java/org/apache/catalina/storeconfig/ManagerSF.java
+++ b/java/org/apache/catalina/storeconfig/ManagerSF.java
@@ -68,11 +68,21 @@ public class ManagerSF extends StoreFactoryBase {
*/
protected boolean isDefaultManager(StandardManager smanager) {
+ // A custom sub-class is configured through its className attribute
+ if (!StandardManager.class.equals(smanager.getClass())) {
+ return false;
+ }
+
// StandardManager-specific property
if (smanager.getPathname() != null) {
return false;
}
+ // LifecycleBase property
+ if (!smanager.getThrowOnFailure()) {
+ return false;
+ }
+
// ManagerBase properties
if (smanager.getMaxActiveSessions() != -1) {
return false;
@@ -113,26 +123,36 @@ public class ManagerSF extends StoreFactoryBase {
if (smanager.getSessionLastAccessAtStart() != Globals.STRICT_SERVLET_COMPLIANCE) {
return false;
}
+ // A generator that has not been created yet is the default: the manager
+ // creates one when it starts
SessionIdGenerator sessionIdGenerator = smanager.getSessionIdGenerator();
- SessionIdGeneratorBase sigBase = null;
- if (sessionIdGenerator == null || !StandardSessionIdGenerator.class.isInstance(sessionIdGenerator)) {
+ if (sessionIdGenerator != null &&
+ !StandardSessionIdGenerator.class.equals(sessionIdGenerator.getClass())) {
return false;
}
- sigBase = (SessionIdGeneratorBase) sessionIdGenerator;
- if (!"".equals(sigBase.getJvmRoute())) {
- return false;
- }
- if (sigBase.getSecureRandomClass() != null) {
- return false;
- }
- if (!SessionIdGeneratorBase.DEFAULT_SECURE_RANDOM_ALGORITHM.equals(sigBase.getSecureRandomAlgorithm())) {
- return false;
- }
- if (sigBase.getSecureRandomProvider() != null) {
- return false;
- }
- if (sigBase.getSessionIdLength() != 16) {
- return false;
+ if (sessionIdGenerator instanceof SessionIdGeneratorBase sigBase) {
+ // The manager propagates its jvmRoute to the generator when it starts,
+ // so a value matching the manager's is runtime state, not configuration
+ String sigJvmRoute = sigBase.getJvmRoute();
+ if (sigJvmRoute != null && !sigJvmRoute.isEmpty() &&
+ !sigJvmRoute.equals(smanager.getJvmRoute())) {
+ return false;
+ }
+ if (!sigBase.getThrowOnFailure()) {
+ return false;
+ }
+ if (sigBase.getSecureRandomClass() != null) {
+ return false;
+ }
+ if (!SessionIdGeneratorBase.DEFAULT_SECURE_RANDOM_ALGORITHM.equals(sigBase.getSecureRandomAlgorithm())) {
+ return false;
+ }
+ if (sigBase.getSecureRandomProvider() != null) {
+ return false;
+ }
+ if (sigBase.getSessionIdLength() != 16) {
+ return false;
+ }
}
return true;
diff --git a/test/org/apache/catalina/storeconfig/TestStoreConfig.java b/test/org/apache/catalina/storeconfig/TestStoreConfig.java
index b287846b7d..9d97398bed 100644
--- a/test/org/apache/catalina/storeconfig/TestStoreConfig.java
+++ b/test/org/apache/catalina/storeconfig/TestStoreConfig.java
@@ -34,11 +34,13 @@ import org.apache.catalina.Host;
import org.apache.catalina.connector.Connector;
import org.apache.catalina.core.StandardContext;
import org.apache.catalina.realm.LockOutRealm;
+import org.apache.catalina.session.StandardManager;
import org.apache.catalina.startup.Catalina;
import org.apache.catalina.startup.CatalinaBaseConfigurationSource;
import org.apache.catalina.startup.Tomcat;
import org.apache.catalina.startup.TomcatBaseTest;
import org.apache.catalina.util.IOTools;
+import org.apache.catalina.util.SessionIdGeneratorBase;
import org.apache.catalina.valves.AccessLogValve;
import org.apache.tomcat.util.net.SSLHostConfigPreSharedKey;
import org.xml.sax.InputSource;
@@ -330,6 +332,253 @@ public class TestStoreConfig extends TomcatBaseTest {
.parse(new InputSource(new StringReader(contextXmlDump)));
}
+ /**
+ * Verify that the <Manager> element is not stored for a running context using the default StandardManager
+ * configuration. The jvmRoute of the session ID generator is runtime state that the manager propagates from the
+ * Engine when it starts and must not defeat the default detection.
+ *
+ * @throws Exception if the test experiences an unexpected error
+ */
+ @Test
+ public void testDefaultManagerNotStored() throws Exception {
+ Tomcat tomcat = getTomcatInstance();
+ StoreConfigLifecycleListener storeConfigListener = new StoreConfigLifecycleListener();
+ tomcat.getServer().addLifecycleListener(storeConfigListener);
+
+ // Use a storable realm. The default embedded realm (Tomcat.SimpleRealm) is an inner class that the store
+ // path cannot instantiate a default instance of.
+ tomcat.getEngine().setRealm(new LockOutRealm());
+
+ File appDir = new File(getTemporaryDirectory(), "webapps/defmgr");
+ if (!appDir.mkdirs()) {
+ Assert.fail("Unable to create the webapp directory");
+ }
+ Context context = tomcat.addContext("/defmgr", appDir.getAbsolutePath());
+ ((StandardContext) context).setDeployedFromServerXml(true);
+
+ File conf = new File(getTemporaryDirectory(), "conf");
+ if (!conf.isDirectory() && !conf.mkdirs()) {
+ Assert.fail("Unable to create conf directory");
+ }
+ addDeleteOnTearDown(conf);
+
+ tomcat.start();
+
+ // Undo the FastNonSecureRandom configuration the test base applies to every manager, so the started manager
+ // is in the same (default) state as it would be on a regular server
+ StandardManager manager = (StandardManager) ((StandardContext) context).getManager();
+ manager.setSecureRandomClass(null);
+ ((SessionIdGeneratorBase) manager.getSessionIdGenerator()).setSecureRandomClass(null);
+
+ IStoreConfig storeConfig = storeConfigListener.getStoreConfig();
+ StoreDescription desc = storeConfig.getRegistry().findDescription(StandardContext.class);
+ Assert.assertNotNull(desc);
+ boolean oldSeparate = desc.isStoreSeparate();
+ boolean oldExternalAllowed = desc.isExternalAllowed();
+ boolean oldExternalOnly = desc.isExternalOnly();
+ String serverXmlDump;
+ try {
+ desc.setStoreSeparate(true);
+ desc.setExternalAllowed(true);
+ desc.setExternalOnly(false);
+ StringWriter buffer = new StringWriter();
+ storeConfig.store(new PrintWriter(buffer), -2, tomcat.getServer());
+ serverXmlDump = buffer.toString();
+ } finally {
+ desc.setStoreSeparate(oldSeparate);
+ desc.setExternalAllowed(oldExternalAllowed);
+ desc.setExternalOnly(oldExternalOnly);
+ }
+
+ Assert.assertTrue(serverXmlDump, serverXmlDump.contains("path=\"/defmgr\""));
+ // The manager of the started context is at its default configuration, so no <Manager> element may be stored
+ Assert.assertFalse(serverXmlDump, serverXmlDump.contains("<Manager"));
+ Assert.assertFalse(serverXmlDump, serverXmlDump.contains("StandardManager"));
+ Assert.assertFalse(serverXmlDump, serverXmlDump.contains("SessionIdGenerator"));
+ }
+
+ /**
+ * Verify that a <Manager> element is still stored for a context whose StandardManager deviates from the
+ * default configuration.
+ *
+ * @throws Exception if the test experiences an unexpected error
+ */
+ @Test
+ public void testNonDefaultManagerStored() throws Exception {
+ Tomcat tomcat = getTomcatInstance();
+ StoreConfigLifecycleListener storeConfigListener = new StoreConfigLifecycleListener();
+ tomcat.getServer().addLifecycleListener(storeConfigListener);
+
+ // Use a storable realm. The default embedded realm (Tomcat.SimpleRealm) is an inner class that the store
+ // path cannot instantiate a default instance of.
+ tomcat.getEngine().setRealm(new LockOutRealm());
+
+ File appDir = new File(getTemporaryDirectory(), "webapps/nondflmgr");
+ if (!appDir.mkdirs()) {
+ Assert.fail("Unable to create the webapp directory");
+ }
+ Context context = tomcat.addContext("/nondflmgr", appDir.getAbsolutePath());
+ ((StandardContext) context).setDeployedFromServerXml(true);
+ StandardManager manager = new StandardManager();
+ manager.setMaxActiveSessions(100);
+ ((StandardContext) context).setManager(manager);
+
+ File conf = new File(getTemporaryDirectory(), "conf");
+ if (!conf.isDirectory() && !conf.mkdirs()) {
+ Assert.fail("Unable to create conf directory");
+ }
+ addDeleteOnTearDown(conf);
+
+ tomcat.start();
+
+ IStoreConfig storeConfig = storeConfigListener.getStoreConfig();
+ StoreDescription desc = storeConfig.getRegistry().findDescription(StandardContext.class);
+ Assert.assertNotNull(desc);
+ boolean oldSeparate = desc.isStoreSeparate();
+ boolean oldExternalAllowed = desc.isExternalAllowed();
+ boolean oldExternalOnly = desc.isExternalOnly();
+ String serverXmlDump;
+ try {
+ desc.setStoreSeparate(true);
+ desc.setExternalAllowed(true);
+ desc.setExternalOnly(false);
+ StringWriter buffer = new StringWriter();
+ storeConfig.store(new PrintWriter(buffer), -2, tomcat.getServer());
+ serverXmlDump = buffer.toString();
+ } finally {
+ desc.setStoreSeparate(oldSeparate);
+ desc.setExternalAllowed(oldExternalAllowed);
+ desc.setExternalOnly(oldExternalOnly);
+ }
+
+ Assert.assertTrue(serverXmlDump, serverXmlDump.contains("<Manager"));
+ Assert.assertTrue(serverXmlDump, serverXmlDump.contains("maxActiveSessions=\"100\""));
+ }
+
+ /**
+ * Verify that a <Manager> element with a StandardManager sub-class is still stored, since the sub-class is
+ * configured through its class name even when all inherited properties are at their defaults.
+ *
+ * @throws Exception if the test experiences an unexpected error
+ */
+ @Test
+ public void testSubClassManagerStored() throws Exception {
+ Tomcat tomcat = getTomcatInstance();
+ StoreConfigLifecycleListener storeConfigListener = new StoreConfigLifecycleListener();
+ tomcat.getServer().addLifecycleListener(storeConfigListener);
+
+ // Use a storable realm. The default embedded realm (Tomcat.SimpleRealm) is an inner class that the store
+ // path cannot instantiate a default instance of.
+ tomcat.getEngine().setRealm(new LockOutRealm());
+
+ File appDir = new File(getTemporaryDirectory(), "webapps/subcmgr");
+ if (!appDir.mkdirs()) {
+ Assert.fail("Unable to create the webapp directory");
+ }
+ Context context = tomcat.addContext("/subcmgr", appDir.getAbsolutePath());
+ ((StandardContext) context).setDeployedFromServerXml(true);
+ ((StandardContext) context).setManager(new CustomTestManager());
+
+ File conf = new File(getTemporaryDirectory(), "conf");
+ if (!conf.isDirectory() && !conf.mkdirs()) {
+ Assert.fail("Unable to create conf directory");
+ }
+ addDeleteOnTearDown(conf);
+
+ tomcat.start();
+
+ IStoreConfig storeConfig = storeConfigListener.getStoreConfig();
+ StoreDescription desc = storeConfig.getRegistry().findDescription(StandardContext.class);
+ Assert.assertNotNull(desc);
+ boolean oldSeparate = desc.isStoreSeparate();
+ boolean oldExternalAllowed = desc.isExternalAllowed();
+ boolean oldExternalOnly = desc.isExternalOnly();
+ String serverXmlDump;
+ try {
+ desc.setStoreSeparate(true);
+ desc.setExternalAllowed(true);
+ desc.setExternalOnly(false);
+ StringWriter buffer = new StringWriter();
+ storeConfig.store(new PrintWriter(buffer), -2, tomcat.getServer());
+ serverXmlDump = buffer.toString();
+ } finally {
+ desc.setStoreSeparate(oldSeparate);
+ desc.setExternalAllowed(oldExternalAllowed);
+ desc.setExternalOnly(oldExternalOnly);
+ }
+
+ Assert.assertTrue(serverXmlDump, serverXmlDump.contains("<Manager"));
+ Assert.assertTrue(serverXmlDump, serverXmlDump.contains(CustomTestManager.class.getName()));
+ }
+
+ /**
+ * Verify that a <Manager> element is stored when the only deviation from the default configuration is a
+ * non-default <code>throwOnFailure</code> value, which is a persistable property that must not be lost.
+ *
+ * @throws Exception if the test experiences an unexpected error
+ */
+ @Test
+ public void testManagerThrowOnFailureStored() throws Exception {
+ Tomcat tomcat = getTomcatInstance();
+ StoreConfigLifecycleListener storeConfigListener = new StoreConfigLifecycleListener();
+ tomcat.getServer().addLifecycleListener(storeConfigListener);
+
+ // Use a storable realm. The default embedded realm (Tomcat.SimpleRealm) is an inner class that the store
+ // path cannot instantiate a default instance of.
+ tomcat.getEngine().setRealm(new LockOutRealm());
+
+ File appDir = new File(getTemporaryDirectory(), "webapps/tofmgr");
+ if (!appDir.mkdirs()) {
+ Assert.fail("Unable to create the webapp directory");
+ }
+ Context context = tomcat.addContext("/tofmgr", appDir.getAbsolutePath());
+ ((StandardContext) context).setDeployedFromServerXml(true);
+
+ File conf = new File(getTemporaryDirectory(), "conf");
+ if (!conf.isDirectory() && !conf.mkdirs()) {
+ Assert.fail("Unable to create conf directory");
+ }
+ addDeleteOnTearDown(conf);
+
+ tomcat.start();
+
+ // Undo the FastNonSecureRandom configuration the test base applies to every manager, so throwOnFailure is the
+ // only deviation from the default manager configuration
+ StandardManager manager = (StandardManager) ((StandardContext) context).getManager();
+ manager.setSecureRandomClass(null);
+ ((SessionIdGeneratorBase) manager.getSessionIdGenerator()).setSecureRandomClass(null);
+ manager.setThrowOnFailure(false);
+
+ IStoreConfig storeConfig = storeConfigListener.getStoreConfig();
+ StoreDescription desc = storeConfig.getRegistry().findDescription(StandardContext.class);
+ Assert.assertNotNull(desc);
+ boolean oldSeparate = desc.isStoreSeparate();
+ boolean oldExternalAllowed = desc.isExternalAllowed();
+ boolean oldExternalOnly = desc.isExternalOnly();
+ String serverXmlDump;
+ try {
+ desc.setStoreSeparate(true);
+ desc.setExternalAllowed(true);
+ desc.setExternalOnly(false);
+ StringWriter buffer = new StringWriter();
+ storeConfig.store(new PrintWriter(buffer), -2, tomcat.getServer());
+ serverXmlDump = buffer.toString();
+ } finally {
+ desc.setStoreSeparate(oldSeparate);
+ desc.setExternalAllowed(oldExternalAllowed);
+ desc.setExternalOnly(oldExternalOnly);
+ }
+
+ Assert.assertTrue(serverXmlDump, serverXmlDump.contains("<Manager"));
+ Assert.assertTrue(serverXmlDump, serverXmlDump.contains("throwOnFailure=\"false\""));
+ }
+
+ public static class CustomTestManager extends StandardManager {
+
+ public CustomTestManager() {
+ }
+ }
+
/**
* Verify that a Context parsed from a Context element in server.xml is flagged as deployed from server.xml, so
* storeconfig can detect it. The flag must also be set when server.xml is processed through the generated code
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 02656435bc..e9c553a1c7 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -333,6 +333,20 @@
a warning in the temporary location only in the rare cases where
restoring it is not possible. (remm)
</fix>
+ <fix>
+ Fix the <code>ManagerSF</code> store configuration factory always
+ treating a started <code>StandardManager</code> as non-default, since
+ the manager propagates its (possibly <code>null</code>) node identifier
+ to its session ID generator when it starts, which the default check
+ compared against an empty string. This caused a redundant
+ <code>Manager</code> element with a nested
+ <code>SessionIdGenerator</code> to be written for every context using
+ the default manager configuration when the running configuration was
+ stored. The default check was strengthened to require the exact
+ manager and session ID generator classes and to test
+ <code>throwOnFailure</code>, so the now reachable skip path cannot
+ silently drop a customised configuration. (remm)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org