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 235848c219 Code review fixes
235848c219 is described below
commit 235848c2193b06654752096aef21e5ca833310cf
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 257177f872..f49ae15b6f 100644
--- a/java/org/apache/catalina/loader/WebappClassLoaderBase.java
+++ b/java/org/apache/catalina/loader/WebappClassLoaderBase.java
@@ -620,6 +620,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 7d72c98c64..5b080c1ab7 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 bfcfb8c7be..54c46da4a6 100644
--- a/java/org/apache/catalina/manager/LocalStrings.properties
+++ b/java/org/apache/catalina/manager/LocalStrings.properties
@@ -156,7 +156,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 2a1fc81857..4cd020936c 100644
--- a/java/org/apache/catalina/manager/host/LocalStrings.properties
+++ b/java/org/apache/catalina/manager/host/LocalStrings.properties
@@ -36,6 +36,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]
