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
The following commit(s) were added to refs/heads/main by this push:
new be63410148 Trivial fixes from code review
be63410148 is described below
commit be634101482376ed50d6964e39ab0ecf8a95b575
Author: remm <[email protected]>
AuthorDate: Mon Jun 29 21:47:37 2026 +0200
Trivial fixes from code review
The FailedContext changes are not necessary, but what if.
Make some package private fields private instead (some came from a
strange commit from Violeta from code review back then - was complaining
that protected inside a final class did not make sense).
---
java/org/apache/catalina/startup/Catalina.java | 1 +
.../catalina/startup/CatalinaProperties.java | 4 +-
.../catalina/startup/ClassLoaderFactory.java | 2 +-
.../org/apache/catalina/startup/ContextConfig.java | 44 ++++++++--------------
.../apache/catalina/startup/ContextRuleSet.java | 2 +
.../catalina/startup/CredentialHandlerRuleSet.java | 2 +-
.../org/apache/catalina/startup/FailedContext.java | 39 +++++++++----------
java/org/apache/catalina/startup/HostConfig.java | 3 +-
.../catalina/startup/ListenerCreateRule.java | 2 +-
.../catalina/startup/LocalStrings.properties | 2 +
.../apache/catalina/startup/SetNextNamingRule.java | 10 +++--
java/org/apache/catalina/startup/Tomcat.java | 2 +-
java/org/apache/catalina/startup/UserConfig.java | 12 +++---
13 files changed, 61 insertions(+), 64 deletions(-)
diff --git a/java/org/apache/catalina/startup/Catalina.java
b/java/org/apache/catalina/startup/Catalina.java
index 3ab65ae2d7..6cf5c80437 100644
--- a/java/org/apache/catalina/startup/Catalina.java
+++ b/java/org/apache/catalina/startup/Catalina.java
@@ -805,6 +805,7 @@ public class Catalina {
if (loaded) {
return;
}
+ // No load retry after a failure
loaded = true;
long t1 = System.nanoTime();
diff --git a/java/org/apache/catalina/startup/CatalinaProperties.java
b/java/org/apache/catalina/startup/CatalinaProperties.java
index f8d97839c4..ef96a4a85e 100644
--- a/java/org/apache/catalina/startup/CatalinaProperties.java
+++ b/java/org/apache/catalina/startup/CatalinaProperties.java
@@ -72,8 +72,8 @@ public class CatalinaProperties {
try {
String configUrl = System.getProperty("catalina.config");
if (configUrl != null) {
- if (configUrl.indexOf('/') == -1) {
- // No '/'. Must be a file name rather than a URL
+ if (configUrl.indexOf(':') == -1) {
+ // No ':'. Must be a file name rather than a URL
fileName = configUrl;
} else {
is = new URI(configUrl).toURL().openStream();
diff --git a/java/org/apache/catalina/startup/ClassLoaderFactory.java
b/java/org/apache/catalina/startup/ClassLoaderFactory.java
index 80c8897ee6..7f5f39879a 100644
--- a/java/org/apache/catalina/startup/ClassLoaderFactory.java
+++ b/java/org/apache/catalina/startup/ClassLoaderFactory.java
@@ -41,7 +41,7 @@ import org.apache.juli.logging.LogFactory;
* repositories.</li>
* <li>A set of directories containing classes and resources in JAR files.
Each readable JAR file discovered in these
* directories will be added to the class loader's repositories.</li>
- * <li><code>ClassLoader</code> instance that should become the parent of the
new class loader.</li>
+ * <li>{@link java.lang.ClassLoader} instance that should become the parent of
the new class loader.</li>
* </ul>
*/
public final class ClassLoaderFactory {
diff --git a/java/org/apache/catalina/startup/ContextConfig.java
b/java/org/apache/catalina/startup/ContextConfig.java
index 78734195af..582f1e05f0 100644
--- a/java/org/apache/catalina/startup/ContextConfig.java
+++ b/java/org/apache/catalina/startup/ContextConfig.java
@@ -696,8 +696,12 @@ public class ContextConfig implements LifecycleListener {
if (generateCode) {
contextXmlJavaSource =
getContextXmlJavaSource(contextXmlPackageName,
contextXmlSimpleClassName);
- digester.startGeneratingCode();
- generateClassHeader(digester, contextXmlPackageName,
contextXmlSimpleClassName);
+ if (contextXmlJavaSource != null) {
+ digester.startGeneratingCode();
+ generateClassHeader(digester,
contextXmlPackageName, contextXmlSimpleClassName);
+ } else {
+ generateCode = false;
+ }
}
URL defaultContextUrl =
contextXmlResource.getURI().toURL();
processContextConfig(digester, defaultContextUrl,
contextXmlResource.getInputStream());
@@ -735,8 +739,12 @@ public class ContextConfig implements LifecycleListener {
} else if (!useGeneratedCode) {
if (generateCode) {
contextXmlJavaSource =
getContextXmlJavaSource(contextXmlPackageName, contextXmlSimpleClassName);
- digester.startGeneratingCode();
- generateClassHeader(digester, contextXmlPackageName,
contextXmlSimpleClassName);
+ if (contextXmlJavaSource != null) {
+ digester.startGeneratingCode();
+ generateClassHeader(digester, contextXmlPackageName,
contextXmlSimpleClassName);
+ } else {
+ generateCode = false;
+ }
}
processContextConfig(digester, context.getConfigFile(), null);
if (generateCode) {
@@ -1021,7 +1029,7 @@ public class ContextConfig implements LifecycleListener {
/**
- * Process a "contextConfig" event for this Context.
+ * Process a "configure_start" event for this Context.
*/
protected synchronized void configureStart() {
// Called from StandardContext.start()
@@ -1077,7 +1085,7 @@ public class ContextConfig implements LifecycleListener {
/**
- * Process a "stop" event for this Context.
+ * Process a "configure_stop" event for this Context.
*/
protected synchronized void configureStop() {
@@ -1686,44 +1694,24 @@ public class ContextConfig implements LifecycleListener
{
long hostTimeStamp = 0;
if (globalWebXml != null) {
- URLConnection uc = null;
try {
URI uri = new URI(globalWebXml.getSystemId());
URL url = uri.toURL();
- uc = url.openConnection();
+ URLConnection uc = url.openConnection();
globalTimeStamp = uc.getLastModified();
} catch (IOException | URISyntaxException |
IllegalArgumentException e) {
globalTimeStamp = -1;
- } finally {
- if (uc != null) {
- try {
- uc.getInputStream().close();
- } catch (IOException ioe) {
- ExceptionUtils.handleThrowable(ioe);
- globalTimeStamp = -1;
- }
- }
}
}
if (hostWebXml != null) {
- URLConnection uc = null;
try {
URI uri = new URI(hostWebXml.getSystemId());
URL url = uri.toURL();
- uc = url.openConnection();
+ URLConnection uc = url.openConnection();
hostTimeStamp = uc.getLastModified();
} catch (IOException | URISyntaxException |
IllegalArgumentException e) {
hostTimeStamp = -1;
- } finally {
- if (uc != null) {
- try {
- uc.getInputStream().close();
- } catch (IOException ioe) {
- ExceptionUtils.handleThrowable(ioe);
- hostTimeStamp = -1;
- }
- }
}
}
diff --git a/java/org/apache/catalina/startup/ContextRuleSet.java
b/java/org/apache/catalina/startup/ContextRuleSet.java
index d65934cda5..dd0e8aa3c5 100644
--- a/java/org/apache/catalina/startup/ContextRuleSet.java
+++ b/java/org/apache/catalina/startup/ContextRuleSet.java
@@ -42,6 +42,7 @@ public class ContextRuleSet implements RuleSet {
/**
* Construct an instance of this <code>RuleSet</code> with the default
matching pattern prefix.
+ * The context instance will be created by the digester.
*/
public ContextRuleSet() {
this("");
@@ -50,6 +51,7 @@ public class ContextRuleSet implements RuleSet {
/**
* Construct an instance of this <code>RuleSet</code> with the specified
matching pattern prefix.
+ * The context instance will be created by the digester.
*
* @param prefix Prefix for matching pattern rules (including the trailing
slash character)
*/
diff --git a/java/org/apache/catalina/startup/CredentialHandlerRuleSet.java
b/java/org/apache/catalina/startup/CredentialHandlerRuleSet.java
index defd44d63c..f6acdddf31 100644
--- a/java/org/apache/catalina/startup/CredentialHandlerRuleSet.java
+++ b/java/org/apache/catalina/startup/CredentialHandlerRuleSet.java
@@ -21,7 +21,7 @@ import org.apache.tomcat.util.digester.RuleSet;
/**
* <strong>RuleSet</strong> for processing the contents of a CredentialHandler
definition element. This
- * <code>RuleSet</code> supports CredentialHandler such as the
<code>NestedCredentialHandler</code> that used nested
+ * <code>RuleSet</code> supports CredentialHandler such as the
<code>NestedCredentialHandler</code> that uses nested
* CredentialHandlers.
*/
public class CredentialHandlerRuleSet implements RuleSet {
diff --git a/java/org/apache/catalina/startup/FailedContext.java
b/java/org/apache/catalina/startup/FailedContext.java
index 819a1e7034..26dbcbd20a 100644
--- a/java/org/apache/catalina/startup/FailedContext.java
+++ b/java/org/apache/catalina/startup/FailedContext.java
@@ -19,6 +19,7 @@ package org.apache.catalina.startup;
import java.beans.PropertyChangeListener;
import java.io.File;
import java.net.URL;
+import java.util.Collections;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
@@ -383,7 +384,7 @@ public class FailedContext extends LifecycleMBeanBase
implements Context {
@Override
public ContainerListener[] findContainerListeners() {
- return null;
+ return new ContainerListener[0];
}
@Override
@@ -438,7 +439,7 @@ public class FailedContext extends LifecycleMBeanBase
implements Context {
@Override
public Object[] getApplicationEventListeners() {
- return null;
+ return new Object[0];
}
@Override
@@ -448,7 +449,7 @@ public class FailedContext extends LifecycleMBeanBase
implements Context {
@Override
public Object[] getApplicationLifecycleListeners() {
- return null;
+ return new Object[0];
}
@Override
@@ -793,7 +794,7 @@ public class FailedContext extends LifecycleMBeanBase
implements Context {
@Override
public String[] findApplicationListeners() {
- return null;
+ return new String[0];
}
@Override
@@ -808,7 +809,7 @@ public class FailedContext extends LifecycleMBeanBase
implements Context {
@Override
public ApplicationParameter[] findApplicationParameters() {
- return null;
+ return new ApplicationParameter[0];
}
@Override
@@ -823,7 +824,7 @@ public class FailedContext extends LifecycleMBeanBase
implements Context {
@Override
public SecurityConstraint[] findConstraints() {
- return null;
+ return new SecurityConstraint[0];
}
@Override
@@ -848,7 +849,7 @@ public class FailedContext extends LifecycleMBeanBase
implements Context {
@Override
public ErrorPage[] findErrorPages() {
- return null;
+ return new ErrorPage[0];
}
@Override
@@ -868,7 +869,7 @@ public class FailedContext extends LifecycleMBeanBase
implements Context {
@Override
public FilterDef[] findFilterDefs() {
- return null;
+ return new FilterDef[0];
}
@Override
@@ -888,7 +889,7 @@ public class FailedContext extends LifecycleMBeanBase
implements Context {
@Override
public FilterMap[] findFilterMaps() {
- return null;
+ return new FilterMap[0];
}
@Override
@@ -913,7 +914,7 @@ public class FailedContext extends LifecycleMBeanBase
implements Context {
@Override
public String[] findMimeMappings() {
- return null;
+ return new String[0];
}
@Override
@@ -933,7 +934,7 @@ public class FailedContext extends LifecycleMBeanBase
implements Context {
@Override
public String[] findParameters() {
- return null;
+ return new String[0];
}
@Override
@@ -968,7 +969,7 @@ public class FailedContext extends LifecycleMBeanBase
implements Context {
@Override
public String[] findSecurityRoles() {
- return null;
+ return new String[0];
}
@Override
@@ -988,7 +989,7 @@ public class FailedContext extends LifecycleMBeanBase
implements Context {
@Override
public String[] findServletMappings() {
- return null;
+ return new String[0];
}
@Override
@@ -1008,7 +1009,7 @@ public class FailedContext extends LifecycleMBeanBase
implements Context {
@Override
public String[] findWelcomeFiles() {
- return null;
+ return new String[0];
}
@Override
@@ -1023,7 +1024,7 @@ public class FailedContext extends LifecycleMBeanBase
implements Context {
@Override
public String[] findWrapperLifecycles() {
- return null;
+ return new String[0];
}
@Override
@@ -1038,7 +1039,7 @@ public class FailedContext extends LifecycleMBeanBase
implements Context {
@Override
public String[] findWrapperListeners() {
- return null;
+ return new String[0];
}
@Override
@@ -1124,7 +1125,7 @@ public class FailedContext extends LifecycleMBeanBase
implements Context {
@Override
public Set<String> addServletSecurity(ServletRegistration.Dynamic
registration,
ServletSecurityElement servletSecurityElement) {
- return null;
+ return Collections.emptySet();
}
@Override
@@ -1238,12 +1239,12 @@ public class FailedContext extends LifecycleMBeanBase
implements Context {
@Override
public Map<String,String> findPostConstructMethods() {
- return null;
+ return Collections.emptyMap();
}
@Override
public Map<String,String> findPreDestroyMethods() {
- return null;
+ return Collections.emptyMap();
}
@Override
diff --git a/java/org/apache/catalina/startup/HostConfig.java
b/java/org/apache/catalina/startup/HostConfig.java
index c1e62403f2..5b9cca9546 100644
--- a/java/org/apache/catalina/startup/HostConfig.java
+++ b/java/org/apache/catalina/startup/HostConfig.java
@@ -302,7 +302,7 @@ public class HostConfig implements LifecycleListener {
/**
- * Removed a serviced application from the list.
+ * Remove a serviced application from the list.
*
* @param name the context name
*/
@@ -1218,6 +1218,7 @@ public class HostConfig implements LifecycleListener {
Files.move(destination.toPath(), tempOld.toPath());
}
Files.move(tempNew.toPath(), destination.toPath());
+ // Only delete the previous webapp if everything went fine
ExpandWar.delete(tempOld);
} catch (Throwable t) {
diff --git a/java/org/apache/catalina/startup/ListenerCreateRule.java
b/java/org/apache/catalina/startup/ListenerCreateRule.java
index a8774f74db..81f863394e 100644
--- a/java/org/apache/catalina/startup/ListenerCreateRule.java
+++ b/java/org/apache/catalina/startup/ListenerCreateRule.java
@@ -127,7 +127,7 @@ public class ListenerCreateRule extends ObjectCreateRule {
}
/**
- * Return a property from the protocol handler.
+ * Return a property from the listener.
*
* @param name the property name
*
diff --git a/java/org/apache/catalina/startup/LocalStrings.properties
b/java/org/apache/catalina/startup/LocalStrings.properties
index a622174243..7f34d349c5 100644
--- a/java/org/apache/catalina/startup/LocalStrings.properties
+++ b/java/org/apache/catalina/startup/LocalStrings.properties
@@ -163,6 +163,8 @@ listener.notServer=This listener must only be nested within
Server elements, but
passwdUserDatabase.readFail=Failed to obtain a complete set of users from
/etc/passwd
+setNextNamingRule.invalidParent=Parent should be either a Context or a
NamingResources
+
tomcat.addWebapp.conflictChild=Unable to deploy WAR at [{0}] to context path
[{1}] because of existing context [{2}]
tomcat.addWebapp.conflictFile=Unable to deploy WAR at [{0}] to context path
[{1}] because of existing file [{2}]
tomcat.baseDirMakeFail=Unable to create the directory [{0}] to use as the base
directory
diff --git a/java/org/apache/catalina/startup/SetNextNamingRule.java
b/java/org/apache/catalina/startup/SetNextNamingRule.java
index 1d07c18ebe..c13571ed21 100644
--- a/java/org/apache/catalina/startup/SetNextNamingRule.java
+++ b/java/org/apache/catalina/startup/SetNextNamingRule.java
@@ -17,8 +17,8 @@
package org.apache.catalina.startup;
import org.apache.catalina.Context;
-import org.apache.catalina.deploy.NamingResourcesImpl;
import org.apache.tomcat.util.IntrospectionUtils;
+import org.apache.tomcat.util.descriptor.web.NamingResources;
import org.apache.tomcat.util.digester.Rule;
@@ -78,12 +78,14 @@ public class SetNextNamingRule extends Rule {
Object parent = digester.peek(1);
boolean context = false;
- NamingResourcesImpl namingResources;
+ NamingResources namingResources;
if (parent instanceof Context) {
namingResources = ((Context) parent).getNamingResources();
context = true;
+ } else if (parent instanceof NamingResources) {
+ namingResources = (NamingResources) parent;
} else {
- namingResources = (NamingResourcesImpl) parent;
+ throw new
IllegalArgumentException(sm.getString("setNextNamingRule.invalidParent"));
}
// Call the specified method
@@ -105,7 +107,7 @@ public class SetNextNamingRule extends Rule {
@Override
public String toString() {
- return "SetNextRule[" + "methodName=" + methodName + ", paramType=" +
paramType + ']';
+ return "SetNextNamingRule[" + "methodName=" + methodName + ",
paramType=" + paramType + ']';
}
diff --git a/java/org/apache/catalina/startup/Tomcat.java
b/java/org/apache/catalina/startup/Tomcat.java
index 2febc5892e..294478e31d 100644
--- a/java/org/apache/catalina/startup/Tomcat.java
+++ b/java/org/apache/catalina/startup/Tomcat.java
@@ -863,7 +863,7 @@ public class Tomcat {
System.setProperty(Globals.CATALINA_HOME_PROP,
server.getCatalinaHome().getPath());
}
- static final String[] silences = new String[] {
"org.apache.coyote.http11.Http11NioProtocol",
+ private static final String[] silences = new String[] {
"org.apache.coyote.http11.Http11NioProtocol",
"org.apache.catalina.core.StandardService",
"org.apache.catalina.core.StandardEngine",
"org.apache.catalina.startup.ContextConfig",
"org.apache.catalina.core.ApplicationContext",
"org.apache.catalina.core.AprLifecycleListener",
"org.apache.catalina.core.OpenSSLLifecycleListener" };
diff --git a/java/org/apache/catalina/startup/UserConfig.java
b/java/org/apache/catalina/startup/UserConfig.java
index d7aaacee18..b9f86b2d43 100644
--- a/java/org/apache/catalina/startup/UserConfig.java
+++ b/java/org/apache/catalina/startup/UserConfig.java
@@ -97,14 +97,14 @@ public final class UserConfig implements LifecycleListener {
private String userClass =
"org.apache.catalina.startup.PasswdUserDatabase";
/**
- * A regular expression defining user who deployment is allowed.
+ * A regular expression defining users for whom deployment is allowed.
*/
- Pattern allow = null;
+ private Pattern allow = null;
/**
- * A regular expression defining user who deployment is denied.
+ * A regular expression defining users for whom deployment is denied.
*/
- Pattern deny = null;
+ private Pattern deny = null;
// ------------------------------------------------------------- Properties
@@ -222,7 +222,7 @@ public final class UserConfig implements LifecycleListener {
/**
- * Set the regular expression used to test for user who deployment is
allowed.
+ * Set the regular expression used to test for users for whom deployment
is allowed.
*
* @param allow The new allow expression
*/
@@ -249,7 +249,7 @@ public final class UserConfig implements LifecycleListener {
/**
- * Set the regular expression used to test for user who deployment is
denied.
+ * Set the regular expression used to test for users for whom deployment
is denied.
*
* @param deny The new deny expression
*/
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
