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 dae3dee8e1 Minor fixes from code review
dae3dee8e1 is described below
commit dae3dee8e1881f2469663802c8d460c2e3c52b57
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 | 26 +++++++++++++++++-----
.../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, 67 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 353d0f8510..13aaee912e 100644
--- a/java/org/apache/catalina/valves/AbstractAccessLogValve.java
+++ b/java/org/apache/catalina/valves/AbstractAccessLogValve.java
@@ -2290,16 +2290,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 bfa8859a6b..c67229d9be 100644
--- a/java/org/apache/catalina/valves/CrawlerSessionManagerValve.java
+++ b/java/org/apache/catalina/valves/CrawlerSessionManagerValve.java
@@ -74,7 +74,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 acadedc2ce..9f4211e396 100644
--- a/java/org/apache/catalina/valves/ErrorReportValve.java
+++ b/java/org/apache/catalina/valves/ErrorReportValve.java
@@ -437,7 +437,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);
@@ -464,7 +469,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 c005fb9e29..8700a4126f 100644
--- a/java/org/apache/catalina/valves/ExtendedAccessLogValve.java
+++ b/java/org/apache/catalina/valves/ExtendedAccessLogValve.java
@@ -357,7 +357,7 @@ public class ExtendedAccessLogValve extends AccessLogValve {
@Override
public void addElement(CharArrayWriter buf, Request request, Response
response, long time) {
- wrap(request.getAttribute(attribute), buf);
+ wrap(request == null ? "??" : request.getAttribute(attribute),
buf);
}
}
@@ -383,7 +383,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 f1842a6ad1..0b489b9532 100644
--- a/java/org/apache/catalina/valves/JsonAccessLogValve.java
+++ b/java/org/apache/catalina/valves/JsonAccessLogValve.java
@@ -286,6 +286,7 @@ public class JsonAccessLogValve extends AccessLogValve {
if (quoteValue) {
buf.append('"');
}
+ // The value will already be log escaped
delegate.addElement(buf, 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 4f836147d3..0cb4d4545b 100644
--- a/java/org/apache/catalina/valves/LocalStrings.properties
+++ b/java/org/apache/catalina/valves/LocalStrings.properties
@@ -169,6 +169,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 2a6caf6da4..ae269a800b 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 5934e46978..31e047d748 100644
--- a/java/org/apache/catalina/valves/PersistentValve.java
+++ b/java/org/apache/catalina/valves/PersistentValve.java
@@ -50,8 +50,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
@@ -332,7 +332,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 7c2b435649..af678f943f 100644
--- a/java/org/apache/catalina/valves/RemoteIpValve.java
+++ b/java/org/apache/catalina/valves/RemoteIpValve.java
@@ -20,8 +20,10 @@ 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.List;
import jakarta.servlet.ServletException;
@@ -569,8 +571,14 @@ 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);
+ 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());
+ }
boolean isInternal = isInternalProxy(originalRemoteAddr);
@@ -728,16 +736,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 201e474cbb..3336df886b 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 c6ae6e6e8e..d87bd37608 100644
--- a/java/org/apache/catalina/valves/SSLValve.java
+++ b/java/org/apache/catalina/valves/SSLValve.java
@@ -255,6 +255,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 90a77b915c..99a5593793 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]
