This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new 732de655e7 Various minor storeconfig fixes from code review
732de655e7 is described below
commit 732de655e706aea97dec8072497ef00cc25a4c0e
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 | 6 +-
.../catalina/storeconfig/LocalStrings.properties | 1 +
.../org/apache/catalina/storeconfig/ManagerSF.java | 69 ++++++++++++++++++++--
.../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, 130 insertions(+), 41 deletions(-)
diff --git a/java/org/apache/catalina/storeconfig/ConnectorStoreAppender.java
b/java/org/apache/catalina/storeconfig/ConnectorStoreAppender.java
index eb581573f6..95f57d19ff 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 and the attribute sslProtocol
need a special handling
+ * 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 {
@@ -153,6 +171,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];
@@ -294,6 +314,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 62d3918633..80f1c6ac4c 100644
--- a/java/org/apache/catalina/storeconfig/InterceptorSF.java
+++ b/java/org/apache/catalina/storeconfig/InterceptorSF.java
@@ -71,7 +71,7 @@ public class InterceptorSF extends StoreFactoryBase {
if (aInterceptor instanceof StaticMembershipInterceptor) {
ChannelInterceptor interceptor = (ChannelInterceptor) aInterceptor;
// Store nested <Member> elements
- storeElementArray(aWriter, indent + 2, interceptor.getMembers());
+ storeElementArray(aWriter, indent, interceptor.getMembers());
}
}
}
diff --git a/java/org/apache/catalina/storeconfig/LoaderSF.java
b/java/org/apache/catalina/storeconfig/LoaderSF.java
index b657358f5f..96db8b16b4 100644
--- a/java/org/apache/catalina/storeconfig/LoaderSF.java
+++ b/java/org/apache/catalina/storeconfig/LoaderSF.java
@@ -45,15 +45,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);
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 15988f2e26..fd28c92a7f 100644
--- a/java/org/apache/catalina/storeconfig/ManagerSF.java
+++ b/java/org/apache/catalina/storeconfig/ManagerSF.java
@@ -21,6 +21,8 @@ import java.io.PrintWriter;
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;
@@ -53,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()));
}
}
@@ -68,7 +68,68 @@ public class ManagerSF extends StoreFactoryBase {
*/
protected boolean isDefaultManager(StandardManager smanager) {
- return "SESSIONS.ser".equals(smanager.getPathname()) &&
(smanager.getMaxActiveSessions() == -1);
+ // StandardManager-specific property
+ if (!"SESSIONS.ser".equals(smanager.getPathname())) {
+ 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;
+ }
+ 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 fa67454d0f..cd7cf8553e 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 f1ebe06bd5..e66144e08a 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 bfc5e7e806..f48fe4ee49 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;
@@ -232,7 +233,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();
}
@@ -326,8 +326,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 4c69624ddb..e4fa966d83 100644
--- a/java/org/apache/catalina/storeconfig/StandardEngineSF.java
+++ b/java/org/apache/catalina/storeconfig/StandardEngineSF.java
@@ -65,12 +65,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);
}
@@ -92,10 +87,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);
-
}
}
}
diff --git a/java/org/apache/catalina/storeconfig/StoreAppender.java
b/java/org/apache/catalina/storeconfig/StoreAppender.java
index c5063b7fb1..8a56fad2c0 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
*/
@@ -223,7 +223,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
@@ -261,7 +261,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
@@ -279,7 +279,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]
