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 5de34ca7e7 Minor fixes from code review
5de34ca7e7 is described below
commit 5de34ca7e743f3e1caccc6a635f9ce5bdc4bf318
Author: remm <[email protected]>
AuthorDate: Thu Jul 16 15:28:57 2026 +0200
Minor fixes from code review
---
java/org/apache/catalina/util/XMLWriter.java | 3 +--
.../catalina/valves/AbstractAccessLogValve.java | 8 +++----
.../valves/CrawlerSessionManagerValve.java | 2 +-
.../apache/catalina/valves/ErrorReportValve.java | 14 ++++++++++--
.../catalina/valves/ExtendedAccessLogValve.java | 7 +++++-
.../apache/catalina/valves/JsonAccessLogValve.java | 1 +
.../apache/catalina/valves/LocalStrings.properties | 3 +++
.../catalina/valves/ParameterLimitValve.java | 2 +-
.../apache/catalina/valves/PersistentValve.java | 6 +++---
java/org/apache/catalina/valves/RemoteIpValve.java | 25 ++++++++++++++++------
.../apache/catalina/valves/RequestFilterValve.java | 10 ++++-----
java/org/apache/catalina/valves/SSLValve.java | 1 +
.../org/apache/catalina/valves/SemaphoreValve.java | 8 +++++++
.../catalina/valves/rewrite/RewriteRule.java | 1 +
14 files changed, 66 insertions(+), 25 deletions(-)
diff --git a/java/org/apache/catalina/util/XMLWriter.java
b/java/org/apache/catalina/util/XMLWriter.java
index a2eb721e8f..c14c9c2ad4 100644
--- a/java/org/apache/catalina/util/XMLWriter.java
+++ b/java/org/apache/catalina/util/XMLWriter.java
@@ -22,8 +22,7 @@ import java.io.Writer;
import org.apache.tomcat.util.security.Escape;
/**
- * XMLWriter helper class. writeText is the only method in the class doing
- * XML escaping.
+ * XMLWriter helper class.
*/
public class XMLWriter {
diff --git a/java/org/apache/catalina/valves/AbstractAccessLogValve.java
b/java/org/apache/catalina/valves/AbstractAccessLogValve.java
index 716ba4dea5..bc82d1233e 100644
--- a/java/org/apache/catalina/valves/AbstractAccessLogValve.java
+++ b/java/org/apache/catalina/valves/AbstractAccessLogValve.java
@@ -2209,16 +2209,16 @@ public abstract class AbstractAccessLogValve extends
ValveBase implements Access
* - %to trailer response header - %V server name per UseCanonicalName
setting
*
* The following escaped elements are not escaped in Tomcat because values
that would require escaping are rejected
- * before they reach the AccessLogValve: - %h remote host - %H request
protocol - %m request method - %q query
- * string - %r request line - %U request URI - %v canonical server name
+ * before they reach the AccessLogValve: - %h remote host - %H request
protocol - %m request method
+ * - %v canonical server name
*
* The following escaped elements are supported by Tomcat: - %{}i request
header - %{}o response header - %u remote
* user
*
* The following additional Tomcat elements are escaped for consistency: -
%{}c cookie value - %{}r request
- * attribute - %{}s session attribute
+ * attribute - %{}s session attribute - %q query string - %r request line
- %U request URI
*
- * giving a total of 6 elements that are escaped in Tomcat.
+ * giving a total of 9 elements that are escaped in Tomcat.
*
* Quoting from the httpd docs: "...non-printable and other special
characters in %r, %i and %o are escaped using
* \xhh sequences, where hh stands for the hexadecimal representation of
the raw byte. Exceptions from this rule are
diff --git a/java/org/apache/catalina/valves/CrawlerSessionManagerValve.java
b/java/org/apache/catalina/valves/CrawlerSessionManagerValve.java
index 58ff046a1b..856a6e9131 100644
--- a/java/org/apache/catalina/valves/CrawlerSessionManagerValve.java
+++ b/java/org/apache/catalina/valves/CrawlerSessionManagerValve.java
@@ -73,7 +73,7 @@ public class CrawlerSessionManagerValve extends ValveBase {
/**
* Specify the regular expression (using {@link Pattern}) that will be
used to identify crawlers based in the
- * User-Agent header provided. The default is
".*GoogleBot.*|.*bingbot.*|.*Yahoo! Slurp.*"
+ * User-Agent header provided. The default is ".*[bB]ot.*|.*Yahoo!
Slurp.*|.*Feedfetcher-Google.*"
*
* @param crawlerUserAgents The regular expression using {@link Pattern}
*/
diff --git a/java/org/apache/catalina/valves/ErrorReportValve.java
b/java/org/apache/catalina/valves/ErrorReportValve.java
index a3369b6ad0..3a33b39647 100644
--- a/java/org/apache/catalina/valves/ErrorReportValve.java
+++ b/java/org/apache/catalina/valves/ErrorReportValve.java
@@ -441,7 +441,12 @@ public class ErrorReportValve extends ValveBase {
*/
public boolean setProperty(String name, String value) {
if (name.startsWith("errorCode.")) {
- int code = Integer.parseInt(name.substring(10));
+ int code;
+ try {
+ code = Integer.parseInt(name.substring(10));
+ } catch (NumberFormatException e) {
+ return false;
+ }
ErrorPage ep = new ErrorPage();
ep.setErrorCode(code);
ep.setLocation(value);
@@ -468,7 +473,12 @@ public class ErrorReportValve extends ValveBase {
public String getProperty(String name) {
String result;
if (name.startsWith("errorCode.")) {
- int code = Integer.parseInt(name.substring(10));
+ int code;
+ try {
+ code = Integer.parseInt(name.substring(10));
+ } catch (NumberFormatException e) {
+ return null;
+ }
ErrorPage ep = errorPageSupport.find(code);
if (ep == null) {
result = null;
diff --git a/java/org/apache/catalina/valves/ExtendedAccessLogValve.java
b/java/org/apache/catalina/valves/ExtendedAccessLogValve.java
index 16d8f32fc2..aa8e4c665b 100644
--- a/java/org/apache/catalina/valves/ExtendedAccessLogValve.java
+++ b/java/org/apache/catalina/valves/ExtendedAccessLogValve.java
@@ -342,7 +342,7 @@ public class ExtendedAccessLogValve extends AccessLogValve {
@Override
public void addElement(CharArrayWriter buf, Date date, Request
request, Response response, long time) {
- wrap(request.getAttribute(attribute), buf);
+ wrap(request == null ? "??" : request.getAttribute(attribute),
buf);
}
}
@@ -368,7 +368,12 @@ public class ExtendedAccessLogValve extends AccessLogValve
{
session = request.getSession(false);
if (session != null) {
wrap(session.getAttribute(attribute), buf);
+ return;
+ } else {
+ buf.append('-');
}
+ } else {
+ buf.append("??");
}
}
}
diff --git a/java/org/apache/catalina/valves/JsonAccessLogValve.java
b/java/org/apache/catalina/valves/JsonAccessLogValve.java
index 278ccfbbe1..4a910aa0d7 100644
--- a/java/org/apache/catalina/valves/JsonAccessLogValve.java
+++ b/java/org/apache/catalina/valves/JsonAccessLogValve.java
@@ -283,6 +283,7 @@ public class JsonAccessLogValve extends AccessLogValve {
if (quoteValue) {
buf.append('"');
}
+ // The value will already be log escaped
delegate.addElement(buf, date, request, response, time);
if (quoteValue) {
buf.append('"');
diff --git a/java/org/apache/catalina/valves/LocalStrings.properties
b/java/org/apache/catalina/valves/LocalStrings.properties
index 771bcc6f63..5b8053a6a4 100644
--- a/java/org/apache/catalina/valves/LocalStrings.properties
+++ b/java/org/apache/catalina/valves/LocalStrings.properties
@@ -167,6 +167,9 @@ remoteIpValve.invalidRemoteAddress=Unable to determine the
remote host because t
requestFilterValve.configInvalid=One or more invalid configuration settings
were provided for the Remote[Addr|Host]Valve which prevented the Valve and its
parent containers from starting
requestFilterValve.deny=Denied request for [{0}] based on property [{1}]
+semaphoreValve.invalidConcurrency=Invalid concurrency value [{0}] specified
+semaphoreValve.invalidStatus=Invalid high concurrency status [{0}] specified
+
sslValve.certError=Failed to process certificate string [{0}] to create a
java.security.cert.X509Certificate object
sslValve.invalidHeader=Invalid value [{0}] for header [{1}]
sslValve.invalidProvider=The SSL provider specified on the connector
associated with this request of [{0}] is invalid. The certificate data could
not be processed.
diff --git a/java/org/apache/catalina/valves/ParameterLimitValve.java
b/java/org/apache/catalina/valves/ParameterLimitValve.java
index 796715c86b..c63a2484ee 100644
--- a/java/org/apache/catalina/valves/ParameterLimitValve.java
+++ b/java/org/apache/catalina/valves/ParameterLimitValve.java
@@ -61,7 +61,7 @@ import org.apache.tomcat.util.file.ConfigurationSource;
* <pre>
* {@code
* <Context>
- * <Valve className="org.apache.catalina.valves.ParameterLimitValve"
+ * <Valve className="org.apache.catalina.valves.ParameterLimitValve"/>
* </Context>
* }
* and in <code>parameter_limit.config</code>:
diff --git a/java/org/apache/catalina/valves/PersistentValve.java
b/java/org/apache/catalina/valves/PersistentValve.java
index 8539130919..cd699778bb 100644
--- a/java/org/apache/catalina/valves/PersistentValve.java
+++ b/java/org/apache/catalina/valves/PersistentValve.java
@@ -51,8 +51,8 @@ import org.apache.juli.logging.LogFactory;
* <p>
* To avoid conflicts and/or errors when updating the session store, each
session must only be accessed by no more than
* one concurrent request. The {@code filter} field can be used to define
requests (e.g. those for static resources)
- * that do not need access to the session and can Requests for resources that
do not need to access the session and can
- * bypass the session load/save functionality provided by this Valve.
+ * that do not need access to the session and can bypass the session load/save
functionality
+ * provided by this Valve.
* <p>
* The Valve uses a per session {@code Semaphore} to ensure that each session
is accessed by no more than one request at
* a time within a single Tomcat instance. The behaviour if multiple requests
try to access the session concurrently can
@@ -333,7 +333,7 @@ public class PersistentValve extends ValveBase {
if (session != null) {
int maxInactiveInterval = session.getMaxInactiveInterval();
if (maxInactiveInterval > 0) {
- int timeIdle = (int) (session.getIdleTimeInternal() / 1000L);
+ long timeIdle = session.getIdleTimeInternal() / 1000L;
return timeIdle >= maxInactiveInterval;
}
}
diff --git a/java/org/apache/catalina/valves/RemoteIpValve.java
b/java/org/apache/catalina/valves/RemoteIpValve.java
index d06e2057ee..4f336d3a89 100644
--- a/java/org/apache/catalina/valves/RemoteIpValve.java
+++ b/java/org/apache/catalina/valves/RemoteIpValve.java
@@ -20,6 +20,7 @@ import java.io.IOException;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.util.ArrayDeque;
+import java.util.ArrayList;
import java.util.Deque;
import java.util.Enumeration;
import java.util.Iterator;
@@ -657,9 +658,15 @@ public class RemoteIpValve extends ValveBase {
final String originalLocalName = isChangeLocalName() ?
request.getLocalName() : null;
final int originalServerPort = request.getServerPort();
final int originalLocalPort = request.getLocalPort();
- final String originalProxiesHeader = request.getHeader(proxiesHeader);
- final String originalRemoteIpHeader =
request.getHeader(remoteIpHeader);
boolean isInternal = internalProxies != null &&
internalProxies.matcher(originalRemoteAddr).matches();
+ final List<String> originalProxiesHeaderValues = new ArrayList<>();
+ for (Enumeration<String> e = request.getHeaders(proxiesHeader);
e.hasMoreElements();) {
+ originalProxiesHeaderValues.add(e.nextElement());
+ }
+ final List<String> originalRemoteIpHeaderValues = new ArrayList<>();
+ for (Enumeration<String> e = request.getHeaders(remoteIpHeader);
e.hasMoreElements();) {
+ originalRemoteIpHeaderValues.add(e.nextElement());
+ }
if (isInternal || (trustedProxies != null &&
trustedProxies.matcher(originalRemoteAddr).matches())) {
String remoteIp = null;
@@ -815,16 +822,22 @@ public class RemoteIpValve extends ValveBase {
request.setLocalPort(originalLocalPort);
MimeHeaders headers =
request.getCoyoteRequest().getMimeHeaders();
- if (originalProxiesHeader == null ||
originalProxiesHeader.isEmpty()) {
+ if (originalProxiesHeaderValues.isEmpty()) {
headers.removeHeader(proxiesHeader);
} else {
-
headers.setValue(proxiesHeader).setString(originalProxiesHeader);
+ headers.removeHeader(proxiesHeader);
+ for (String v : originalProxiesHeaderValues) {
+ headers.addValue(proxiesHeader).setString(v);
+ }
}
- if (originalRemoteIpHeader == null ||
originalRemoteIpHeader.isEmpty()) {
+ if (originalRemoteIpHeaderValues.isEmpty()) {
headers.removeHeader(remoteIpHeader);
} else {
-
headers.setValue(remoteIpHeader).setString(originalRemoteIpHeader);
+ headers.removeHeader(remoteIpHeader);
+ for (String v : originalRemoteIpHeaderValues) {
+ headers.addValue(remoteIpHeader).setString(v);
+ }
}
}
}
diff --git a/java/org/apache/catalina/valves/RequestFilterValve.java
b/java/org/apache/catalina/valves/RequestFilterValve.java
index f1ef980e73..545731eb74 100644
--- a/java/org/apache/catalina/valves/RequestFilterValve.java
+++ b/java/org/apache/catalina/valves/RequestFilterValve.java
@@ -43,8 +43,8 @@ import org.apache.juli.logging.Log;
* this request will be rejected with a "Forbidden" HTTP response.</li>
* <li>If there is an allow expression configured, the property will be
compared to each such expression. If a match is
* found, this request will be allowed to pass through to the next Valve in
the current pipeline.</li>
- * <li>If a deny expression was specified but no allow expression, allow this
request to pass through (because none of
- * the deny expressions matched it).
+ * <li>If a deny expression was specified but no allow expression was
specified, allow this request to pass through
+ * (because none of the deny expressions matched it).</li>
* <li>The request will be rejected with a "Forbidden" HTTP response.</li>
* </ul>
* <p>
@@ -123,7 +123,7 @@ public abstract class RequestFilterValve extends ValveBase {
private volatile boolean addConnectorPort = false;
/**
- * Flag deciding whether we use the connection peer address or the remote
address. This makes a dfifference when
+ * Flag deciding whether we use the connection peer address or the remote
address. This makes a difference when
* using AJP or the RemoteIpValve.
*/
private volatile boolean usePeerAddress = false;
@@ -284,7 +284,7 @@ public abstract class RequestFilterValve extends ValveBase {
/**
- * Get the flag deciding whether we use the connection peer address or the
remote address. This makes a dfifference
+ * Get the flag deciding whether we use the connection peer address or the
remote address. This makes a difference
* when using AJP or the RemoteIpValve.
*
* @return <code>true</code> if we use the connection peer address
@@ -295,7 +295,7 @@ public abstract class RequestFilterValve extends ValveBase {
/**
- * Set the flag deciding whether we use the connection peer address or the
remote address. This makes a dfifference
+ * Set the flag deciding whether we use the connection peer address or the
remote address. This makes a difference
* when using AJP or the RemoteIpValve.
*
* @param usePeerAddress The new flag
diff --git a/java/org/apache/catalina/valves/SSLValve.java
b/java/org/apache/catalina/valves/SSLValve.java
index ca3b14d546..3470642b40 100644
--- a/java/org/apache/catalina/valves/SSLValve.java
+++ b/java/org/apache/catalina/valves/SSLValve.java
@@ -235,6 +235,7 @@ public class SSLValve extends ValveBase {
} catch (NoSuchProviderException e) {
log.error(sm.getString("sslValve.invalidProvider",
providerName), e);
}
+ // If parsing fails, remove certificates
request.setAttribute(Globals.CERTIFICATES_ATTR, jsseCerts);
}
}
diff --git a/java/org/apache/catalina/valves/SemaphoreValve.java
b/java/org/apache/catalina/valves/SemaphoreValve.java
index 12ab42a32d..096a473069 100644
--- a/java/org/apache/catalina/valves/SemaphoreValve.java
+++ b/java/org/apache/catalina/valves/SemaphoreValve.java
@@ -79,6 +79,10 @@ public class SemaphoreValve extends ValveBase {
* @param concurrency the concurrency level
*/
public void setConcurrency(int concurrency) {
+ if (concurrency < 0) {
+ throw new IllegalArgumentException(
+ sm.getString("semaphoreValve.invalidConcurrency",
Integer.valueOf(concurrency)));
+ }
this.concurrency = concurrency;
}
@@ -175,6 +179,10 @@ public class SemaphoreValve extends ValveBase {
* @param highConcurrencyStatus the status code to return
*/
public void setHighConcurrencyStatus(int highConcurrencyStatus) {
+ if (highConcurrencyStatus != -1 && (highConcurrencyStatus < 100 ||
highConcurrencyStatus >= 600)) {
+ throw new IllegalArgumentException(
+ sm.getString("semaphoreValve.invalidStatus",
Integer.valueOf(highConcurrencyStatus)));
+ }
this.highConcurrencyStatus = highConcurrencyStatus;
}
diff --git a/java/org/apache/catalina/valves/rewrite/RewriteRule.java
b/java/org/apache/catalina/valves/rewrite/RewriteRule.java
index 49c0353636..91777daa59 100644
--- a/java/org/apache/catalina/valves/rewrite/RewriteRule.java
+++ b/java/org/apache/catalina/valves/rewrite/RewriteRule.java
@@ -89,6 +89,7 @@ public class RewriteRule {
if (isNocase()) {
flags |= Pattern.CASE_INSENSITIVE;
}
+ // Check that the pattern compiles
Pattern.compile(patternString, flags);
// Parse conditions
for (RewriteCond condition : conditions) {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
