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 fc0e2553db Start another round of code review
fc0e2553db is described below
commit fc0e2553db302a574aba555c87ada2d7f48f3a81
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 bcdd4e7f66..dff4496a12 100644
--- a/java/org/apache/catalina/AccessLog.java
+++ b/java/org/apache/catalina/AccessLog.java
@@ -68,11 +68,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 c59b58be03..bfa5567006 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 88db6d87bb..686f5e480a 100644
--- a/java/org/apache/catalina/Context.java
+++ b/java/org/apache/catalina/Context.java
@@ -1617,7 +1617,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 d465432257..43c6531e90 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
@@ -273,7 +267,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 b2c1209e2f..713740ede4 100644
--- a/java/org/apache/catalina/authenticator/AuthenticatorBase.java
+++ b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
@@ -670,8 +670,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()) {
@@ -1263,7 +1264,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 901614b470..2ee81ccd87 100644
--- a/java/org/apache/catalina/authenticator/BasicAuthenticator.java
+++ b/java/org/apache/catalina/authenticator/BasicAuthenticator.java
@@ -52,9 +52,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;
@@ -66,6 +65,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)
@@ -287,6 +287,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 5922c47625..c557f32e63 100644
--- a/java/org/apache/catalina/authenticator/LocalStrings.properties
+++ b/java/org/apache/catalina/authenticator/LocalStrings.properties
@@ -97,5 +97,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 5312ec525a..1b1eef3948 100644
--- a/java/org/apache/catalina/authenticator/SSLAuthenticator.java
+++ b/java/org/apache/catalina/authenticator/SSLAuthenticator.java
@@ -164,18 +164,21 @@ public class SSLAuthenticator extends AuthenticatorBase {
*/
Container container = getContainer();
if (!(container instanceof Context)) {
+ log.warn(sm.getString("sslAuthenticatorValve.invalidContainer"));
return;
}
Context context = (Context) container;
container = context.getParent();
if (!(container instanceof Host)) {
+ log.warn(sm.getString("sslAuthenticatorValve.invalidContainer"));
return;
}
Host host = (Host) container;
container = host.getParent();
if (!(container instanceof Engine)) {
+ log.warn(sm.getString("sslAuthenticatorValve.invalidContainer"));
return;
}
Engine engine = (Engine) container;
diff --git
a/java/org/apache/catalina/authenticator/jaspic/SimpleServerAuthContext.java
b/java/org/apache/catalina/authenticator/jaspic/SimpleServerAuthContext.java
index 813a647e64..a496d75749 100644
--- a/java/org/apache/catalina/authenticator/jaspic/SimpleServerAuthContext.java
+++ b/java/org/apache/catalina/authenticator/jaspic/SimpleServerAuthContext.java
@@ -48,7 +48,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
@SuppressWarnings("unchecked") // JASPIC API uses raw types
diff --git a/java/org/apache/catalina/connector/Connector.java
b/java/org/apache/catalina/connector/Connector.java
index c4cddd5431..c44c15ecb6 100644
--- a/java/org/apache/catalina/connector/Connector.java
+++ b/java/org/apache/catalina/connector/Connector.java
@@ -112,11 +112,13 @@ public class Connector extends LifecycleMBeanBase {
boolean apr = AprStatus.getUseAprConnector() &&
AprStatus.isInstanceCreated() &&
AprLifecycleListener.isAprAvailable();
ProtocolHandler p = null;
+ Exception ex = null;
try {
p = ProtocolHandler.create(protocol, apr);
} catch (Exception e) {
-
log.error(sm.getString("coyoteConnector.protocolHandlerInstantiationFailed"),
e);
+ ex = e;
}
+ protocolHandlerCreationException = ex;
if (p != null) {
protocolHandler = p;
protocolHandlerClassName = protocolHandler.getClass().getName();
@@ -135,6 +137,7 @@ public class Connector extends LifecycleMBeanBase {
* @param protocolHandler The protocol handler to use
*/
public Connector(ProtocolHandler protocolHandler) {
+ protocolHandlerCreationException = null;
protocolHandlerClassName = protocolHandler.getClass().getName();
this.protocolHandler = protocolHandler;
// Default for Connector depends on this system property
@@ -144,6 +147,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).
*/
@@ -1217,7 +1227,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 4d39ac9d07..27b1e3181e 100644
--- a/java/org/apache/catalina/connector/CoyoteInputStream.java
+++ b/java/org/apache/catalina/connector/CoyoteInputStream.java
@@ -68,7 +68,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 b716a171e9..3ff5c15fe2 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 91a62c38b9..361091c95d 100644
--- a/java/org/apache/catalina/connector/InputBuffer.java
+++ b/java/org/apache/catalina/connector/InputBuffer.java
@@ -191,7 +191,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 fd6c90dacc..e89cfce924 100644
--- a/java/org/apache/catalina/connector/Request.java
+++ b/java/org/apache/catalina/connector/Request.java
@@ -1941,7 +1941,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 96137abac8..7a621bd07b 100644
--- a/java/org/apache/catalina/connector/RequestFacade.java
+++ b/java/org/apache/catalina/connector/RequestFacade.java
@@ -230,7 +230,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 b619b275e2..446837494b 100644
--- a/java/org/apache/catalina/connector/Response.java
+++ b/java/org/apache/catalina/connector/Response.java
@@ -489,9 +489,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
*/
@@ -939,11 +939,15 @@ 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
// the header name is Set-Cookie for both "old" and v.1 ( RFC2109 )
// RFC2965 is not supported by browsers and the Servlet spec
// asks for 2109.
- addHeader("Set-Cookie", header,
getContext().getCookieProcessor().getCharset());
+ addHeader("Set-Cookie", header,
context.getCookieProcessor().getCharset());
}
/**
@@ -1126,10 +1130,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;
}
@@ -1147,7 +1153,7 @@ public class Response implements HttpServletResponse {
try {
absolute = toAbsolute(url);
} catch (IllegalArgumentException iae) {
- // Relative URL
+ // URL construction failed
return url;
}
@@ -1525,7 +1531,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 36f71c03f8..753a0b02e7 100644
--- a/java/org/apache/catalina/connector/ResponseFacade.java
+++ b/java/org/apache/catalina/connector/ResponseFacade.java
@@ -138,7 +138,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]
