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 e053bfdb34 Start another round of code review
e053bfdb34 is described below
commit e053bfdb348ceacda6ff065a5afedf4fdee40518
Author: remm <[email protected]>
AuthorDate: Fri Jun 19 23:25:54 2026 +0200
Start another round of code review
Add a tweak to cors, needs review.
---
java/org/apache/catalina/AccessLog.java | 10 ++++-----
java/org/apache/catalina/Authenticator.java | 2 +-
java/org/apache/catalina/Context.java | 2 +-
.../apache/catalina/ant/AbstractCatalinaTask.java | 10 ++-------
.../apache/catalina/ant/JKStatusUpdateTask.java | 7 ++++---
.../catalina/authenticator/AuthenticatorBase.java | 7 ++++---
.../catalina/authenticator/BasicAuthenticator.java | 5 +++--
.../catalina/authenticator/LocalStrings.properties | 1 +
.../catalina/authenticator/SSLAuthenticator.java | 3 +++
.../jaspic/SimpleServerAuthContext.java | 3 ++-
java/org/apache/catalina/connector/Connector.java | 14 +++++++++++--
.../catalina/connector/CoyoteInputStream.java | 2 +-
.../catalina/connector/CoyoteOutputStream.java | 2 +-
.../org/apache/catalina/connector/InputBuffer.java | 2 +-
java/org/apache/catalina/connector/Request.java | 2 +-
.../apache/catalina/connector/RequestFacade.java | 2 +-
java/org/apache/catalina/connector/Response.java | 24 ++++++++++++++--------
.../apache/catalina/connector/ResponseFacade.java | 2 +-
18 files changed, 59 insertions(+), 41 deletions(-)
diff --git a/java/org/apache/catalina/AccessLog.java
b/java/org/apache/catalina/AccessLog.java
index 2a60d48aa1..6657f3e055 100644
--- a/java/org/apache/catalina/AccessLog.java
+++ b/java/org/apache/catalina/AccessLog.java
@@ -69,11 +69,11 @@ public interface AccessLog {
* Should this valve use request attributes for IP address, hostname,
protocol and port used for the request? The
* attributes used are:
* <ul>
- * <li>org.apache.catalina.RemoteAddr</li>
- * <li>org.apache.catalina.RemoteHost</li>
- * <li>org.apache.catalina.Protocol</li>
- * <li>org.apache.catalina.ServerName</li>
- * <li>org.apache.catalina.ServerPost</li>
+ * <li>org.apache.catalina.AccessLog.RemoteAddr</li>
+ * <li>org.apache.catalina.AccessLog.RemoteHost</li>
+ * <li>org.apache.catalina.AccessLog.Protocol</li>
+ * <li>org.apache.catalina.AccessLog.ServerName</li>
+ * <li>org.apache.catalina.AccessLog.ServerPort</li>
* </ul>
*
* @param requestAttributesEnabled <code>true</code> causes the attributes
to be used, <code>false</code> causes the
diff --git a/java/org/apache/catalina/Authenticator.java
b/java/org/apache/catalina/Authenticator.java
index dcaf8edbd3..abfbbf318c 100644
--- a/java/org/apache/catalina/Authenticator.java
+++ b/java/org/apache/catalina/Authenticator.java
@@ -37,7 +37,7 @@ public interface Authenticator {
* @param request Request we are processing
* @param response Response we are populating
*
- * @return <code>true</code> if any specified constraints have been
satisfied, or <code>false</code> if one more
+ * @return <code>true</code> if any specified constraints have been
satisfied, or <code>false</code> if one or more
* constraints were not satisfied (in which case an
authentication challenge will have been written to
* the response).
*
diff --git a/java/org/apache/catalina/Context.java
b/java/org/apache/catalina/Context.java
index 384355ef64..809f83a009 100644
--- a/java/org/apache/catalina/Context.java
+++ b/java/org/apache/catalina/Context.java
@@ -1618,7 +1618,7 @@ public interface Context extends Container, ContextBind {
* class - IllegalArgumentException will be thrown.
*
* @param clazz Fully qualified class name
- * @param method Post construct method name
+ * @param method Pre destroy method name
*
* @throws IllegalArgumentException if the fully qualified class name or
method name are <code>NULL</code>; if there
* is already pre destroy method
definition for the given class
diff --git a/java/org/apache/catalina/ant/AbstractCatalinaTask.java
b/java/org/apache/catalina/ant/AbstractCatalinaTask.java
index 05fa31d980..85e6b7608d 100644
--- a/java/org/apache/catalina/ant/AbstractCatalinaTask.java
+++ b/java/org/apache/catalina/ant/AbstractCatalinaTask.java
@@ -26,6 +26,7 @@ import java.net.PasswordAuthentication;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URLConnection;
+import java.nio.charset.StandardCharsets;
import org.apache.catalina.util.IOTools;
import org.apache.tomcat.util.http.Method;
@@ -46,13 +47,6 @@ public abstract class AbstractCatalinaTask extends
BaseRedirectorHelperTask {
protected AbstractCatalinaTask() {
}
- // ----------------------------------------------------- Instance Variables
-
- /**
- * manager webapp's encoding.
- */
- private static final String CHARSET = "utf-8";
-
// ------------------------------------------------------------- Properties
@@ -270,7 +264,7 @@ public abstract class AbstractCatalinaTask extends
BaseRedirectorHelperTask {
}
// Process the response message
- reader = new InputStreamReader(hconn.getInputStream(), CHARSET);
+ reader = new InputStreamReader(hconn.getInputStream(),
StandardCharsets.UTF_8);
StringBuilder buff = new StringBuilder();
String error = null;
int msgPriority = Project.MSG_INFO;
diff --git a/java/org/apache/catalina/ant/JKStatusUpdateTask.java
b/java/org/apache/catalina/ant/JKStatusUpdateTask.java
index 1f0c136665..45e6cb70b1 100644
--- a/java/org/apache/catalina/ant/JKStatusUpdateTask.java
+++ b/java/org/apache/catalina/ant/JKStatusUpdateTask.java
@@ -366,8 +366,9 @@ public class JKStatusUpdateTask extends
AbstractCatalinaTask {
sb.append("&ws=");
sb.append(workerStopped);
}
- if ((workerRedirect != null)) { // other worker conrecte lb's
+ if ((workerRedirect != null)) { // redirect to other worker or
load balancer
sb.append("&wr=");
+ sb.append(URLEncoder.encode(workerRedirect, getCharset()));
}
if ((workerClusterDomain != null)) {
sb.append("&wc=");
@@ -400,10 +401,10 @@ public class JKStatusUpdateTask extends
AbstractCatalinaTask {
throw new BuildException(
"Must specify at a lb worker either" +
"'lbStickySession' and 'lbForceSession' attribute");
}
- if (null != lbRecovertime && 60 < lbRecovertime.intValue()) {
+ if (null != lbRecovertime && lbRecovertime.intValue() < 60) {
throw new BuildException("The 'lbRecovertime' must be greater
than 59");
}
- if (null != lbRetries && 1 < lbRetries.intValue()) {
+ if (null != lbRetries && lbRetries.intValue() < 2) {
throw new BuildException("The 'lbRetries' must be greater than
1");
}
isLBMode = true;
diff --git a/java/org/apache/catalina/authenticator/AuthenticatorBase.java
b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
index 26341e4a9d..2451b65b50 100644
--- a/java/org/apache/catalina/authenticator/AuthenticatorBase.java
+++ b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
@@ -671,8 +671,9 @@ public abstract class AuthenticatorBase extends ValveBase
implements Authenticat
// This is a subset of the tests in CorsFilter.checkRequestType
if (Method.OPTIONS.equals(request.getMethod())) {
String originHeader =
request.getHeader(CorsFilter.REQUEST_HEADER_ORIGIN);
- if (originHeader != null && !originHeader.isEmpty() &&
RequestUtil.isValidOrigin(originHeader) &&
- !RequestUtil.isSameOrigin(request, originHeader)) {
+ if (originHeader != null && !originHeader.isEmpty()
+ && !"null".equals(originHeader) &&
RequestUtil.isValidOrigin(originHeader)
+ && !RequestUtil.isSameOrigin(request, originHeader)) {
String accessControlRequestMethodHeader =
request.getHeader(CorsFilter.REQUEST_HEADER_ACCESS_CONTROL_REQUEST_METHOD);
if (accessControlRequestMethodHeader != null &&
!accessControlRequestMethodHeader.isEmpty()) {
@@ -1269,7 +1270,7 @@ public abstract class AuthenticatorBase extends ValveBase
implements Authenticat
* @param username The user
* @param password The password
*
- * @return The authenticated Principal
+ * @return The non null authenticated Principal
*
* @throws ServletException No principal was authenticated with the
specified credentials
*/
diff --git a/java/org/apache/catalina/authenticator/BasicAuthenticator.java
b/java/org/apache/catalina/authenticator/BasicAuthenticator.java
index 68f915bb21..a22d5779f4 100644
--- a/java/org/apache/catalina/authenticator/BasicAuthenticator.java
+++ b/java/org/apache/catalina/authenticator/BasicAuthenticator.java
@@ -51,9 +51,8 @@ public class BasicAuthenticator extends AuthenticatorBase {
/**
* Returns the character set used for encoding credentials, as set by the
user.
- * If the charset was null or empty, the effective charset will be
ISO-8859-1.
*
- * @return the character set name
+ * @return the character set name, the default value is "UTF-8"
*/
public String getCharset() {
return charsetString;
@@ -65,6 +64,7 @@ public class BasicAuthenticator extends AuthenticatorBase {
* ISO-8859-1.
*
* @param charsetString the character set name
+ * @throws IllegalArgumentException if the charset is not supported
*/
public void setCharset(String charsetString) {
// Only acceptable options are null, "" or "UTF-8" (case-insensitive)
@@ -241,6 +241,7 @@ public class BasicAuthenticator extends AuthenticatorBase {
}
}
+ // Tomcat allows a null password
if (colon < 0) {
username = new String(decoded, charset);
// password will remain null!
diff --git a/java/org/apache/catalina/authenticator/LocalStrings.properties
b/java/org/apache/catalina/authenticator/LocalStrings.properties
index 4d29073bdf..b1f1e68c89 100644
--- a/java/org/apache/catalina/authenticator/LocalStrings.properties
+++ b/java/org/apache/catalina/authenticator/LocalStrings.properties
@@ -94,5 +94,6 @@ spnegoAuthenticator.ticketValidateFail=Failed to validate
client supplied ticket
sslAuthenticatorValve.authFailed=Authentication with the provided certificates
failed
sslAuthenticatorValve.http2=The context [{0}] in virtual host [{1}] is
configured to use CLIENT-CERT authentication and [{2}] is configured to support
HTTP/2. Use of CLIENT-CERT authentication is not compatible with the use of
HTTP/2 unless certificateVerification is set to required.
+sslAuthenticatorValve.invalidContainer=The SSL authenticator valve must be
associated with a Context container, with a parent Host and a parent Engine
sslAuthenticatorValve.noCertificates=No certificates are included with this
request
sslAuthenticatorValve.tls13=The context [{0}] in virtual host [{1}] is
configured to use CLIENT-CERT authentication and [{2}] is configured to support
TLS 1.3 using JSSE. Use of CLIENT-CERT authentication is not compatible with
the use of TLS 1.3 and JSSE unless certificateVerification is set to required.
diff --git a/java/org/apache/catalina/authenticator/SSLAuthenticator.java
b/java/org/apache/catalina/authenticator/SSLAuthenticator.java
index 4817d76e19..5eb7b47d74 100644
--- a/java/org/apache/catalina/authenticator/SSLAuthenticator.java
+++ b/java/org/apache/catalina/authenticator/SSLAuthenticator.java
@@ -164,14 +164,17 @@ public class SSLAuthenticator extends AuthenticatorBase {
*/
Container container = getContainer();
if (!(container instanceof Context context)) {
+ log.warn(sm.getString("sslAuthenticatorValve.invalidContainer"));
return;
}
container = context.getParent();
if (!(container instanceof Host host)) {
+ log.warn(sm.getString("sslAuthenticatorValve.invalidContainer"));
return;
}
container = host.getParent();
if (!(container instanceof Engine engine)) {
+ log.warn(sm.getString("sslAuthenticatorValve.invalidContainer"));
return;
}
diff --git
a/java/org/apache/catalina/authenticator/jaspic/SimpleServerAuthContext.java
b/java/org/apache/catalina/authenticator/jaspic/SimpleServerAuthContext.java
index c87ec2950c..4721b5fa71 100644
--- a/java/org/apache/catalina/authenticator/jaspic/SimpleServerAuthContext.java
+++ b/java/org/apache/catalina/authenticator/jaspic/SimpleServerAuthContext.java
@@ -49,7 +49,8 @@ public class SimpleServerAuthContext implements
ServerAuthContext {
/**
* {@inheritDoc}
* <p>
- * Iterates through the configured modules and returns the first result
that is not {@code SEND_FAILURE}.
+ * Iterates through the configured modules in order and returns the first
result that is not
+ * {@code SEND_FAILURE}. If all modules return {@code SEND_FAILURE},
returns {@code SEND_FAILURE}.
*/
@Override
public AuthStatus validateRequest(MessageInfo messageInfo, Subject
clientSubject, Subject serviceSubject)
diff --git a/java/org/apache/catalina/connector/Connector.java
b/java/org/apache/catalina/connector/Connector.java
index 1137ace9e9..e3f89a0eb4 100644
--- a/java/org/apache/catalina/connector/Connector.java
+++ b/java/org/apache/catalina/connector/Connector.java
@@ -101,11 +101,13 @@ public class Connector extends LifecycleMBeanBase {
public Connector(String protocol) {
configuredProtocol = protocol;
ProtocolHandler p = null;
+ Exception ex = null;
try {
p = ProtocolHandler.create(protocol);
} catch (Exception e) {
-
log.error(sm.getString("coyoteConnector.protocolHandlerInstantiationFailed"),
e);
+ ex = e;
}
+ protocolHandlerCreationException = ex;
if (p != null) {
protocolHandler = p;
protocolHandlerClassName = protocolHandler.getClass().getName();
@@ -124,6 +126,7 @@ public class Connector extends LifecycleMBeanBase {
* @param protocolHandler The protocol handler to use
*/
public Connector(ProtocolHandler protocolHandler) {
+ protocolHandlerCreationException = null;
protocolHandlerClassName = protocolHandler.getClass().getName();
configuredProtocol = protocolHandlerClassName;
this.protocolHandler = protocolHandler;
@@ -134,6 +137,13 @@ public class Connector extends LifecycleMBeanBase {
// ----------------------------------------------------- Instance Variables
+
+ /**
+ * Exception creating the protocol handler.
+ */
+ protected final Throwable protocolHandlerCreationException;
+
+
/**
* The <code>Service</code> we are associated with (if any).
*/
@@ -1247,7 +1257,7 @@ public class Connector extends LifecycleMBeanBase {
super.initInternal();
if (protocolHandler == null) {
- throw new
LifecycleException(sm.getString("coyoteConnector.protocolHandlerInstantiationFailed"));
+ throw new
LifecycleException(sm.getString("coyoteConnector.protocolHandlerInstantiationFailed"),
protocolHandlerCreationException);
}
// Initialize adapter
diff --git a/java/org/apache/catalina/connector/CoyoteInputStream.java
b/java/org/apache/catalina/connector/CoyoteInputStream.java
index 71cc3638d8..86617cf2a4 100644
--- a/java/org/apache/catalina/connector/CoyoteInputStream.java
+++ b/java/org/apache/catalina/connector/CoyoteInputStream.java
@@ -64,7 +64,7 @@ public class CoyoteInputStream extends ServletInputStream {
* Prevent cloning the facade.
*/
@Override
- protected Object clone() throws CloneNotSupportedException {
+ public Object clone() throws CloneNotSupportedException {
throw new CloneNotSupportedException();
}
diff --git a/java/org/apache/catalina/connector/CoyoteOutputStream.java
b/java/org/apache/catalina/connector/CoyoteOutputStream.java
index 153276b04e..6c4dfb6aaf 100644
--- a/java/org/apache/catalina/connector/CoyoteOutputStream.java
+++ b/java/org/apache/catalina/connector/CoyoteOutputStream.java
@@ -64,7 +64,7 @@ public class CoyoteOutputStream extends ServletOutputStream {
* Prevent cloning the facade.
*/
@Override
- protected Object clone() throws CloneNotSupportedException {
+ public Object clone() throws CloneNotSupportedException {
throw new CloneNotSupportedException();
}
diff --git a/java/org/apache/catalina/connector/InputBuffer.java
b/java/org/apache/catalina/connector/InputBuffer.java
index d1c7d7529c..c84a38bdda 100644
--- a/java/org/apache/catalina/connector/InputBuffer.java
+++ b/java/org/apache/catalina/connector/InputBuffer.java
@@ -175,7 +175,7 @@ public class InputBuffer extends Reader implements
ByteChunk.ByteInputChannel, A
// --------------------------------------------------------- Public Methods
/**
- * Recycle the output buffer.
+ * Recycle the input buffer.
*/
public void recycle() {
diff --git a/java/org/apache/catalina/connector/Request.java
b/java/org/apache/catalina/connector/Request.java
index 2ab88e0da9..e31540b352 100644
--- a/java/org/apache/catalina/connector/Request.java
+++ b/java/org/apache/catalina/connector/Request.java
@@ -1851,7 +1851,7 @@ public class Request implements HttpServletRequest {
/**
* Get the decoded request URI.
*
- * @return the URL decoded request URI
+ * @return the URL decoded request URI as MessageBytes
*/
public MessageBytes getDecodedRequestURIMB() {
return coyoteRequest.decodedURI();
diff --git a/java/org/apache/catalina/connector/RequestFacade.java
b/java/org/apache/catalina/connector/RequestFacade.java
index ed53ef659c..41b5eec8a8 100644
--- a/java/org/apache/catalina/connector/RequestFacade.java
+++ b/java/org/apache/catalina/connector/RequestFacade.java
@@ -80,7 +80,7 @@ public class RequestFacade implements HttpServletRequest {
* Prevent cloning the facade.
*/
@Override
- protected Object clone() throws CloneNotSupportedException {
+ public Object clone() throws CloneNotSupportedException {
throw new CloneNotSupportedException();
}
diff --git a/java/org/apache/catalina/connector/Response.java
b/java/org/apache/catalina/connector/Response.java
index 6b9229c64b..f7f3299425 100644
--- a/java/org/apache/catalina/connector/Response.java
+++ b/java/org/apache/catalina/connector/Response.java
@@ -445,9 +445,9 @@ public class Response implements HttpServletResponse {
* Return a PrintWriter that can be used to render error messages,
regardless of whether a stream or writer has
* already been acquired.
*
- * @return Writer which can be used for error reports. If the response is
not an error report returned using
- * sendError or triggered by an unexpected exception thrown
during the servlet processing (and only in
- * that case), null will be returned if the response stream
has already been used.
+ * @return Writer which can be used for error reports. Returns null if the
output
+ * buffer is not in a new state (i.e., data has been written
or headers
+ * have been modified).
*
* @exception IOException if an input/output error occurs
*/
@@ -904,8 +904,12 @@ public class Response implements HttpServletResponse {
if (header == null) {
return;
}
+ Context context = getContext();
+ if (context == null) {
+ return;
+ }
// if we reached here, no exception, cookie is valid
- addHeader("Set-Cookie", header,
getContext().getCookieProcessor().getCharset());
+ addHeader("Set-Cookie", header,
context.getCookieProcessor().getCharset());
}
/**
@@ -1088,10 +1092,12 @@ public class Response implements HttpServletResponse {
@Override
public String encodeRedirectURL(String url) {
if (isEncodeable(toAbsolute(url))) {
- return toEncoded(url,
request.getSessionInternal().getIdInternal());
- } else {
- return url;
+ Session session = request.getSessionInternal();
+ if (session != null) {
+ return toEncoded(url, session.getIdInternal());
+ }
}
+ return url;
}
@@ -1102,7 +1108,7 @@ public class Response implements HttpServletResponse {
try {
absolute = toAbsolute(url);
} catch (IllegalArgumentException iae) {
- // Relative URL
+ // URL construction failed
return url;
}
@@ -1414,7 +1420,7 @@ public class Response implements HttpServletResponse {
*
* @param location URL to be (possibly) converted and then returned
*
- * @return the encoded URL
+ * @return the absolute URL
*
* @exception IllegalArgumentException if a MalformedURLException is
thrown when converting the relative URL to an
* absolute one
diff --git a/java/org/apache/catalina/connector/ResponseFacade.java
b/java/org/apache/catalina/connector/ResponseFacade.java
index 1ebf9d9489..3affab22c0 100644
--- a/java/org/apache/catalina/connector/ResponseFacade.java
+++ b/java/org/apache/catalina/connector/ResponseFacade.java
@@ -75,7 +75,7 @@ public class ResponseFacade implements HttpServletResponse {
* Prevent cloning the facade.
*/
@Override
- protected Object clone() throws CloneNotSupportedException {
+ public Object clone() throws CloneNotSupportedException {
throw new CloneNotSupportedException();
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
