This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/11.0.x by this push:
new 96ecbd0e0d Various minor storeconfig fixes from code review
96ecbd0e0d is described below
commit 96ecbd0e0de3f776191634ab898f648ce8b107b1
Author: remm <[email protected]>
AuthorDate: Tue Jun 30 16:21:18 2026 +0200
Various minor storeconfig fixes from code review
---
.../storeconfig/ConnectorStoreAppender.java | 29 ++++++++-
.../apache/catalina/storeconfig/InterceptorSF.java | 2 +-
java/org/apache/catalina/storeconfig/LoaderSF.java | 8 +--
.../catalina/storeconfig/LocalStrings.properties | 1 +
.../org/apache/catalina/storeconfig/ManagerSF.java | 76 ++++++++++++++++++++--
.../catalina/storeconfig/NamingResourcesSF.java | 2 +-
java/org/apache/catalina/storeconfig/RealmSF.java | 4 +-
.../catalina/storeconfig/StandardContextSF.java | 15 ++++-
.../catalina/storeconfig/StandardEngineSF.java | 9 +--
.../apache/catalina/storeconfig/StoreAppender.java | 8 +--
.../catalina/storeconfig/StoreContextAppender.java | 2 +-
.../catalina/storeconfig/StoreDescription.java | 12 ++++
.../catalina/storeconfig/StoreFactoryBase.java | 2 +-
.../apache/catalina/storeconfig/StoreLoader.java | 10 +--
14 files changed, 138 insertions(+), 42 deletions(-)
diff --git a/java/org/apache/catalina/storeconfig/ConnectorStoreAppender.java
b/java/org/apache/catalina/storeconfig/ConnectorStoreAppender.java
index b42d7649c0..4e274aad33 100644
--- a/java/org/apache/catalina/storeconfig/ConnectorStoreAppender.java
+++ b/java/org/apache/catalina/storeconfig/ConnectorStoreAppender.java
@@ -36,9 +36,27 @@ import org.apache.tomcat.util.IntrospectionUtils;
import org.apache.tomcat.util.net.SocketProperties;
/**
- * Store the Connector attributes. Connector has really special design. A
Connector is only a startup Wrapper for a
- * ProtocolHandler. This meant that ProtocolHandler get all there attributes
from the Connector attribute map. Strange
- * is that some attributes change their name.
+ * Specialized {@link StoreAppender} for persisting
+ * {@link org.apache.catalina.connector.Connector} instances to XML
configuration.
+ * <p>
+ * A {@code Connector} acts as a wrapper around a {@link
org.apache.coyote.ProtocolHandler},
+ * meaning configuration attributes are spread across both objects. This
appender
+ * collects properties from the Connector, its ProtocolHandler, and its
+ * {@link org.apache.tomcat.util.net.SocketProperties}, applying the following
+ * transformations during persistence:
+ * </p>
+ * <ul>
+ * <li>Maps certain attribute names to their XML equivalents (e.g., {@code
timeout} to
+ * {@code connectionUploadTimeout})</li>
+ * <li>Filters internal executor attributes when an external executor is
configured</li>
+ * <li>Prefixes socket properties with {@code socket.}</li>
+ * <li>Suppresses default values by comparing against a freshly created
Connector</li>
+ * <li>Handles special cases such as {@code jkHome} and catalina.base path
resolution</li>
+ * </ul>
+ *
+ * @see org.apache.catalina.connector.Connector
+ * @see org.apache.coyote.ProtocolHandler
+ * @see StoreAppender
*/
public class ConnectorStoreAppender extends StoreAppender {
@@ -147,6 +165,8 @@ public class ConnectorStoreAppender extends StoreAppender {
// Add the properties of the protocol handler
if (protocolHandler != null) {
descriptors =
Introspector.getBeanInfo(protocolHandler.getClass()).getPropertyDescriptors();
+ } else {
+ descriptors = null;
}
if (descriptors == null) {
descriptors = new PropertyDescriptor[0];
@@ -288,6 +308,9 @@ public class ConnectorStoreAppender extends StoreAppender {
protected File getJkHomeBase(String jkHome, File appBase) {
File jkHomeBase;
+ if (jkHome == null) {
+ return null;
+ }
File file = new File(jkHome);
if (!file.isAbsolute()) {
file = new File(appBase, jkHome);
diff --git a/java/org/apache/catalina/storeconfig/InterceptorSF.java
b/java/org/apache/catalina/storeconfig/InterceptorSF.java
index a859bbca46..0140a20cef 100644
--- a/java/org/apache/catalina/storeconfig/InterceptorSF.java
+++ b/java/org/apache/catalina/storeconfig/InterceptorSF.java
@@ -69,7 +69,7 @@ public class InterceptorSF extends StoreFactoryBase {
throws Exception {
if (aInterceptor instanceof StaticMembershipInterceptor interceptor) {
// Store nested <Member> elements
- storeElementArray(aWriter, indent + 2, interceptor.getMembers());
+ storeElementArray(aWriter, indent, interceptor.getMembers());
}
}
}
\ No newline at end of file
diff --git a/java/org/apache/catalina/storeconfig/LoaderSF.java
b/java/org/apache/catalina/storeconfig/LoaderSF.java
index 0b67d8c6c9..336a6403b3 100644
--- a/java/org/apache/catalina/storeconfig/LoaderSF.java
+++ b/java/org/apache/catalina/storeconfig/LoaderSF.java
@@ -44,15 +44,13 @@ public class LoaderSF extends StoreFactoryBase {
if (elementDesc != null) {
if (!isDefaultLoader(loader)) {
if (log.isTraceEnabled()) {
- log.trace("store " + elementDesc.getTag() + "( " +
aElement + " )");
+ log.trace(sm.getString("factory.storeTag",
elementDesc.getTag(), aElement));
}
getStoreAppender().printIndent(aWriter, indent + 2);
getStoreAppender().printTag(aWriter, indent + 2, loader,
elementDesc);
}
} else {
- if (log.isWarnEnabled()) {
- log.warn(sm.getString("factory.storeNoDescriptor",
aElement.getClass()));
- }
+ log.warn(sm.getString("factory.storeNoDescriptor",
aElement.getClass()));
}
} else {
super.store(aWriter, indent, aElement);
@@ -71,7 +69,7 @@ public class LoaderSF extends StoreFactoryBase {
if (!(loader instanceof WebappLoader wloader)) {
return false;
}
- return (!wloader.getDelegate()) &&
+ return (!wloader.getDelegate()) && wloader.getJakartaConverter() ==
null &&
wloader.getLoaderClass().equals(ParallelWebappClassLoader.class.getName());
}
}
diff --git a/java/org/apache/catalina/storeconfig/LocalStrings.properties
b/java/org/apache/catalina/storeconfig/LocalStrings.properties
index 064dfcf71e..1befcfc471 100644
--- a/java/org/apache/catalina/storeconfig/LocalStrings.properties
+++ b/java/org/apache/catalina/storeconfig/LocalStrings.properties
@@ -37,6 +37,7 @@ registry.optionalClassNotFound=Optional class [{0}] not
found, skipping
standardContextSF.cannotWriteFile=Cannot write file at [{0}]
standardContextSF.canonicalPathError=Failed to obtain the canonical path of
the configuration file [{0}]
standardContextSF.moveFailed=Context original file at [{0}] is null, not a
file or not writable
+standardContextSF.nonFileConfigUrl=The config URL [{0}] is not file based
standardContextSF.storeContext=Store context [{0}] configuration separately at
path [{1}]
standardContextSF.storeContextWithBackup=Store context [{0}] configuration
separately with backup at path [{1}]
diff --git a/java/org/apache/catalina/storeconfig/ManagerSF.java
b/java/org/apache/catalina/storeconfig/ManagerSF.java
index c10c89b5fe..ee29281d94 100644
--- a/java/org/apache/catalina/storeconfig/ManagerSF.java
+++ b/java/org/apache/catalina/storeconfig/ManagerSF.java
@@ -18,9 +18,12 @@ package org.apache.catalina.storeconfig;
import java.io.PrintWriter;
+import org.apache.catalina.Globals;
import org.apache.catalina.Manager;
import org.apache.catalina.SessionIdGenerator;
import org.apache.catalina.session.StandardManager;
+import org.apache.catalina.util.SessionIdGeneratorBase;
+import org.apache.catalina.util.StandardSessionIdGenerator;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
@@ -52,9 +55,7 @@ public class ManagerSF extends StoreFactoryBase {
super.store(aWriter, indent, aElement);
}
} else {
- if (log.isWarnEnabled()) {
- log.warn(sm.getString("factory.storeNoDescriptor",
aElement.getClass()));
- }
+ log.warn(sm.getString("factory.storeNoDescriptor",
aElement.getClass()));
}
}
@@ -67,7 +68,74 @@ public class ManagerSF extends StoreFactoryBase {
*/
protected boolean isDefaultManager(StandardManager smanager) {
- return smanager.getPathname() == null &&
(smanager.getMaxActiveSessions() == -1);
+ // StandardManager-specific property
+ if (smanager.getPathname() != null) {
+ return false;
+ }
+
+ // ManagerBase properties
+ if (smanager.getMaxActiveSessions() != -1) {
+ return false;
+ }
+ if (smanager.getSecureRandomClass() != null) {
+ return false;
+ }
+ if
(!SessionIdGeneratorBase.DEFAULT_SECURE_RANDOM_ALGORITHM.equals(smanager.getSecureRandomAlgorithm()))
{
+ return false;
+ }
+ if (smanager.getSecureRandomProvider() != null) {
+ return false;
+ }
+ if (smanager.getProcessExpiresFrequency() != 6) {
+ return false;
+ }
+ if (smanager.getSessionAttributeNameFilter() != null) {
+ return false;
+ }
+ if (smanager.getSessionAttributeValueClassNameFilter() != null) {
+ return false;
+ }
+ if (smanager.getWarnOnSessionAttributeFilterFailure()) {
+ return false;
+ }
+ if (smanager.getNotifyBindingListenerOnUnchangedValue()) {
+ return false;
+ }
+ if (!smanager.getNotifyAttributeListenerOnUnchangedValue()) {
+ return false;
+ }
+ if (smanager.getPersistAuthentication()) {
+ return false;
+ }
+ if (smanager.getSessionActivityCheck() !=
Globals.STRICT_SERVLET_COMPLIANCE) {
+ return false;
+ }
+ if (smanager.getSessionLastAccessAtStart() !=
Globals.STRICT_SERVLET_COMPLIANCE) {
+ return false;
+ }
+ SessionIdGenerator sessionIdGenerator =
smanager.getSessionIdGenerator();
+ SessionIdGeneratorBase sigBase = null;
+ if (sessionIdGenerator == null ||
!StandardSessionIdGenerator.class.isInstance(sessionIdGenerator)) {
+ 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;
+ }
+
+ return true;
}
diff --git a/java/org/apache/catalina/storeconfig/NamingResourcesSF.java
b/java/org/apache/catalina/storeconfig/NamingResourcesSF.java
index 3d4af855e3..650c92038f 100644
--- a/java/org/apache/catalina/storeconfig/NamingResourcesSF.java
+++ b/java/org/apache/catalina/storeconfig/NamingResourcesSF.java
@@ -46,7 +46,7 @@ public class NamingResourcesSF extends StoreFactoryBase {
StoreDescription elementDesc =
getRegistry().findDescription(aElement.getClass());
if (elementDesc != null) {
if (log.isTraceEnabled()) {
- log.trace("store " + elementDesc.getTag() + "( " + aElement +
" )");
+ log.trace(sm.getString("factory.storeTag",
elementDesc.getTag(), aElement));
}
// Note: The elements go directly inside another one without any
specific parent
storeChildren(aWriter, indent, aElement, elementDesc);
diff --git a/java/org/apache/catalina/storeconfig/RealmSF.java
b/java/org/apache/catalina/storeconfig/RealmSF.java
index c543e4924a..5f98e7dc88 100644
--- a/java/org/apache/catalina/storeconfig/RealmSF.java
+++ b/java/org/apache/catalina/storeconfig/RealmSF.java
@@ -53,9 +53,7 @@ public class RealmSF extends StoreFactoryBase {
getStoreAppender().printIndent(aWriter, indent + 2);
getStoreAppender().printCloseTag(aWriter, elementDesc);
} else {
- if (log.isWarnEnabled()) {
- log.warn(sm.getString("factory.storeNoDescriptor",
aElement.getClass()));
- }
+ log.warn(sm.getString("factory.storeNoDescriptor",
aElement.getClass()));
}
} else {
super.store(aWriter, indent, aElement);
diff --git a/java/org/apache/catalina/storeconfig/StandardContextSF.java
b/java/org/apache/catalina/storeconfig/StandardContextSF.java
index a3076673c8..de30be7e4f 100644
--- a/java/org/apache/catalina/storeconfig/StandardContextSF.java
+++ b/java/org/apache/catalina/storeconfig/StandardContextSF.java
@@ -21,6 +21,7 @@ import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStreamWriter;
import java.io.PrintWriter;
+import java.net.URISyntaxException;
import java.net.URL;
import java.util.ArrayList;
import java.util.List;
@@ -231,7 +232,6 @@ public class StandardContextSF extends StoreFactoryBase {
Realm realm = context.getRealm();
if (realm != null) {
Realm parentRealm = null;
- // @TODO is this case possible?
if (context.getParent() != null) {
parentRealm = context.getParent().getRealm();
}
@@ -325,8 +325,17 @@ public class StandardContextSF extends StoreFactoryBase {
new File(System.getProperty(Globals.CATALINA_BASE_PROP),
"conf/context.xml").getCanonicalPath();
String confWeb = new
File(System.getProperty(Globals.CATALINA_BASE_PROP),
"conf/web.xml").getCanonicalPath();
String confHostDefault = new File(configBase,
"context.xml.default").getCanonicalPath();
- String configFile =
- (context.getConfigFile() != null ? new
File(context.getConfigFile().toURI()).getCanonicalPath() : null);
+ String configFile = null;
+ if (context.getConfigFile() != null) {
+ try {
+ configFile = new
File(context.getConfigFile().toURI()).getCanonicalPath();
+ } catch (URISyntaxException | IllegalArgumentException e) {
+ // Non-file scheme URL (e.g., jar:), skip configFile comparison
+ if (log.isDebugEnabled()) {
+
log.debug(sm.getString("standardContextSF.nonFileConfigUrl",
context.getConfigFile()), e);
+ }
+ }
+ }
String webxml = "WEB-INF/web.xml";
String tomcatwebxml = "WEB-INF/tomcat-web.xml";
diff --git a/java/org/apache/catalina/storeconfig/StandardEngineSF.java
b/java/org/apache/catalina/storeconfig/StandardEngineSF.java
index 7621671e92..d0a35dbc90 100644
--- a/java/org/apache/catalina/storeconfig/StandardEngineSF.java
+++ b/java/org/apache/catalina/storeconfig/StandardEngineSF.java
@@ -64,12 +64,7 @@ public class StandardEngineSF extends StoreFactoryBase {
// Store nested <Realm> element
Realm realm = engine.getRealm();
- Realm parentRealm = null;
- // TODO is this case possible? (see it a old Server 5.0 impl)
- if (engine.getParent() != null) {
- parentRealm = engine.getParent().getRealm();
- }
- if (realm != parentRealm) {
+ if (realm != null) {
storeElement(aWriter, indent, realm);
}
@@ -91,10 +86,10 @@ public class StandardEngineSF extends StoreFactoryBase {
if (cluster != null) {
storeElement(aWriter, indent, cluster);
}
+
// store all <Host> elements
Container[] children = engine.findChildren();
storeElementArray(aWriter, indent, children);
-
}
}
}
\ No newline at end of file
diff --git a/java/org/apache/catalina/storeconfig/StoreAppender.java
b/java/org/apache/catalina/storeconfig/StoreAppender.java
index 41571a750d..80ef3a803a 100644
--- a/java/org/apache/catalina/storeconfig/StoreAppender.java
+++ b/java/org/apache/catalina/storeconfig/StoreAppender.java
@@ -174,7 +174,7 @@ public class StoreAppender {
* @param indent Indentation level
* @param include Should we include a <code>className</code> attribute?
* @param bean Bean whose properties are to be rendered as attributes,
- * @param desc RegistryDescriptor from this bean
+ * @param desc Store description from this bean
*
* @exception Exception if an exception occurs while storing
*/
@@ -222,7 +222,7 @@ public class StoreAppender {
/**
* Check if the attribute should be printed.
*
- * @param desc RegistryDescriptor from this bean
+ * @param desc Store description from this bean
* @param descriptor PropertyDescriptor from this bean property
* @param attributeName The attribute name to store
* @param bean The current bean
@@ -260,7 +260,7 @@ public class StoreAppender {
* @param writer PrintWriter to which we are storing
* @param indent Indentation level
* @param bean The current bean
- * @param desc RegistryDescriptor from this bean
+ * @param desc Store description from this bean
* @param attributeName The attribute name to store
* @param bean2 A default instance of the bean for comparison
* @param value The attribute value
@@ -278,7 +278,7 @@ public class StoreAppender {
* @param bean original bean
* @param bean2 default bean
* @param attrName attribute name
- * @param desc StoreDescription from bean
+ * @param desc Store description from this bean
*
* @return <code>true</code> if the value should be stored
*/
diff --git a/java/org/apache/catalina/storeconfig/StoreContextAppender.java
b/java/org/apache/catalina/storeconfig/StoreContextAppender.java
index 5049b59049..b49e43f713 100644
--- a/java/org/apache/catalina/storeconfig/StoreContextAppender.java
+++ b/java/org/apache/catalina/storeconfig/StoreContextAppender.java
@@ -67,7 +67,7 @@ public class StoreContextAppender extends StoreAppender {
@Override
public boolean isPrintValue(Object bean, Object bean2, String attrName,
StoreDescription desc) {
boolean isPrint = super.isPrintValue(bean, bean2, attrName, desc);
- if (isPrint) {
+ if (isPrint && bean instanceof StandardContext) {
StandardContext context = ((StandardContext) bean);
if ("workDir".equals(attrName)) {
String defaultWorkDir = getDefaultWorkDir(context);
diff --git a/java/org/apache/catalina/storeconfig/StoreDescription.java
b/java/org/apache/catalina/storeconfig/StoreDescription.java
index a8edcde81c..c7ec6cf694 100644
--- a/java/org/apache/catalina/storeconfig/StoreDescription.java
+++ b/java/org/apache/catalina/storeconfig/StoreDescription.java
@@ -222,7 +222,9 @@ public class StoreDescription {
* Returns the fully qualified class name of the StoreWriter
implementation.
*
* @return the StoreWriter class name
+ * @deprecated Will be removed in Tomcat 12
*/
+ @Deprecated
public String getStoreWriterClass() {
return storeWriterClass;
}
@@ -231,7 +233,9 @@ public class StoreDescription {
* Sets the fully qualified class name of the StoreWriter implementation.
*
* @param storeWriterClass the StoreWriter class name
+ * @deprecated Will be removed in Tomcat 12
*/
+ @Deprecated
public void setStoreWriterClass(String storeWriterClass) {
this.storeWriterClass = storeWriterClass;
}
@@ -276,7 +280,9 @@ public class StoreDescription {
* Returns the list of attribute names that should not be persisted.
*
* @return the list of transient attribute names
+ * @deprecated Will be removed in Tomcat 12
*/
+ @Deprecated
public List<String> getTransientAttributes() {
return transientAttributes;
}
@@ -285,7 +291,9 @@ public class StoreDescription {
* Sets the list of attribute names that should not be persisted.
*
* @param transientAttributes the list of transient attribute names
+ * @deprecated Will be removed in Tomcat 12
*/
+ @Deprecated
public void setTransientAttributes(List<String> transientAttributes) {
this.transientAttributes = transientAttributes;
}
@@ -317,7 +325,9 @@ public class StoreDescription {
* Returns the list of child class names that should not be persisted.
*
* @return the list of transient child class names
+ * @deprecated Will be removed in Tomcat 12
*/
+ @Deprecated
public List<String> getTransientChildren() {
return transientChildren;
}
@@ -326,7 +336,9 @@ public class StoreDescription {
* Sets the list of child class names that should not be persisted.
*
* @param transientChildren the list of transient child class names
+ * @deprecated Will be removed in Tomcat 12
*/
+ @Deprecated
public void setTransientChildren(List<String> transientChildren) {
this.transientChildren = transientChildren;
}
diff --git a/java/org/apache/catalina/storeconfig/StoreFactoryBase.java
b/java/org/apache/catalina/storeconfig/StoreFactoryBase.java
index 626a5e6642..e1aa4d5900 100644
--- a/java/org/apache/catalina/storeconfig/StoreFactoryBase.java
+++ b/java/org/apache/catalina/storeconfig/StoreFactoryBase.java
@@ -47,7 +47,7 @@ public class StoreFactoryBase implements IStoreFactory {
/**
* The descriptive information string for this implementation.
*/
- private static final String info =
"org.apache.catalina.config.StoreFactoryBase/1.0";
+ private static final String info =
"org.apache.catalina.storeconfig.StoreFactoryBase/1.0";
/**
* Returns descriptive information about this Factory implementation and
the corresponding version number, in the
diff --git a/java/org/apache/catalina/storeconfig/StoreLoader.java
b/java/org/apache/catalina/storeconfig/StoreLoader.java
index 1ccb0e22ef..7150495e61 100644
--- a/java/org/apache/catalina/storeconfig/StoreLoader.java
+++ b/java/org/apache/catalina/storeconfig/StoreLoader.java
@@ -50,19 +50,11 @@ import
org.apache.tomcat.util.file.ConfigurationSource.Resource;
*
* Convention:
* <ul>
- * <li>Factories at subpackage <i>org.apache.catalina.core.storeconfig.xxxSF
</i>.</li>
+ * <li>Factories at subpackage
<i>org.apache.catalina.storeconfig.xxxSF</i>.</li>
* <li>Element name are the unique Class name</li>
* <li>SF for StoreFactory</li>
* <li>standard implementation is false</li>
* </ul>
- * other things:
- * <ul>
- * <li>Registry XML format is a very good option</li>
- * <li>Store format is not fix</li>
- * <li>We hope with the parent declaration we can build recursive child store
operation //dream</li>
- * <li>Problem is to access child data from array,collections or normal detail
object</li>
- * <li>Default definitions for Listener, Valve Resource? - Based on interface
type!</li>
- * </ul>
*/
public class StoreLoader {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
