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 40442d72ae Code review fixes
40442d72ae is described below
commit 40442d72ae5dab38d414a5705b67563cf3c17685
Author: remm <[email protected]>
AuthorDate: Wed Jun 24 20:27:45 2026 +0200
Code review fixes
---
.../catalina/loader/WebappClassLoaderBase.java | 4 ++++
java/org/apache/catalina/loader/WebappLoader.java | 7 ++++--
.../catalina/manager/HTMLManagerServlet.java | 7 ++++--
.../apache/catalina/manager/JMXProxyServlet.java | 3 ++-
.../catalina/manager/LocalStrings.properties | 2 +-
.../apache/catalina/manager/ManagerServlet.java | 10 +++++++--
.../manager/host/HTMLHostManagerServlet.java | 2 +-
.../catalina/manager/host/HostManagerServlet.java | 25 ++++++++++++++++++++++
.../catalina/manager/host/LocalStrings.properties | 2 ++
9 files changed, 53 insertions(+), 9 deletions(-)
diff --git a/java/org/apache/catalina/loader/WebappClassLoaderBase.java
b/java/org/apache/catalina/loader/WebappClassLoaderBase.java
index af9971a438..4f7ebb62b6 100644
--- a/java/org/apache/catalina/loader/WebappClassLoaderBase.java
+++ b/java/org/apache/catalina/loader/WebappClassLoaderBase.java
@@ -575,6 +575,10 @@ public abstract class WebappClassLoaderBase extends
URLClassLoader
base.clearReferencesStopTimerThreads =
this.clearReferencesStopTimerThreads;
base.clearReferencesLogFactoryRelease =
this.clearReferencesLogFactoryRelease;
base.clearReferencesHttpClientKeepAliveThread =
this.clearReferencesHttpClientKeepAliveThread;
+ base.clearReferencesRmiTargets = this.clearReferencesRmiTargets;
+ base.clearReferencesThreadLocals = this.clearReferencesThreadLocals;
+ base.skipMemoryLeakChecksOnJvmShutdown =
this.skipMemoryLeakChecksOnJvmShutdown;
+
base.notFoundClassResources.setLimit(this.notFoundClassResources.getLimit());
base.jarModificationTimes.putAll(this.jarModificationTimes);
base.permissionList.addAll(this.permissionList);
base.loaderPC.putAll(this.loaderPC);
diff --git a/java/org/apache/catalina/loader/WebappLoader.java
b/java/org/apache/catalina/loader/WebappLoader.java
index a973425676..7427aa60b6 100644
--- a/java/org/apache/catalina/loader/WebappLoader.java
+++ b/java/org/apache/catalina/loader/WebappLoader.java
@@ -269,8 +269,11 @@ public class WebappLoader extends LifecycleMBeanBase
implements Loader {
public String getLoaderRepositoriesString() {
String[] repositories = getLoaderRepositories();
StringBuilder sb = new StringBuilder();
- for (String repository : repositories) {
- sb.append(repository).append(':');
+ for (int i = 0; i < repositories.length; i++) {
+ if (i > 0) {
+ sb.append(':');
+ }
+ sb.append(repositories[i]);
}
return sb.toString();
}
diff --git a/java/org/apache/catalina/manager/HTMLManagerServlet.java
b/java/org/apache/catalina/manager/HTMLManagerServlet.java
index f624475280..0a42f943f1 100644
--- a/java/org/apache/catalina/manager/HTMLManagerServlet.java
+++ b/java/org/apache/catalina/manager/HTMLManagerServlet.java
@@ -976,7 +976,7 @@ public class HTMLManagerServlet extends ManagerServlet {
req.setAttribute("sort", sortBy);
req.setAttribute("order", orderBy);
req.setAttribute("activeSessions", sessions);
- // strong>NOTE</strong> - This header will be overridden
+ // <strong>NOTE</strong> - This header will be overridden
// automatically if a <code>RequestDispatcher.forward()</code> call is
// ultimately invoked.
resp.setHeader("Pragma", "No-cache"); // HTTP 1.0
@@ -1000,7 +1000,10 @@ public class HTMLManagerServlet extends ManagerServlet {
protected void displaySessionDetailPage(HttpServletRequest req,
HttpServletResponse resp, ContextName cn,
String sessionId, StringManager smClient) throws ServletException,
IOException {
Session session = getSessionForNameAndId(cn, sessionId, smClient);
- // strong>NOTE</strong> - This header will be overridden
+ if (session == null) {
+ req.setAttribute(APPLICATION_ERROR, "Session not found: " +
sessionId);
+ }
+ // <strong>NOTE</strong> - This header will be overridden
// automatically if a <code>RequestDispatcher.forward()</code> call is
// ultimately invoked.
resp.setHeader("Pragma", "No-cache"); // HTTP 1.0
diff --git a/java/org/apache/catalina/manager/JMXProxyServlet.java
b/java/org/apache/catalina/manager/JMXProxyServlet.java
index 0a6b48c61c..af12533355 100644
--- a/java/org/apache/catalina/manager/JMXProxyServlet.java
+++ b/java/org/apache/catalina/manager/JMXProxyServlet.java
@@ -43,7 +43,8 @@ import org.apache.tomcat.util.modeler.Registry;
import org.apache.tomcat.util.res.StringManager;
/**
- * This servlet will dump JMX attributes in a simple format and implement
proxy services for modeler.
+ * This servlet is an administrative tool that will dump JMX attributes
+ * in a simple format and implement proxy services for modeler.
*/
public class JMXProxyServlet extends HttpServlet {
diff --git a/java/org/apache/catalina/manager/LocalStrings.properties
b/java/org/apache/catalina/manager/LocalStrings.properties
index 8965fbae74..f360ad8920 100644
--- a/java/org/apache/catalina/manager/LocalStrings.properties
+++ b/java/org/apache/catalina/manager/LocalStrings.properties
@@ -153,7 +153,7 @@ managerServlet.noCommand=FAIL - No command was specified
managerServlet.noContext=FAIL - No context exists named [{0}]
managerServlet.noGlobal=FAIL - No global JNDI resources are available
managerServlet.noManager=FAIL - No manager exists for path [{0}]
-managerServlet.noSelf=FAIL - The manager cannot reload, undeploy, stop, or
undeploy itself
+managerServlet.noSelf=FAIL - The manager cannot reload, stop, or undeploy
itself
managerServlet.noWrapper=Container has not called setWrapper() for this servlet
managerServlet.notDeployed=FAIL - Context [{0}] is defined in server.xml and
may not be undeployed
managerServlet.notSslConnector=SSL is not enabled for this connector
diff --git a/java/org/apache/catalina/manager/ManagerServlet.java
b/java/org/apache/catalina/manager/ManagerServlet.java
index cc060f74b3..5533b43eed 100644
--- a/java/org/apache/catalina/manager/ManagerServlet.java
+++ b/java/org/apache/catalina/manager/ManagerServlet.java
@@ -677,7 +677,7 @@ public class ManagerServlet extends HttpServlet implements
ContainerServlet {
* Deploy a web application archive (included in the current request) at
the specified context path.
*
* @param writer Writer to render results to
- * @param config URL of the context configuration file to be installed
+ * @param config URL of the context configuration file to be copied into
the local config folder
* @param cn Name of the application to be installed
* @param tag Tag to be associated with the webapp
* @param update Flag that indicates that any existing app should be
replaced
@@ -889,7 +889,7 @@ public class ManagerServlet extends HttpServlet implements
ContainerServlet {
* Install an application for the specified path from the specified web
application archive.
*
* @param writer Writer to render results to
- * @param config URL of the context configuration file to be installed
+ * @param config URL of the context configuration file to be copied into
the local config folder
* @param cn Name of the application to be installed
* @param war URL of the web application archive to be installed
* @param update true to override any existing webapp on the path
@@ -1451,6 +1451,12 @@ public class ManagerServlet extends HttpServlet
implements ContainerServlet {
return;
}
+ // It isn't possible for the manager to undeploy itself
+ if (context.getName().equals(this.context.getName())) {
+ writer.println(smClient.getString("managerServlet.noSelf"));
+ return;
+ }
+
if (!isDeployed(name)) {
writer.println(
smClient.getString("managerServlet.notDeployed",
Escape.htmlElementContent(displayPath)));
diff --git a/java/org/apache/catalina/manager/host/HTMLHostManagerServlet.java
b/java/org/apache/catalina/manager/host/HTMLHostManagerServlet.java
index d90708bbb8..23e4594bc8 100644
--- a/java/org/apache/catalina/manager/host/HTMLHostManagerServlet.java
+++ b/java/org/apache/catalina/manager/host/HTMLHostManagerServlet.java
@@ -48,7 +48,7 @@ import org.apache.tomcat.util.security.Escape;
* However if you use a software that parses the output of
<code>HostManagerServlet</code> you won't be able to upgrade
* to this Servlet since the output are not in the same format as from
<code>HostManagerServlet</code>
*
- * @see org.apache.catalina.manager.ManagerServlet
+ * @see org.apache.catalina.manager.host.HostManagerServlet
*/
public class HTMLHostManagerServlet extends HostManagerServlet {
diff --git a/java/org/apache/catalina/manager/host/HostManagerServlet.java
b/java/org/apache/catalina/manager/host/HostManagerServlet.java
index 8a528cf0ac..a3963efd55 100644
--- a/java/org/apache/catalina/manager/host/HostManagerServlet.java
+++ b/java/org/apache/catalina/manager/host/HostManagerServlet.java
@@ -290,6 +290,24 @@ public class HostManagerServlet extends HttpServlet
implements ContainerServlet
}
+ private static boolean pathCheck(File input, File expected, PrintWriter
writer, StringManager smClient) {
+ try {
+ if
(!input.getCanonicalFile().toPath().startsWith(expected.getCanonicalFile().toPath()))
{
+ if (writer != null) {
+
writer.println(smClient.getString("hostManagerServlet.pathCheckFail", input,
expected));
+ }
+ return false;
+ }
+ } catch (IOException ioe) {
+ if (writer != null) {
+
writer.println(smClient.getString("hostManagerServlet.pathCheckError", input,
expected, ioe.getMessage()));
+ }
+ return false;
+ }
+ return true;
+ }
+
+
// -------------------------------------------------------- Private Methods
@@ -343,6 +361,10 @@ public class HostManagerServlet extends HttpServlet
implements ContainerServlet
} catch (IOException ioe) {
appBaseFile = file;
}
+ if (!pathCheck(appBaseFile, engine.getCatalinaBase(), writer,
smClient)) {
+ // Any error reported in pathCheck()
+ return;
+ }
if (!appBaseFile.mkdirs() && !appBaseFile.isDirectory()) {
writer.println(smClient.getString("hostManagerServlet.appBaseCreateFail",
appBaseFile.toString(), name));
return;
@@ -638,6 +660,9 @@ public class HostManagerServlet extends HttpServlet
implements ContainerServlet
if (installedHost != null) {
configBase = new File(configBase, hostName);
}
+ if (!pathCheck(configBase, new File(context.getCatalinaBase(),
"conf"), null, null)) {
+ return null;
+ }
if (!configBase.mkdirs() && !configBase.isDirectory()) {
return null;
}
diff --git a/java/org/apache/catalina/manager/host/LocalStrings.properties
b/java/org/apache/catalina/manager/host/LocalStrings.properties
index 8be8a004dc..91dce2478d 100644
--- a/java/org/apache/catalina/manager/host/LocalStrings.properties
+++ b/java/org/apache/catalina/manager/host/LocalStrings.properties
@@ -33,6 +33,8 @@ hostManagerServlet.noCommand=FAIL - No command was specified
hostManagerServlet.noHost=FAIL - Host name [{0}] does not exist
hostManagerServlet.noStoreConfig=FAIL - Please enable StoreConfig to use this
feature
hostManagerServlet.noWrapper=Container has not called setWrapper() for this
servlet
+hostManagerServlet.pathCheckFail=FAIL - Unable to use [{0}] as that is outside
the expected directory [{1}]
+hostManagerServlet.pathCheckError=FAIL - Unable to use [{0}] due to [{2}]
while checking if it was outside the expected directory [{1}]
hostManagerServlet.persist=persist: Persisting current configuration
hostManagerServlet.persistFailed=FAIL - Failed to persist configuration
hostManagerServlet.persisted=OK - Configuration persisted
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
