You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2026/10/17 16:04:45 UTC
(tomcat) branch 10.1.x updated (e732b3bcfa -> f5c38da8dd)
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a change to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
from e732b3bcfa Report the correct length in Asn1Parser.parseLength() error messages
new fc6c4cf296 Honor the write timeout in AsyncChannelWrapperSecure
new 1c97826065 Fix WebSocket client digest response calculation for qop auth-int
new d25c96829b Reject digest challenges without a realm in WebSocket client
new 93dd66eb44 Use UTF-8 for WebSocket client basic authentication credentials
new 8131080bf1 Reject the permessage-deflate compressed bit on continuation frames
new df373c3464 Report primitive message decode failures as DecodeException
new 79ddf5f954 Make TransformationFactory thread-safe for concurrent registration
new 2d2797f4d0 Fix resource bundle key used by WsFrameClient
new 1ba25169f3 Validate the WebSocket maximum message buffer size
new 108be07936 Fix 32-bit truncation of the session close timeout
new 3fc9ce43f4 Fix 32-bit truncation of the WebSocket client I/O timeout
new 28266f852c Fix NPE handling connections to a POJO endpoint with no annotated methods
new 871f498805 Reject @OnMessage methods with a primitive payload and boolean last flag
new 433b97f098 Fix covariant return type overrides of annotated POJO lifecycle methods wrongly rejected as duplicates
new fd18ad357b Reject WebSocket upgrade requests with an invalid percent-escape sequence in the query string with a 400 response
new f5c38da8dd Remove stray raw path entry added to the WebSocket template match map when a programmatic registration duplicates an annotated POJO template path
The 16 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../websocket/AsyncChannelWrapperSecure.java | 35 ++++-
.../tomcat/websocket/BasicAuthenticator.java | 20 ++-
.../tomcat/websocket/DigestAuthenticator.java | 12 +-
.../tomcat/websocket/LocalStrings.properties | 5 +
.../apache/tomcat/websocket/PerMessageDeflate.java | 5 +-
.../tomcat/websocket/TransformationFactory.java | 11 +-
.../org/apache/tomcat/websocket/WsFrameClient.java | 2 +-
java/org/apache/tomcat/websocket/WsSession.java | 8 +-
.../tomcat/websocket/WsWebSocketContainer.java | 10 +-
.../tomcat/websocket/pojo/LocalStrings.properties | 2 +
.../pojo/PojoMessageHandlerWholeText.java | 8 +-
.../tomcat/websocket/pojo/PojoMethodMapping.java | 14 +-
.../tomcat/websocket/server/UpgradeUtil.java | 20 ++-
.../websocket/server/WsHandshakeRequest.java | 4 +-
.../tomcat/websocket/server/WsServerContainer.java | 13 +-
.../tomcat/websocket/TestBasicAuthenticator.java | 82 +++++++++++
.../tomcat/websocket/TestDigestAuthenticator.java | 102 ++++++++++++++
.../tomcat/websocket/TestPerMessageDeflate.java | 36 +++++
.../websocket/TestTransformationFactory.java | 127 +++++++++++++++++
.../tomcat/websocket/TestWsWebSocketContainer.java | 47 +++++++
.../websocket/pojo/TestPojoEndpointBase.java | 39 ++++++
.../pojo/TestPojoMessageHandlerWholeText.java | 154 +++++++++++++++++++++
.../websocket/pojo/TestPojoMethodMapping.java | 125 +++++++++++++++++
...estKeyHeader.java => TestHandshakeRequest.java} | 59 +++++---
.../websocket/server/TestWsServerContainer.java | 57 ++++++++
webapps/docs/changelog.xml | 22 +++
26 files changed, 967 insertions(+), 52 deletions(-)
create mode 100644 test/org/apache/tomcat/websocket/TestBasicAuthenticator.java
create mode 100644 test/org/apache/tomcat/websocket/TestDigestAuthenticator.java
create mode 100644 test/org/apache/tomcat/websocket/TestTransformationFactory.java
create mode 100644 test/org/apache/tomcat/websocket/pojo/TestPojoMessageHandlerWholeText.java
copy test/org/apache/tomcat/websocket/server/{TestKeyHeader.java => TestHandshakeRequest.java} (52%)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 15/16: Reject WebSocket upgrade requests with an invalid percent-escape sequence in the query string with a 400 response
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit fd18ad357bb16035ad62aa4d733465ec50932925
Author: opencode <re...@apache.org>
AuthorDate: Sat Oct 17 14:24:56 2026 +0200
Reject WebSocket upgrade requests with an invalid percent-escape sequence in the query string with a 400 response
Invalid percent-escape sequences in a query string are not rejected by
the HTTP parser since the individual characters are legal. Escape
pairs are only checked when the query string is decoded. Building the
absolute request URI for the handshake in WsHandshakeRequest uses
java.net.URI with the raw query string, so a sequence such as "?x=%ZZ"
resulted in a URISyntaxException that was wrapped in an
IllegalArgumentException which nothing caught. The uncaught runtime
exception propagated out of the filter chain and produced an HTTP 500
response and a SEVERE container log entry with a stack trace for every
such request from an untrusted client, instead of the 400 Bad Request
response that every other handshake validation failure produces.
UpgradeUtil.doUpgrade() now catches the failure to construct the
WsHandshakeRequest and rejects the handshake with a 400 response. The
response is reset before the error is sent so the rejection is not
accompanied by the Upgrade, Connection and Sec-WebSocket-Accept
headers that indicate a successful handshake.
Adds TestHandshakeRequest covering invalid escape pairs, incomplete
escapes and trailing percent characters in the query string (400), as
well as valid escapes and an empty query string (101). Note that a
query string that is valid URI syntax but undecodable as UTF-8, such
as "?x=%C0%80", is already rejected with a 400 through the separate
InvalidParameterException handling in the valve layer; that behavior
is pinned by a test as well.
---
.../tomcat/websocket/server/UpgradeUtil.java | 13 ++-
.../websocket/server/WsHandshakeRequest.java | 4 +-
.../websocket/server/TestHandshakeRequest.java | 109 +++++++++++++++++++++
3 files changed, 124 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/tomcat/websocket/server/UpgradeUtil.java b/java/org/apache/tomcat/websocket/server/UpgradeUtil.java
index d5e6b37838..8d531e6cec 100644
--- a/java/org/apache/tomcat/websocket/server/UpgradeUtil.java
+++ b/java/org/apache/tomcat/websocket/server/UpgradeUtil.java
@@ -226,7 +226,18 @@ public class UpgradeUtil {
WsPerSessionServerEndpointConfig perSessionServerEndpointConfig = new WsPerSessionServerEndpointConfig(sec);
- WsHandshakeRequest wsRequest = new WsHandshakeRequest(req, pathParams);
+ WsHandshakeRequest wsRequest;
+ try {
+ wsRequest = new WsHandshakeRequest(req, pathParams);
+ } catch (IllegalArgumentException iae) {
+ // The request target could not be represented as a URI, for example because the query
+ // string contains an invalid percent-escape sequence. Reject the handshake. The
+ // response is reset first so the rejection is not sent with headers that would
+ // indicate a successful handshake.
+ resp.reset();
+ resp.sendError(HttpServletResponse.SC_BAD_REQUEST);
+ return;
+ }
WsHandshakeResponse wsResponse = new WsHandshakeResponse();
sec.getConfigurator().modifyHandshake(perSessionServerEndpointConfig, wsRequest, wsResponse);
wsRequest.finished();
diff --git a/java/org/apache/tomcat/websocket/server/WsHandshakeRequest.java b/java/org/apache/tomcat/websocket/server/WsHandshakeRequest.java
index 01daf1a584..0719c8badc 100644
--- a/java/org/apache/tomcat/websocket/server/WsHandshakeRequest.java
+++ b/java/org/apache/tomcat/websocket/server/WsHandshakeRequest.java
@@ -181,7 +181,9 @@ public class WsHandshakeRequest implements HandshakeRequest {
try {
return new URI(uri.toString());
} catch (URISyntaxException e) {
- // Should never happen
+ // This can happen with a client-supplied query string that contains an invalid
+ // percent-escape sequence. Upgrade rejects the handshake with a 400 response in
+ // this case.
throw new IllegalArgumentException(sm.getString("wsHandshakeRequest.invalidUri", uri.toString()), e);
}
}
diff --git a/test/org/apache/tomcat/websocket/server/TestHandshakeRequest.java b/test/org/apache/tomcat/websocket/server/TestHandshakeRequest.java
new file mode 100644
index 0000000000..b435f38137
--- /dev/null
+++ b/test/org/apache/tomcat/websocket/server/TestHandshakeRequest.java
@@ -0,0 +1,109 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.websocket.server;
+
+import java.nio.charset.StandardCharsets;
+
+import jakarta.servlet.http.HttpServletResponse;
+import jakarta.websocket.CloseReason.CloseCodes;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.tomcat.websocket.TesterEchoServer;
+import org.apache.tomcat.websocket.WebSocketBaseTest;
+
+/*
+ * Invalid percent-escape sequences in the query string are not rejected by the HTTP parser (they are only checked
+ * when the query string is decoded). The upgrade processing must reject such requests with a 400 response rather than
+ * failing with an uncaught IllegalArgumentException.
+ */
+public class TestHandshakeRequest extends WebSocketBaseTest {
+
+ @Test
+ public void testInvalidEscapePair() throws Exception {
+ doTest("?x=%ZZ", HttpServletResponse.SC_BAD_REQUEST);
+ }
+
+
+ @Test
+ public void testIncompleteEscape() throws Exception {
+ doTest("?x=%2", HttpServletResponse.SC_BAD_REQUEST);
+ }
+
+
+ @Test
+ public void testTrailingPercent() throws Exception {
+ doTest("?x=%", HttpServletResponse.SC_BAD_REQUEST);
+ }
+
+
+ @Test
+ public void testLowerCaseInvalidEscapePair() throws Exception {
+ doTest("?x=%zz", HttpServletResponse.SC_BAD_REQUEST);
+ }
+
+
+ @Test
+ public void testInvalidEscapeInOneOfMultipleParameters() throws Exception {
+ doTest("?x=1&y=%GG", HttpServletResponse.SC_BAD_REQUEST);
+ }
+
+
+ /*
+ * URI syntax is valid here so the request reaches query string decoding. That mechanism already rejects the
+ * invalid UTF-8 sequence with a 400 response. Pinned here since it is a different code path.
+ */
+ @Test
+ public void testInvalidUtf8Escape() throws Exception {
+ doTest("?x=%C0%80", HttpServletResponse.SC_BAD_REQUEST);
+ }
+
+
+ @Test
+ public void testEmptyQueryString() throws Exception {
+ doTest("?", HttpServletResponse.SC_SWITCHING_PROTOCOLS);
+ }
+
+
+ @Test
+ public void testValidEscape() throws Exception {
+ doTest("?x=%20", HttpServletResponse.SC_SWITCHING_PROTOCOLS);
+ }
+
+
+ private void doTest(String queryString, int expectedStatusCode) throws Exception {
+ startServer(TesterEchoServer.Config.class);
+
+ TesterWsClient client = new TesterWsClient("localhost", getPort());
+ String req = client.createUpgradeRequest(TesterEchoServer.Config.PATH_BASIC + queryString);
+ client.write(req.getBytes(StandardCharsets.UTF_8));
+ int rc = client.readUpgradeResponse();
+
+ Assert.assertEquals(expectedStatusCode, rc);
+
+ if (expectedStatusCode == HttpServletResponse.SC_SWITCHING_PROTOCOLS) {
+ client.sendCloseFrame(CloseCodes.NORMAL_CLOSURE);
+ // Read (and ignore) the response
+ byte[] buf = new byte[256];
+ while (client.read(buf) > 0) {
+ // Ignore
+ }
+ }
+ client.closeSocket();
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 10/16: Fix 32-bit truncation of the session close timeout
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 108be07936c53d7051d5d3239a7f29f03421eaf5
Author: opencode <re...@apache.org>
AuthorDate: Sat Oct 17 12:46:36 2026 +0200
Fix 32-bit truncation of the session close timeout
WsSession.getSessionCloseTimeout() read the Long value of the
org.apache.tomcat.websocket.SESSION_CLOSE_TIMEOUT user property with
intValue(), silently truncating it to 32 bits. A configured timeout
greater than 2^31-1 ms (~24.8 days) was misinterpreted: a value whose
low 32 bits are negative (e.g. 30 days) was rejected by the check for a
non-positive value and silently replaced with the 30 second default,
while other large values were silently shortened to their low 32 bits.
Use longValue(), matching the sibling methods
getAbnormalSessionCloseSendTimeout() and getBlockingSendTimeout(). The
close timeout expiry is compared using the wrap-safe
System.nanoTime() subtraction form, so the now usable full long range
does not introduce any overflow related regression.
---
java/org/apache/tomcat/websocket/WsSession.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/java/org/apache/tomcat/websocket/WsSession.java b/java/org/apache/tomcat/websocket/WsSession.java
index 90f532fd82..e8dff8d270 100644
--- a/java/org/apache/tomcat/websocket/WsSession.java
+++ b/java/org/apache/tomcat/websocket/WsSession.java
@@ -698,7 +698,7 @@ public class WsSession implements Session {
long result = 0;
Object obj = userProperties.get(Constants.SESSION_CLOSE_TIMEOUT_PROPERTY);
if (obj instanceof Long) {
- result = ((Long) obj).intValue();
+ result = ((Long) obj).longValue();
}
if (result <= 0) {
result = Constants.DEFAULT_SESSION_CLOSE_TIMEOUT;
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 03/16: Reject digest challenges without a realm in WebSocket client
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit d25c96829b4852ec32871fb98fc39d2297816b81
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 23:06:23 2026 +0200
Reject digest challenges without a realm in WebSocket client
The mandatory realm parameter of the server Digest challenge (RFC 7616
section 3.3) was not validated. If a non-compliant server omitted the
realm and the user had not configured one, the client sent an
authorization header containing the literal text realm="null" and
computed the digest using the string "null" in A1, resulting in a
malformed header and a silent authentication failure.
Throw an explicit AuthenticationException when the challenge omits the
realm, matching the existing handling of a missing qop value. Add a unit
test for the case.
Found by code review.
---
java/org/apache/tomcat/websocket/DigestAuthenticator.java | 3 +++
java/org/apache/tomcat/websocket/LocalStrings.properties | 1 +
test/org/apache/tomcat/websocket/TestDigestAuthenticator.java | 9 +++++++++
3 files changed, 13 insertions(+)
diff --git a/java/org/apache/tomcat/websocket/DigestAuthenticator.java b/java/org/apache/tomcat/websocket/DigestAuthenticator.java
index f885b55607..0cb58aeb7a 100644
--- a/java/org/apache/tomcat/websocket/DigestAuthenticator.java
+++ b/java/org/apache/tomcat/websocket/DigestAuthenticator.java
@@ -58,6 +58,9 @@ public class DigestAuthenticator extends Authenticator {
Map<String,String> parameterMap = parseAuthenticateHeader(authenticateHeader);
String realm = parameterMap.get("realm");
+ if (realm == null || realm.isEmpty()) {
+ throw new AuthenticationException(sm.getString("digestAuthenticator.noRealm"));
+ }
validateRealm(userRealm, realm);
diff --git a/java/org/apache/tomcat/websocket/LocalStrings.properties b/java/org/apache/tomcat/websocket/LocalStrings.properties
index d6f4ca7bb7..edf3041a57 100644
--- a/java/org/apache/tomcat/websocket/LocalStrings.properties
+++ b/java/org/apache/tomcat/websocket/LocalStrings.properties
@@ -45,6 +45,7 @@ clientEndpointHolder.instanceRegistrationFailed=Failed to register Endpoint inst
digestAuthenticator.algorithm=Unable to generate request digest [{0}]
digestAuthenticator.noQop=The server challenge did not include a qop value
+digestAuthenticator.noRealm=The server challenge did not include a realm value
futureToSendHandler.timeout=Operation timed out after waiting [{0}] [{1}] to complete
diff --git a/test/org/apache/tomcat/websocket/TestDigestAuthenticator.java b/test/org/apache/tomcat/websocket/TestDigestAuthenticator.java
index 682c1973f4..de790d458b 100644
--- a/test/org/apache/tomcat/websocket/TestDigestAuthenticator.java
+++ b/test/org/apache/tomcat/websocket/TestDigestAuthenticator.java
@@ -42,6 +42,15 @@ public class TestDigestAuthenticator {
doTestAuthorization("auth-int");
}
+ @Test(expected = AuthenticationException.class)
+ public void testAuthorizationMissingRealm() throws Exception {
+ String authenticateHeader =
+ "Digest qop=\"auth\", nonce=\"0000f844a605d5c0cb0270f1e2f10ca0\", algorithm=MD5";
+
+ DigestAuthenticator authenticator = new DigestAuthenticator();
+ authenticator.getAuthorization("GET", "/ws/echo", authenticateHeader, "user", "password", null);
+ }
+
private void doTestAuthorization(String qop) throws Exception {
String method = "GET";
String requestUri = "/ws/echo";
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 04/16: Use UTF-8 for WebSocket client basic authentication credentials
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 93dd66eb44303420691b6add4f518c4d5352b8ca
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 23:21:41 2026 +0200
Use UTF-8 for WebSocket client basic authentication credentials
The WebSocket client's BasicAuthenticator only selected UTF-8 when the
server challenge carried a charset parameter with the value UTF-8 (case
insensitively) and silently fell back to ISO-8859-1 for any other
value, including values that RFC 7617 section 2.1 does not allow at
all. Credentials containing characters outside ISO-8859-1 were then
silently corrupted to '?' by the ISO-8859-1 encoding, resulting in
mysterious authentication failures against such servers.
RFC 7617 section 2.1 leaves the default encoding undefined (provided it
is ASCII compatible) and allows only UTF-8 as the value of the charset
parameter, reserving other values for future use. Encode the
credentials with UTF-8 by default and reject challenges asking for any
other encoding rather than silently guessing one. Add a unit test that
checks the encoding byte-wise and the rejection of disallowed values.
Found by code review.
---
.../tomcat/websocket/BasicAuthenticator.java | 20 ++++--
.../tomcat/websocket/LocalStrings.properties | 2 +
.../tomcat/websocket/TestBasicAuthenticator.java | 82 ++++++++++++++++++++++
webapps/docs/changelog.xml | 6 ++
4 files changed, 103 insertions(+), 7 deletions(-)
diff --git a/java/org/apache/tomcat/websocket/BasicAuthenticator.java b/java/org/apache/tomcat/websocket/BasicAuthenticator.java
index 266c2e3c67..649457bac0 100644
--- a/java/org/apache/tomcat/websocket/BasicAuthenticator.java
+++ b/java/org/apache/tomcat/websocket/BasicAuthenticator.java
@@ -16,16 +16,19 @@
*/
package org.apache.tomcat.websocket;
-import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.util.Base64;
import java.util.Map;
+import org.apache.tomcat.util.res.StringManager;
+
/**
* Authenticator supporting the BASIC authentication method.
*/
public class BasicAuthenticator extends Authenticator {
+ private static final StringManager sm = StringManager.getManager(BasicAuthenticator.class);
+
/** Default constructor. */
public BasicAuthenticator() { }
@@ -47,15 +50,18 @@ public class BasicAuthenticator extends Authenticator {
validateRealm(userRealm, realm);
String userPass = userName + ":" + userPassword;
- Charset charset;
- if (parameterMap.get(charsetparam) != null && parameterMap.get(charsetparam).equalsIgnoreCase("UTF-8")) {
- charset = StandardCharsets.UTF_8;
- } else {
- charset = StandardCharsets.ISO_8859_1;
+ /*
+ * RFC 7617 section 2.1 leaves the default character encoding undefined and only allows UTF-8 as the value of
+ * the charset parameter, reserving any other value for future use. Use UTF-8 by default and reject challenges
+ * that ask for any other encoding rather than silently guessing one.
+ */
+ String charsetParam = parameterMap.get(charsetparam);
+ if (charsetParam != null && !charsetParam.equalsIgnoreCase("UTF-8")) {
+ throw new AuthenticationException(sm.getString("basicAuthenticator.invalidCharset", charsetParam));
}
- String base64 = Base64.getEncoder().encodeToString(userPass.getBytes(charset));
+ String base64 = Base64.getEncoder().encodeToString(userPass.getBytes(StandardCharsets.UTF_8));
return " Basic " + base64;
}
diff --git a/java/org/apache/tomcat/websocket/LocalStrings.properties b/java/org/apache/tomcat/websocket/LocalStrings.properties
index edf3041a57..c502427d0a 100644
--- a/java/org/apache/tomcat/websocket/LocalStrings.properties
+++ b/java/org/apache/tomcat/websocket/LocalStrings.properties
@@ -38,6 +38,8 @@ authenticator.realmMismatch=The user provided authentication realm [{0}] did not
backgroundProcessManager.processFailed=A background process failed
+basicAuthenticator.invalidCharset=The value [{0}] of the charset parameter in the authentication challenge is not permitted. The only permitted value is UTF-8
+
caseInsensitiveKeyMap.nullKey=Null keys are not permitted
clientEndpointHolder.instanceCreationFailed=Failed to create WebSocketEndpoint
diff --git a/test/org/apache/tomcat/websocket/TestBasicAuthenticator.java b/test/org/apache/tomcat/websocket/TestBasicAuthenticator.java
new file mode 100644
index 0000000000..62220ef891
--- /dev/null
+++ b/test/org/apache/tomcat/websocket/TestBasicAuthenticator.java
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.websocket;
+
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
+import java.util.Base64;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestBasicAuthenticator {
+
+ @Test
+ public void testAuthorizationNoCharset() throws Exception {
+ // UTF-8 is used by default if no charset is specified. The pound
+ // sign is a single byte in ISO-8859-1 and two bytes in UTF-8 so the
+ // two encodings can be distinguished.
+ String authorization = getAuthorization("Basic realm=\"realm\"", "user", "pass\u00A3word");
+ Assert.assertEquals(" Basic " + encode("user:pass\u00A3word", StandardCharsets.UTF_8), authorization);
+ }
+
+ @Test
+ public void testAuthorizationCharsetUtf8() throws Exception {
+ String authorization = getAuthorization("Basic realm=\"realm\", charset=UTF-8");
+ Assert.assertEquals(" Basic " + encode("user:password", StandardCharsets.UTF_8), authorization);
+ }
+
+ @Test
+ public void testAuthorizationCharsetUtf8NonLatin1() throws Exception {
+ // The UTF-8 bytes of characters that are not present in ISO-8859-1
+ // must be used rather than ISO-8859-1's '?' replacements
+ String authorization = getAuthorization("Basic realm=\"realm\", charset=UTF-8", "user", "\u7528\u6237");
+ Assert.assertEquals(" Basic " + encode("user:\u7528\u6237", StandardCharsets.UTF_8), authorization);
+ }
+
+ @Test
+ public void testAuthorizationCharsetUtf8LowerCase() throws Exception {
+ String authorization = getAuthorization("Basic realm=\"realm\", charset=utf-8");
+ Assert.assertEquals(" Basic " + encode("user:password", StandardCharsets.UTF_8), authorization);
+ }
+
+ @Test(expected = AuthenticationException.class)
+ public void testAuthorizationCharsetIso88592() throws Exception {
+ // RFC 7617 section 2.1: UTF-8 is the only permitted value
+ getAuthorization("Basic realm=\"realm\", charset=ISO-8859-2");
+ }
+
+ @Test(expected = AuthenticationException.class)
+ public void testAuthorizationCharsetInvalid() throws Exception {
+ // RFC 7617 section 2.1: UTF-8 is the only permitted value
+ getAuthorization("Basic realm=\"realm\", charset=garbage");
+ }
+
+ private static String getAuthorization(String authenticateHeader) throws AuthenticationException {
+ return getAuthorization(authenticateHeader, "user", "password");
+ }
+
+ private static String getAuthorization(String authenticateHeader, String userName, String userPassword)
+ throws AuthenticationException {
+ BasicAuthenticator authenticator = new BasicAuthenticator();
+ return authenticator.getAuthorization("GET", "/ws/echo", authenticateHeader, userName, userPassword, null);
+ }
+
+ private static String encode(String value, Charset charset) {
+ return Base64.getEncoder().encodeToString(value.getBytes(charset));
+ }
+}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 980eed0082..cb6ba29eee 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -415,6 +415,12 @@
The hash of the (empty) entity body was omitted, resulting in an
incorrect response that the server always rejected. (remm)
</fix>
+ <fix>
+ Encode the credentials used for basic authentication by the WebSocket
+ client with UTF-8 by default rather than ISO-8859-1, and reject
+ challenges that specify a charset other than UTF-8, the only value
+ RFC 7617 allows. (remm)
+ </fix>
</changelog>
</subsection>
<subsection name="Web applications">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 06/16: Report primitive message decode failures as DecodeException
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit df373c3464d27412dc5e2f9557415eff396ec5c0
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 23:47:17 2026 +0200
Report primitive message decode failures as DecodeException
A @OnMessage method with a primitive (or boxed primitive) text payload
parameter coerces the message via Util.coerceToType(), which throws
NumberFormatException for invalid numeric input and
StringIndexOutOfBoundsException for an empty message targeted at char.
These unchecked exceptions escaped PojoMessageHandlerWholeText.decode(),
passed through PojoMessageHandlerWholeBase.onMessage() which only
catches DecodeException, and were handled by WsFrameBase as an
unexpected failure: the endpoint's onError received the raw runtime
exception and the connection was closed abnormally (1006).
Convert the failure to a jakarta.websocket.DecodeException at the call
site, matching how PojoMethodMapping already wraps the same call for
path parameters. The endpoint's onError now receives a DecodeException
and the connection stays open, as for decoder based endpoints.
Add end-to-end regression tests covering an empty message for a char
parameter and non-numeric text for an int parameter.
---
.../tomcat/websocket/pojo/LocalStrings.properties | 1 +
.../pojo/PojoMessageHandlerWholeText.java | 8 +-
.../pojo/TestPojoMessageHandlerWholeText.java | 154 +++++++++++++++++++++
3 files changed, 162 insertions(+), 1 deletion(-)
diff --git a/java/org/apache/tomcat/websocket/pojo/LocalStrings.properties b/java/org/apache/tomcat/websocket/pojo/LocalStrings.properties
index 52567beb7b..9945db9ca7 100644
--- a/java/org/apache/tomcat/websocket/pojo/LocalStrings.properties
+++ b/java/org/apache/tomcat/websocket/pojo/LocalStrings.properties
@@ -26,6 +26,7 @@ pojoMessageHandlerBase.encodeFail=Encoding failed for POJO of tyoe [{0}] in sess
pojoMessageHandlerBase.onMessageFail=Exception during onMessage call to POJO of type [{0}] in session [{1}]
pojoMessageHandlerWhole.decodeIoFail=IO error while decoding message
+pojoMessageHandlerWhole.decodePrimitiveFail=Failed to decode message value [{0}] to expected type [{1}]
pojoMessageHandlerWhole.maxBufferSize=The maximum supported message size for this implementation is Integer.MAX_VALUE
pojoMessageHandlerWholeBase.decodeDestroyFailed=Failed to destroy the decoder of type [{0}]
diff --git a/java/org/apache/tomcat/websocket/pojo/PojoMessageHandlerWholeText.java b/java/org/apache/tomcat/websocket/pojo/PojoMessageHandlerWholeText.java
index b25a5b2e13..15acf5db4f 100644
--- a/java/org/apache/tomcat/websocket/pojo/PojoMessageHandlerWholeText.java
+++ b/java/org/apache/tomcat/websocket/pojo/PojoMessageHandlerWholeText.java
@@ -105,7 +105,13 @@ public class PojoMessageHandlerWholeText extends PojoMessageHandlerWholeBase<Str
protected Object decode(String message) throws DecodeException {
// Handle primitives
if (primitiveType != null) {
- return Util.coerceToType(primitiveType, message);
+ try {
+ return Util.coerceToType(primitiveType, message);
+ } catch (Exception e) {
+ throw new DecodeException(message,
+ sm.getString("pojoMessageHandlerWhole.decodePrimitiveFail", message,
+ primitiveType.getName()), e);
+ }
}
// Handle full decoders
for (Decoder decoder : decoders) {
diff --git a/test/org/apache/tomcat/websocket/pojo/TestPojoMessageHandlerWholeText.java b/test/org/apache/tomcat/websocket/pojo/TestPojoMessageHandlerWholeText.java
new file mode 100644
index 0000000000..1d4e7c23ae
--- /dev/null
+++ b/test/org/apache/tomcat/websocket/pojo/TestPojoMessageHandlerWholeText.java
@@ -0,0 +1,154 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.websocket.pojo;
+
+import java.net.URI;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.TimeUnit;
+
+import jakarta.websocket.ContainerProvider;
+import jakarta.websocket.DecodeException;
+import jakarta.websocket.OnError;
+import jakarta.websocket.OnMessage;
+import jakarta.websocket.Session;
+import jakarta.websocket.WebSocketContainer;
+import jakarta.websocket.server.ServerEndpoint;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.catalina.Context;
+import org.apache.catalina.servlets.DefaultServlet;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.startup.TomcatBaseTest;
+import org.apache.tomcat.websocket.pojo.TesterUtil.ServerConfigListener;
+import org.apache.tomcat.websocket.pojo.TesterUtil.SingletonConfigurator;
+
+public class TestPojoMessageHandlerWholeText extends TomcatBaseTest {
+
+ private static final long TIMEOUT_SECONDS = 10;
+
+ /*
+ * A @OnMessage method with a char parameter cannot be satisfied by an empty text message. That must be reported to
+ * the endpoint as a DecodeException via onError and must not close the connection.
+ */
+ @Test
+ public void testEmptyTextForCharParameter() throws Exception {
+ CharServer server = new CharServer();
+
+ Session session = connect(server, CharServer.class);
+
+ session.getBasicRemote().sendText("");
+
+ Throwable error = server.errors.poll(TIMEOUT_SECONDS, TimeUnit.SECONDS);
+ Assert.assertNotNull("No error reported to the endpoint", error);
+ Assert.assertTrue("Expected a DecodeException, got: " + error, error instanceof DecodeException);
+
+ // The connection must still be usable after the decode error.
+ session.getBasicRemote().sendText("z");
+ Character received = server.messages.poll(TIMEOUT_SECONDS, TimeUnit.SECONDS);
+ Assert.assertEquals("Endpoint did not receive the message sent after the decode error", Character.valueOf('z'),
+ received);
+
+ session.close();
+ }
+
+
+ /*
+ * A @OnMessage method with an int parameter cannot be satisfied by a non-numeric text message. That must be
+ * reported to the endpoint as a DecodeException via onError and must not close the connection.
+ */
+ @Test
+ public void testNonNumericTextForIntParameter() throws Exception {
+ IntServer server = new IntServer();
+
+ Session session = connect(server, IntServer.class);
+
+ session.getBasicRemote().sendText("abc");
+
+ Throwable error = server.errors.poll(TIMEOUT_SECONDS, TimeUnit.SECONDS);
+ Assert.assertNotNull("No error reported to the endpoint", error);
+ Assert.assertTrue("Expected a DecodeException, got: " + error, error instanceof DecodeException);
+
+ // The connection must still be usable after the decode error.
+ session.getBasicRemote().sendText("123");
+ Integer received = server.messages.poll(TIMEOUT_SECONDS, TimeUnit.SECONDS);
+ Assert.assertEquals("Endpoint did not receive the message sent after the decode error",
+ Integer.valueOf(123), received);
+
+ session.close();
+ }
+
+
+ private Session connect(Object pojo, Class<?> pojoClass) throws Exception {
+ SingletonConfigurator.setInstance(pojo);
+ ServerConfigListener.setPojoClazz(pojoClass);
+
+ Tomcat tomcat = getTomcatInstance();
+ // No file system docBase required
+ Context ctx = getProgrammaticRootContext();
+ ctx.addApplicationListener(ServerConfigListener.class.getName());
+ Tomcat.addServlet(ctx, "default", new DefaultServlet());
+ ctx.addServletMapping("/", "default");
+
+ WebSocketContainer wsContainer = ContainerProvider.getWebSocketContainer();
+
+ tomcat.start();
+
+ return wsContainer.connectToServer(new TesterUtil.SimpleClient(),
+ new URI("ws://localhost:" + getPort() + "/" + pojoClass.getSimpleName().toLowerCase()));
+ }
+
+
+ @ServerEndpoint(value = "/charserver", configurator = SingletonConfigurator.class)
+ public static class CharServer {
+
+ final BlockingQueue<Throwable> errors = new LinkedBlockingQueue<>();
+ final BlockingQueue<Character> messages = new LinkedBlockingQueue<>();
+
+ @OnMessage
+ public void text(char c) {
+ messages.add(Character.valueOf(c));
+ }
+
+
+ @OnError
+ public void error(Throwable t) {
+ errors.add(t);
+ }
+ }
+
+
+ @ServerEndpoint(value = "/intserver", configurator = SingletonConfigurator.class)
+ public static class IntServer {
+
+ final BlockingQueue<Throwable> errors = new LinkedBlockingQueue<>();
+ final BlockingQueue<Integer> messages = new LinkedBlockingQueue<>();
+
+ @OnMessage
+ public void text(int i) {
+ messages.add(Integer.valueOf(i));
+ }
+
+
+ @OnError
+ public void error(Throwable t) {
+ errors.add(t);
+ }
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 01/16: Honor the write timeout in AsyncChannelWrapperSecure
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit fc6c4cf296cf12930d31a5fc2dcc2d28e6a44879
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 22:51:22 2026 +0200
Honor the write timeout in AsyncChannelWrapperSecure
The array based write(...) method ignored its timeout and unit
parameters. WriteTask.run() waited on each socket write with an
unbounded Future.get() so, if the peer stopped accepting data, the
write thread blocked forever, the writing flag was held (blocking all
subsequent writes) and the caller's completion handler never completed.
Compute a deadline from the requested timeout when the write task is
created, with the AsynchronousSocketChannel semantics that a timeout of
zero or less means wait indefinitely. Each socket write now waits only
for the remaining time. On expiry, the pending channel write is
cancelled and the future is failed with a TimeoutException through the
existing error handling, which releases the writing flag and notifies
the completion handler. The single buffer write(ByteBuffer) path, which
has no timeout parameter, is unchanged.
Found by code review.
---
.../websocket/AsyncChannelWrapperSecure.java | 35 ++++++++++++++++++++--
webapps/docs/changelog.xml | 10 +++++++
2 files changed, 42 insertions(+), 3 deletions(-)
diff --git a/java/org/apache/tomcat/websocket/AsyncChannelWrapperSecure.java b/java/org/apache/tomcat/websocket/AsyncChannelWrapperSecure.java
index 38a3f8a407..0535b370ad 100644
--- a/java/org/apache/tomcat/websocket/AsyncChannelWrapperSecure.java
+++ b/java/org/apache/tomcat/websocket/AsyncChannelWrapperSecure.java
@@ -131,7 +131,7 @@ public class AsyncChannelWrapperSecure implements AsyncChannelWrapper {
throw new IllegalStateException(sm.getString("asyncChannelWrapperSecure.concurrentWrite"));
}
- WriteTask writeTask = new WriteTask(srcs, offset, length, future);
+ WriteTask writeTask = new WriteTask(srcs, offset, length, timeout, unit, future);
executor.execute(writeTask);
}
@@ -173,13 +173,24 @@ public class AsyncChannelWrapperSecure implements AsyncChannelWrapper {
private final ByteBuffer[] srcs;
private final int offset;
private final int length;
+ // The time by which the write must complete (a System.nanoTime()
+ // based deadline) or -1 if no timeout was requested
+ private final long deadline;
private final WrapperFuture<Long,?> future;
WriteTask(ByteBuffer[] srcs, int offset, int length, WrapperFuture<Long,?> future) {
+ this(srcs, offset, length, 0, null, future);
+ }
+
+ WriteTask(ByteBuffer[] srcs, int offset, int length, long timeout, TimeUnit unit,
+ WrapperFuture<Long,?> future) {
this.srcs = srcs;
this.future = future;
this.offset = offset;
this.length = length;
+ // Match the AsynchronousSocketChannel semantics: a timeout of
+ // zero or less means wait indefinitely
+ this.deadline = unit == null || timeout <= 0 ? -1 : System.nanoTime() + unit.toNanos(timeout);
}
@Override
@@ -221,8 +232,14 @@ public class AsyncChannelWrapperSecure implements AsyncChannelWrapper {
int toWrite = r.bytesProduced();
while (toWrite > 0) {
Future<Integer> f = socketChannel.write(socketWriteBuffer);
- Integer socketWrite = f.get();
- toWrite -= socketWrite.intValue();
+ try {
+ toWrite -= awaitWrite(f).intValue();
+ } catch (TimeoutException te) {
+ // Ensure the pending write is not left
+ // outstanding on the channel
+ f.cancel(true);
+ throw te;
+ }
}
}
}
@@ -238,6 +255,18 @@ public class AsyncChannelWrapperSecure implements AsyncChannelWrapper {
future.fail(e);
}
}
+
+ private Integer awaitWrite(Future<Integer> f)
+ throws InterruptedException, ExecutionException, TimeoutException {
+ if (deadline == -1) {
+ return f.get();
+ }
+ long remaining = deadline - System.nanoTime();
+ if (remaining <= 0) {
+ throw new TimeoutException();
+ }
+ return f.get(remaining, TimeUnit.NANOSECONDS);
+ }
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 0a6e14ba23..0c0764ad91 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -401,6 +401,16 @@
</fix>
</changelog>
</subsection>
+ <subsection name="WebSocket">
+ <changelog>
+ <fix>
+ Make the WebSocket client secure channel wrapper,
+ <code>AsyncChannelWrapperSecure</code>, honor the timeout specified for
+ write operations so a stalled peer cannot block the write thread
+ indefinitely. (remm)
+ </fix>
+ </changelog>
+ </subsection>
<subsection name="Web applications">
<changelog>
<fix>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 09/16: Validate the WebSocket maximum message buffer size
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 1ba25169f39913ac2006a09230bfff121fb01985
Author: opencode <re...@apache.org>
AuthorDate: Sat Oct 17 11:38:36 2026 +0200
Validate the WebSocket maximum message buffer size
The session setters setMaxBinaryMessageBufferSize() /
setMaxTextMessageBufferSize() and the container default setters
setDefaultMaxBinaryMessageBufferSize() /
setDefaultMaxTextMessageBufferSize() accepted any int without
validation.
With a zero size and a MessageHandler.Partial registered, the frame
receiver enters an unbounded busy loop: the terminal transformation
always reports OVERFLOW because the destination buffer has no room, so
the application handler is invoked repeatedly with empty buffers and
the frame payload is never consumed. A negative size results in a
NegativeArraySizeException when the message buffer is allocated.
Reject values below one with an IllegalArgumentException in the four
setters. Since the session initializes its sizes from the container
defaults and the server context init parameters are applied through the
container setters, this covers all configuration paths. Add a
regression test covering the container and session setters.
---
.../tomcat/websocket/LocalStrings.properties | 2 +
java/org/apache/tomcat/websocket/WsSession.java | 6 +++
.../tomcat/websocket/WsWebSocketContainer.java | 8 ++++
.../tomcat/websocket/TestWsWebSocketContainer.java | 47 ++++++++++++++++++++++
4 files changed, 63 insertions(+)
diff --git a/java/org/apache/tomcat/websocket/LocalStrings.properties b/java/org/apache/tomcat/websocket/LocalStrings.properties
index c502427d0a..c63fb2023e 100644
--- a/java/org/apache/tomcat/websocket/LocalStrings.properties
+++ b/java/org/apache/tomcat/websocket/LocalStrings.properties
@@ -121,6 +121,7 @@ wsSession.duplicateHandlerBinary=A binary message handler has already been confi
wsSession.duplicateHandlerPong=A pong message handler has already been configured
wsSession.duplicateHandlerText=A text message handler has already been configured
wsSession.flushFailOnClose=Failed to flush batched messages on session close
+wsSession.illegalBufferSize=Invalid maximum message buffer size [{0}]. The value must be greater than zero.
wsSession.instanceCreateFailed=Endpoint instance creation failed
wsSession.instanceNew=Endpoint instance registration failed
wsSession.invalidHandlerTypePong=A pong message handler must implement MessageHandler.Whole
@@ -141,6 +142,7 @@ wsWebSocketContainer.connect.write=Writing the HTTP upgrade request from buffer
wsWebSocketContainer.defaultConfiguratorFail=Failed to create the default configurator
wsWebSocketContainer.failedAuthentication=Failed to handle HTTP response code [{0}]. [{1}] header was not accepted by server.
wsWebSocketContainer.httpRequestFailed=The HTTP request to initiate the WebSocket connection to [{0}] failed
+wsWebSocketContainer.illegalBufferSize=Invalid default maximum message buffer size [{0}]. The value must be greater than zero.
wsWebSocketContainer.invalidExtensionParameters=The server responded with extension parameters the client is unable to support
wsWebSocketContainer.invalidExtensionsHeader=The server sent an invalid extension header
wsWebSocketContainer.invalidHeader=Unable to parse HTTP header as no colon is present to delimit header name and header value in [{0}]. The header has been skipped.
diff --git a/java/org/apache/tomcat/websocket/WsSession.java b/java/org/apache/tomcat/websocket/WsSession.java
index ad558d8fd7..90f532fd82 100644
--- a/java/org/apache/tomcat/websocket/WsSession.java
+++ b/java/org/apache/tomcat/websocket/WsSession.java
@@ -495,6 +495,9 @@ public class WsSession implements Session {
@Override
public void setMaxBinaryMessageBufferSize(int max) {
checkState();
+ if (max < 1) {
+ throw new IllegalArgumentException(sm.getString("wsSession.illegalBufferSize", Integer.valueOf(max)));
+ }
this.maxBinaryMessageBufferSize = max;
}
@@ -509,6 +512,9 @@ public class WsSession implements Session {
@Override
public void setMaxTextMessageBufferSize(int max) {
checkState();
+ if (max < 1) {
+ throw new IllegalArgumentException(sm.getString("wsSession.illegalBufferSize", Integer.valueOf(max)));
+ }
this.maxTextMessageBufferSize = max;
}
diff --git a/java/org/apache/tomcat/websocket/WsWebSocketContainer.java b/java/org/apache/tomcat/websocket/WsWebSocketContainer.java
index 62d2d0a007..66efee02f5 100644
--- a/java/org/apache/tomcat/websocket/WsWebSocketContainer.java
+++ b/java/org/apache/tomcat/websocket/WsWebSocketContainer.java
@@ -1061,6 +1061,10 @@ public class WsWebSocketContainer implements WebSocketContainer, BackgroundProce
@Override
public void setDefaultMaxBinaryMessageBufferSize(int max) {
+ if (max < 1) {
+ throw new IllegalArgumentException(
+ sm.getString("wsWebSocketContainer.illegalBufferSize", Integer.valueOf(max)));
+ }
maxBinaryMessageBufferSize = max;
}
@@ -1073,6 +1077,10 @@ public class WsWebSocketContainer implements WebSocketContainer, BackgroundProce
@Override
public void setDefaultMaxTextMessageBufferSize(int max) {
+ if (max < 1) {
+ throw new IllegalArgumentException(
+ sm.getString("wsWebSocketContainer.illegalBufferSize", Integer.valueOf(max)));
+ }
maxTextMessageBufferSize = max;
}
diff --git a/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java b/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java
index 21e332596f..ce42d64a85 100644
--- a/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java
+++ b/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java
@@ -25,6 +25,7 @@ import java.util.Queue;
import java.util.Set;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
+import java.util.function.IntConsumer;
import jakarta.servlet.ServletContextEvent;
import jakarta.websocket.ClientEndpointConfig;
@@ -106,6 +107,52 @@ public class TestWsWebSocketContainer extends WsWebSocketContainerBaseTest {
}
+ @Test
+ public void testMaxMessageBufferSizeValidation() throws Exception {
+ Tomcat tomcat = getTomcatInstance();
+ // No file system docBase required
+ Context ctx = getProgrammaticRootContext();
+ ctx.addApplicationListener(TesterEchoServer.Config.class.getName());
+ Tomcat.addServlet(ctx, "default", new DefaultServlet());
+ ctx.addServletMapping("/", "default");
+
+ tomcat.start();
+
+ WsWebSocketContainer wsContainer = new WsWebSocketContainer();
+
+ doTestIllegalBufferSize(wsContainer::setDefaultMaxBinaryMessageBufferSize);
+ doTestIllegalBufferSize(wsContainer::setDefaultMaxTextMessageBufferSize);
+
+ Session wsSession = wsContainer.connectToServer(TesterProgrammaticEndpoint.class,
+ ClientEndpointConfig.Builder.create().build(),
+ new URI("ws://" + getHostName() + ":" + getPort() + TesterEchoServer.Config.PATH_ASYNC));
+
+ doTestIllegalBufferSize(wsSession::setMaxBinaryMessageBufferSize);
+ doTestIllegalBufferSize(wsSession::setMaxTextMessageBufferSize);
+
+ // Positive values remain valid
+ wsSession.setMaxBinaryMessageBufferSize(1);
+ Assert.assertEquals(1, wsSession.getMaxBinaryMessageBufferSize());
+ wsSession.setMaxTextMessageBufferSize(1);
+ Assert.assertEquals(1, wsSession.getMaxTextMessageBufferSize());
+
+ wsSession.close();
+ wsContainer.destroy();
+ }
+
+
+ private static void doTestIllegalBufferSize(IntConsumer setter) {
+ for (int size : new int[] { 0, -1 }) {
+ try {
+ setter.accept(size);
+ Assert.fail("No exception thrown for buffer size [" + size + "]");
+ } catch (IllegalArgumentException iae) {
+ // Expected
+ }
+ }
+ }
+
+
@Test(expected = DeploymentException.class)
public void testConnectToServerEndpointInvalidScheme() throws Exception {
Tomcat tomcat = getTomcatInstance();
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 11/16: Fix 32-bit truncation of the WebSocket client I/O timeout
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 3fc9ce43f482438fe2567decc525fad03d28ab44
Author: opencode <re...@apache.org>
AuthorDate: Sat Oct 17 12:57:55 2026 +0200
Fix 32-bit truncation of the WebSocket client I/O timeout
The connectToServerRecursive() method read the millisecond value of the
org.apache.tomcat.websocket.IO_TIMEOUT_MS user property with
Long.valueOf(...).intValue(), silently truncating it to 32 bits. A
configured timeout greater than 2^31-1 ms (~24.8 days) was
misinterpreted: a value whose low 32 bits are negative (e.g. 30 days)
became a negative timeout, causing the subsequent Future.get(timeout,
MILLISECONDS) calls for the connection, handshake and request/response
steps to fail immediately, while other large values were silently
shortened to their low 32 bits.
Use longValue(). The value is only ever passed directly to
Future.get(long, TimeUnit.MILLISECONDS), so the restored full long range
introduces no deadline arithmetic issues, and behaviour for values
within the previous int range is unchanged.
---
java/org/apache/tomcat/websocket/WsWebSocketContainer.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/java/org/apache/tomcat/websocket/WsWebSocketContainer.java b/java/org/apache/tomcat/websocket/WsWebSocketContainer.java
index 66efee02f5..859340cd93 100644
--- a/java/org/apache/tomcat/websocket/WsWebSocketContainer.java
+++ b/java/org/apache/tomcat/websocket/WsWebSocketContainer.java
@@ -285,7 +285,7 @@ public class WsWebSocketContainer implements WebSocketContainer, BackgroundProce
long timeout = Constants.IO_TIMEOUT_MS_DEFAULT;
String timeoutValue = (String) userProperties.get(Constants.IO_TIMEOUT_MS_PROPERTY);
if (timeoutValue != null) {
- timeout = Long.valueOf(timeoutValue).intValue();
+ timeout = Long.valueOf(timeoutValue).longValue();
}
AsynchronousSocketChannel socketChannel;
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 12/16: Fix NPE handling connections to a POJO endpoint with no annotated methods
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 28266f852cd74d51108735c78a9c3273720e3346
Author: opencode <re...@apache.org>
AuthorDate: Sat Oct 17 13:19:33 2026 +0200
Fix NPE handling connections to a POJO endpoint with no annotated methods
The method mapping for a POJO @ServerEndpoint class is stored in the user
properties of the ServerEndpointConfig only when the class actually has
annotated methods. PojoEndpointServer.onOpen() and the onClose()/onError()
handling in PojoEndpointBase then dereference that mapping unconditionally,
so the first connection to a class annotated with @ServerEndpoint but with
no @OnOpen, @OnMessage, @OnClose or @OnError method triggered a
NullPointerException. The error handling NPEd a second time and the
exception escaped WsHttpUpgradeHandler.init(), failing the upgrade with an
opaque error instead of a usable session.
The "only if there are methods" condition was introduced for bug 58232 to
avoid storing a mapping for programmatic endpoints (Endpoint subclasses)
since those never use it. That restriction was never intended to cover POJO
endpoints, for which the mapping must always be present. Store it for any
endpoint class that is not an Endpoint subclass, retaining the previous
behaviour for Endpoint subclasses, and always store the mapping created at
upgrade time in UpgradeUtil.doUpgrade() where the surrounding condition
already excludes Endpoint subclasses.
An empty mapping is harmless: PojoMethodMapping performs no validation when
a class has no annotated methods and getMessageHandlers() returns an empty
set, so such an endpoint now behaves like the equivalent @ClientEndpoint
POJO - the connection opens and simply has no handlers.
Add unit tests covering both addEndpoint() paths (stored for a POJO class
with no methods, still not stored for an Endpoint subclass, stored by
upgradeHttpToWebSocket()) and an end-to-end test that a connection to such
an endpoint stays open.
---
.../tomcat/websocket/server/UpgradeUtil.java | 7 +---
.../tomcat/websocket/server/WsServerContainer.java | 8 +++-
.../websocket/pojo/TestPojoEndpointBase.java | 39 +++++++++++++++++++
.../websocket/server/TestWsServerContainer.java | 44 ++++++++++++++++++++++
4 files changed, 92 insertions(+), 6 deletions(-)
diff --git a/java/org/apache/tomcat/websocket/server/UpgradeUtil.java b/java/org/apache/tomcat/websocket/server/UpgradeUtil.java
index 09cc2cc42f..d5e6b37838 100644
--- a/java/org/apache/tomcat/websocket/server/UpgradeUtil.java
+++ b/java/org/apache/tomcat/websocket/server/UpgradeUtil.java
@@ -216,11 +216,8 @@ public class UpgradeUtil {
try {
PojoMethodMapping methodMapping = new PojoMethodMapping(sec.getEndpointClass(), sec.getDecoders(),
sec.getPath(), sc.getInstanceManager(Thread.currentThread().getContextClassLoader()));
- if (methodMapping.getOnClose() != null || methodMapping.getOnOpen() != null ||
- methodMapping.getOnError() != null || methodMapping.hasMessageHandlers()) {
- sec.getUserProperties().put(org.apache.tomcat.websocket.pojo.Constants.POJO_METHOD_MAPPING_KEY,
- methodMapping);
- }
+ sec.getUserProperties().put(org.apache.tomcat.websocket.pojo.Constants.POJO_METHOD_MAPPING_KEY,
+ methodMapping);
} catch (DeploymentException e) {
throw new ServletException(sm.getString("upgradeUtil.pojoMapFail", sec.getEndpointClass().getName()),
e);
diff --git a/java/org/apache/tomcat/websocket/server/WsServerContainer.java b/java/org/apache/tomcat/websocket/server/WsServerContainer.java
index a5ce42e85c..0e1fe472ac 100644
--- a/java/org/apache/tomcat/websocket/server/WsServerContainer.java
+++ b/java/org/apache/tomcat/websocket/server/WsServerContainer.java
@@ -40,6 +40,7 @@ import jakarta.websocket.CloseReason;
import jakarta.websocket.CloseReason.CloseCodes;
import jakarta.websocket.DeploymentException;
import jakarta.websocket.Encoder;
+import jakarta.websocket.Endpoint;
import jakarta.websocket.server.ServerContainer;
import jakarta.websocket.server.ServerEndpoint;
import jakarta.websocket.server.ServerEndpointConfig;
@@ -136,7 +137,12 @@ public class WsServerContainer extends WsWebSocketContainer implements ServerCon
// Add method mapping to user properties
PojoMethodMapping methodMapping = new PojoMethodMapping(sec.getEndpointClass(), sec.getDecoders(), path,
getInstanceManager(Thread.currentThread().getContextClassLoader()));
- if (methodMapping.getOnClose() != null || methodMapping.getOnOpen() != null ||
+ // The mapping is not used for programmatic endpoints (Endpoint subclasses)
+ // so for those classes it is only stored when it has content (bug 58232).
+ // For POJO endpoints it must always be stored, even when empty, since
+ // PojoEndpointServer expects it to be present.
+ if (!Endpoint.class.isAssignableFrom(sec.getEndpointClass()) ||
+ methodMapping.getOnClose() != null || methodMapping.getOnOpen() != null ||
methodMapping.getOnError() != null || methodMapping.hasMessageHandlers()) {
sec.getUserProperties().put(org.apache.tomcat.websocket.pojo.Constants.POJO_METHOD_MAPPING_KEY,
methodMapping);
diff --git a/test/org/apache/tomcat/websocket/pojo/TestPojoEndpointBase.java b/test/org/apache/tomcat/websocket/pojo/TestPojoEndpointBase.java
index 16b1ddf5bf..341135b17f 100644
--- a/test/org/apache/tomcat/websocket/pojo/TestPojoEndpointBase.java
+++ b/test/org/apache/tomcat/websocket/pojo/TestPojoEndpointBase.java
@@ -120,6 +120,45 @@ public class TestPojoEndpointBase extends TomcatBaseTest {
}
+ /*
+ * A POJO endpoint class with no annotated methods deploys but has no handlers.
+ * Previously, the method mapping was not made available to the endpoint wrapper
+ * for such a class, resulting in a NullPointerException when handling the first
+ * connection and an abnormal close of that connection.
+ */
+ @Test
+ public void testNoMethodsServerEndpoint() throws Exception {
+ // Set up utility classes
+ ServerConfigListener.setPojoClazz(NoMethodsServerEndpoint.class);
+
+ Tomcat tomcat = getTomcatInstance();
+ // No file system docBase required
+ Context ctx = getProgrammaticRootContext();
+ ctx.addApplicationListener(ServerConfigListener.class.getName());
+ Tomcat.addServlet(ctx, "default", new DefaultServlet());
+ ctx.addServletMapping("/", "default");
+
+ WebSocketContainer wsContainer = ContainerProvider.getWebSocketContainer();
+
+ tomcat.start();
+
+ Client client = new Client();
+ URI uri = new URI("ws://localhost:" + getPort() + "/");
+
+ Session session = wsContainer.connectToServer(client, uri);
+
+ // Nothing should happen on the connection. Prior to the fix, the
+ // connection was closed abnormally as soon as it was opened.
+ Assert.assertFalse("Connection was closed unexpectedly", client.waitForClose(2));
+ Assert.assertTrue(session.isOpen());
+ }
+
+
+ @ServerEndpoint("/")
+ public static class NoMethodsServerEndpoint {
+ }
+
+
@ServerEndpoint("/")
public static class Bug54716 {
diff --git a/test/org/apache/tomcat/websocket/server/TestWsServerContainer.java b/test/org/apache/tomcat/websocket/server/TestWsServerContainer.java
index ef7b777b14..6ebb47775a 100644
--- a/test/org/apache/tomcat/websocket/server/TestWsServerContainer.java
+++ b/test/org/apache/tomcat/websocket/server/TestWsServerContainer.java
@@ -33,6 +33,8 @@ import jakarta.websocket.ContainerProvider;
import jakarta.websocket.Decoder;
import jakarta.websocket.DeploymentException;
import jakarta.websocket.Encoder;
+import jakarta.websocket.Endpoint;
+import jakarta.websocket.EndpointConfig;
import jakarta.websocket.HandshakeResponse;
import jakarta.websocket.Session;
import jakarta.websocket.WebSocketContainer;
@@ -439,6 +441,44 @@ public class TestWsServerContainer extends WebSocketBaseTest {
public static class PojoTemplate {
}
+
+ /*
+ * The method mapping must be stored in the user properties for POJO endpoints even when the class has no annotated
+ * methods since PojoEndpointServer expects it to be present. Without it, handling the first connection results in a
+ * NullPointerException. The mapping must not be stored for programmatic endpoints (Endpoint subclasses) since it is
+ * not used (see bug 58232).
+ */
+ @Test
+ public void testMethodMappingStoredForPojoWithNoAnnotatedMethods() throws Exception {
+ WsServerContainer sc = new WsServerContainer(new TesterServletContext());
+
+ ServerEndpointConfig pojoConfig = ServerEndpointConfig.Builder.create(PojoNoMethods.class, "/noMethods")
+ .build();
+ sc.addEndpoint(pojoConfig);
+ Assert.assertNotNull(pojoConfig.getUserProperties()
+ .get(org.apache.tomcat.websocket.pojo.Constants.POJO_METHOD_MAPPING_KEY));
+
+ ServerEndpointConfig endpointConfig = ServerEndpointConfig.Builder.create(NoMethodsEndpoint.class, "/endpoint")
+ .build();
+ sc.addEndpoint(endpointConfig);
+ Assert.assertNull(endpointConfig.getUserProperties()
+ .get(org.apache.tomcat.websocket.pojo.Constants.POJO_METHOD_MAPPING_KEY));
+ }
+
+
+ @ServerEndpoint("/noMethods")
+ public static class PojoNoMethods {
+ }
+
+
+ public static class NoMethodsEndpoint extends Endpoint {
+ @Override
+ public void onOpen(@SuppressWarnings("unused") Session session,
+ @SuppressWarnings("unused") EndpointConfig config) {
+ // NO-OP
+ }
+ }
+
@Test
public void testUpgradeHttpToWebSocket01() throws Exception {
TesterHttpServletRequest request = new TesterHttpServletRequest();
@@ -462,6 +502,10 @@ public class TestWsServerContainer extends WebSocketBaseTest {
Assert.assertEquals(Constants.UPGRADE_HEADER_VALUE, response.getHeader(Constants.UPGRADE_HEADER_NAME));
Assert.assertEquals("aJvXj0bbnSSeXm32ngvbBilP0lE=", response.getHeader(HandshakeResponse.SEC_WEBSOCKET_ACCEPT));
+ // The endpoint class is not an Endpoint subclass so the method mapping must
+ // have been created and stored for the POJO endpoint path.
+ Assert.assertNotNull(sec.getUserProperties()
+ .get(org.apache.tomcat.websocket.pojo.Constants.POJO_METHOD_MAPPING_KEY));
}
@Test(expected = DeploymentException.class)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 16/16: Remove stray raw path entry added to the WebSocket template match map when a programmatic registration duplicates an annotated POJO template path
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit f5c38da8ddb70c052e1606a5969fdd6c00bf3a4d
Author: opencode <re...@apache.org>
AuthorDate: Sat Oct 17 14:53:25 2026 +0200
Remove stray raw path entry added to the WebSocket template match map
when a programmatic registration duplicates an annotated POJO template
path
The template match map is keyed by the normalized path in which named
template parameters are replaced with numeric indices. In the case in
which a programmatic registration duplicates the template path of an
annotated POJO for the same endpoint class, the specification requires
the new match to be ignored. The code copied from the exact match
branch below restored the old match into the map using the raw path as
the key. Unlike the exact match branch, the template branch uses
putIfAbsent() which does not modify the map when a match already
exists, so no restoration is required and the put instead added a
second, raw path keyed entry to the map for every such registration,
both keys referencing the same match.
The redundant put is removed. The exact match branch is unchanged
since there the preceding put() does overwrite the old match and the
restoration is required.
The regression test extends testDuplicatePaths22, which exercises this
code path, to assert via reflection on the template match map that
exactly one entry, keyed by the normalized path, remains after the
duplicate registration is ignored.
---
.../apache/tomcat/websocket/server/WsServerContainer.java | 5 +++--
.../tomcat/websocket/server/TestWsServerContainer.java | 13 +++++++++++++
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/tomcat/websocket/server/WsServerContainer.java b/java/org/apache/tomcat/websocket/server/WsServerContainer.java
index 0e1fe472ac..5e7f341aa9 100644
--- a/java/org/apache/tomcat/websocket/server/WsServerContainer.java
+++ b/java/org/apache/tomcat/websocket/server/WsServerContainer.java
@@ -166,8 +166,9 @@ public class WsServerContainer extends WsWebSocketContainer implements ServerCon
// before POJOs in WsSci#onStartup()
if (oldMatch.isFromAnnotatedPojo() && !newMatch.isFromAnnotatedPojo() &&
oldMatch.getConfig().getEndpointClass() == newMatch.getConfig().getEndpointClass()) {
- // The WebSocket spec says to ignore the new match in this case
- templateMatches.put(path, oldMatch);
+ // The WebSocket spec says to ignore the new match in this case. The old
+ // match has not been replaced by putIfAbsent() above so, unlike the exact
+ // match case below, there is no need to restore the old match in the map.
} else {
// Duplicate uriTemplate;
throw new DeploymentException(sm.getString("serverContainer.duplicatePaths", path,
diff --git a/test/org/apache/tomcat/websocket/server/TestWsServerContainer.java b/test/org/apache/tomcat/websocket/server/TestWsServerContainer.java
index 6ebb47775a..3c77c07ad3 100644
--- a/test/org/apache/tomcat/websocket/server/TestWsServerContainer.java
+++ b/test/org/apache/tomcat/websocket/server/TestWsServerContainer.java
@@ -364,6 +364,19 @@ public class TestWsServerContainer extends WebSocketBaseTest {
sc.addEndpoint(configA);
Assert.assertNotEquals(configA, sc.findMapping("/foo/{a}").getConfig());
+
+ // The old match must remain stored under the normalized path. Ignoring the new match must
+ // not add an additional entry to the map.
+ Field configTemplateMatchMapField = WsServerContainer.class.getDeclaredField("configTemplateMatchMap");
+ configTemplateMatchMapField.setAccessible(true);
+ @SuppressWarnings("unchecked")
+ Map<Integer, ConcurrentSkipListMap<String, Object>> configTemplateMatchMap =
+ (Map<Integer, ConcurrentSkipListMap<String, Object>>) configTemplateMatchMapField.get(sc);
+
+ ConcurrentSkipListMap<String, Object> templateMatches = configTemplateMatchMap.get(Integer.valueOf(2));
+
+ Assert.assertEquals(1, templateMatches.size());
+ Assert.assertTrue(templateMatches.containsKey("/foo/{0}"));
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 07/16: Make TransformationFactory thread-safe for concurrent registration
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 79ddf5f954280e405ae3366870834b3aa1249a15
Author: opencode <re...@apache.org>
AuthorDate: Sat Oct 17 00:37:12 2026 +0200
Make TransformationFactory thread-safe for concurrent registration
TransformationFactory is a JVM-wide singleton. Its builders map was a
plain HashMap mutated by the public registerExtension() method, which
third parties (e.g. TCK tests installing custom transformations) may
call at any time, while create() is invoked on every WebSocket
handshake from request threads and getInstalledExtensionNames()/
getInstalledExtensions() from client application threads. A concurrent
put racing get/iteration on a HashMap can lose entries or throw
ConcurrentModificationException from the handshake path.
Use a ConcurrentHashMap so registration and reads are safe without
external coordination. A test exercising concurrent registration and
handshake-style reads reproduces the ConcurrentModificationException
on the previous HashMap implementation.
---
.../tomcat/websocket/TransformationFactory.java | 11 +-
.../websocket/TestTransformationFactory.java | 127 +++++++++++++++++++++
2 files changed, 135 insertions(+), 3 deletions(-)
diff --git a/java/org/apache/tomcat/websocket/TransformationFactory.java b/java/org/apache/tomcat/websocket/TransformationFactory.java
index f8472ec82a..45dccb0dc7 100644
--- a/java/org/apache/tomcat/websocket/TransformationFactory.java
+++ b/java/org/apache/tomcat/websocket/TransformationFactory.java
@@ -17,11 +17,11 @@
package org.apache.tomcat.websocket;
import java.util.Collections;
-import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
import jakarta.websocket.Extension;
@@ -32,7 +32,11 @@ public class TransformationFactory {
private static final TransformationFactory factory = new TransformationFactory();
- private final Map<String,TransformationBuilder> builders = new HashMap<>();
+ /*
+ * Concurrent map since registerExtension() may be called at any time by an application while create() and the
+ * getInstalled*() methods are invoked concurrently by threads processing handshakes or by client applications.
+ */
+ private final Map<String,TransformationBuilder> builders = new ConcurrentHashMap<>();
private TransformationFactory() {
@@ -71,7 +75,8 @@ public class TransformationFactory {
/**
- * Registers a transformation builder for the given extension name.
+ * Registers a transformation builder for the given extension name. This method is thread-safe and may be called
+ * at any time, including while handshakes are in progress.
*
* @param name the extension name
* @param builder the transformation builder
diff --git a/test/org/apache/tomcat/websocket/TestTransformationFactory.java b/test/org/apache/tomcat/websocket/TestTransformationFactory.java
new file mode 100644
index 0000000000..9f7b57cc15
--- /dev/null
+++ b/test/org/apache/tomcat/websocket/TestTransformationFactory.java
@@ -0,0 +1,127 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.websocket;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+
+import jakarta.websocket.Extension.Parameter;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestTransformationFactory {
+
+ /* A single extension offer with no parameters (i.e. use the defaults). */
+ private static final List<List<Parameter>> DEFAULT_OFFER = Collections.singletonList(Collections.emptyList());
+
+ @Test
+ public void testBuiltInExtensionIsAvailable() {
+ TransformationFactory factory = TransformationFactory.getInstance();
+ Assert.assertTrue(factory.getInstalledExtensionNames().contains(PerMessageDeflate.NAME));
+ Assert.assertNotNull(factory.create(PerMessageDeflate.NAME, DEFAULT_OFFER, true));
+ }
+
+
+ @Test
+ public void testCreateUnknownExtensionReturnsNull() {
+ Assert.assertNull(
+ TransformationFactory.getInstance().create("test-unknown-extension-" + System.nanoTime(), DEFAULT_OFFER,
+ true));
+ }
+
+
+ /*
+ * registerExtension() is public API that may be called at any time, while create() and the getInstalled*() methods
+ * are called concurrently by threads processing handshakes. Concurrent registration and reads must neither throw
+ * nor lose entries.
+ */
+ @Test
+ public void testConcurrentRegistrationAndReads() throws Exception {
+ TransformationFactory factory = TransformationFactory.getInstance();
+
+ int writerCount = 4;
+ int readerCount = 4;
+ int extensionsPerWriter = 50;
+ int readerIterations = 2000;
+
+ List<Throwable> failures = new CopyOnWriteArrayList<>();
+ CountDownLatch startLine = new CountDownLatch(1);
+ ExecutorService executor = Executors.newFixedThreadPool(writerCount + readerCount);
+
+ String prefix = "test-ext-" + System.nanoTime() + "-";
+
+ List<String> expected = new ArrayList<>();
+ for (int w = 0; w < writerCount; w++) {
+ for (int i = 0; i < extensionsPerWriter; i++) {
+ expected.add(prefix + w + '-' + i);
+ }
+ }
+
+ for (int w = 0; w < writerCount; w++) {
+ final int writer = w;
+ executor.execute(() -> {
+ try {
+ startLine.await();
+ for (int i = 0; i < extensionsPerWriter; i++) {
+ String name = prefix + writer + '-' + i;
+ factory.registerExtension(name, PerMessageDeflate.BUILDER);
+ }
+ } catch (Throwable t) {
+ failures.add(t);
+ }
+ });
+ }
+
+ for (int r = 0; r < readerCount; r++) {
+ executor.execute(() -> {
+ try {
+ startLine.await();
+ for (int i = 0; i < readerIterations; i++) {
+ // Exercise each read path used by the handshake processing code.
+ factory.create(PerMessageDeflate.NAME, DEFAULT_OFFER, true);
+ factory.getInstalledExtensionNames();
+ factory.getInstalledExtensions();
+ }
+ } catch (Throwable t) {
+ failures.add(t);
+ }
+ });
+ }
+
+ startLine.countDown();
+ executor.shutdown();
+ Assert.assertTrue("Concurrent test did not complete in time",
+ executor.awaitTermination(60, TimeUnit.SECONDS));
+
+ Assert.assertTrue("Unexpected failures during concurrent access: " + failures, failures.isEmpty());
+
+ // Every registered extension must be visible once registration has completed.
+ for (String name : expected) {
+ Assert.assertTrue("Registered extension not visible: " + name,
+ factory.getInstalledExtensionNames().contains(name));
+ Assert.assertNotNull("Create failed for registered extension: " + name,
+ factory.create(name, DEFAULT_OFFER, true));
+ }
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 05/16: Reject the permessage-deflate compressed bit on continuation frames
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 8131080bf15d6127662565d9610735ebe5544bf2
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 23:37:44 2026 +0200
Reject the permessage-deflate compressed bit on continuation frames
RFC 7692 section 6.1: "An endpoint MUST NOT set the 'Per-Message
Compressed' bit of control frames and non-first fragments of a data
message. An endpoint receiving such a frame MUST Fail the WebSocket
Connection."
PerMessageDeflate.validateRsv() only enforced this for control frames.
For any other opcode it stripped RSV1 and delegated to the next
transformation, so a continuation frame carrying the bit was silently
accepted instead of failing the connection. Route continuation frames
(opcode 0x00, non-first fragments by definition) through the same
reject path so WsFrameBase closes the connection with PROTOCOL_ERROR,
as the RFC requires.
Tomcat's own sender sets RSV1 only on the first compressed frame of a
message (firstCompressedFrameWritten), so this cannot break compressed
fragmented messages between Tomcat endpoints.
Add a regression test to TestPerMessageDeflate covering RSV1 on text,
binary, continuation and control frames, plus RSV clear on continuations.
---
.../apache/tomcat/websocket/PerMessageDeflate.java | 5 ++-
.../tomcat/websocket/TestPerMessageDeflate.java | 36 ++++++++++++++++++++++
2 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/java/org/apache/tomcat/websocket/PerMessageDeflate.java b/java/org/apache/tomcat/websocket/PerMessageDeflate.java
index eaa55f8ad0..1c74e1919a 100644
--- a/java/org/apache/tomcat/websocket/PerMessageDeflate.java
+++ b/java/org/apache/tomcat/websocket/PerMessageDeflate.java
@@ -461,7 +461,10 @@ public class PerMessageDeflate implements Transformation {
@Override
public boolean validateRsv(int rsv, byte opCode) {
- if (Util.isControl(opCode)) {
+ if (Util.isControl(opCode) || Util.isContinuation(opCode)) {
+ // RFC 7692 section 6.1: An endpoint MUST NOT set the "Per-Message Compressed" bit of
+ // control frames and non-first fragments of a data message. An endpoint receiving such
+ // a frame MUST Fail the WebSocket Connection.
if ((rsv & RSV_BITMASK) != 0) {
return false;
} else {
diff --git a/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java b/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
index 1374652bc6..893905b760 100644
--- a/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
+++ b/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
@@ -790,6 +790,42 @@ public class TestPerMessageDeflate {
Assert.assertNull(PerMessageDeflate.build(preferences, true));
}
+
+ /*
+ * RFC 7692 section 6.1: "An endpoint MUST NOT set the 'Per-Message Compressed' bit of control frames and non-first
+ * fragments of a data message. An endpoint receiving such a frame MUST Fail the WebSocket Connection." The RSV1 bit
+ * is only meaningful on the first frame of a compressed data message. Continuation frames (non-first fragments) and
+ * control frames must not carry it.
+ */
+ @Test
+ public void testRsvBitMustNotBeSetOnContinuationFrames() {
+ List<List<Parameter>> preferences = new ArrayList<>();
+ preferences.add(Collections.emptyList());
+
+ // Standalone instance (no next transformation), as the terminal transformation would accept rsv==0.
+ PerMessageDeflate perMessageDeflate = PerMessageDeflate.build(preferences, true);
+
+ // First frame of a compressed data message: RSV1 set is valid.
+ Assert.assertTrue("RSV1 set on a text frame should be accepted",
+ perMessageDeflate.validateRsv(0b100, Constants.OPCODE_TEXT));
+ Assert.assertTrue("RSV1 set on a binary frame should be accepted",
+ perMessageDeflate.validateRsv(0b100, Constants.OPCODE_BINARY));
+
+ // Continuation frame (non-first fragment) with RSV1 set: must fail the connection.
+ Assert.assertFalse("RSV1 set on a continuation frame should be rejected",
+ perMessageDeflate.validateRsv(0b100, Constants.OPCODE_CONTINUATION));
+
+ // Control frame with RSV1 set: must fail the connection (already the case).
+ Assert.assertFalse("RSV1 set on a control frame should be rejected",
+ perMessageDeflate.validateRsv(0b100, Constants.OPCODE_PING));
+
+ // Continuation frame without RSV1: valid (this is the normal case for a
+ // fragmented compressed message).
+ Assert.assertTrue("RSV clear on a continuation frame should be accepted",
+ perMessageDeflate.validateRsv(0, Constants.OPCODE_CONTINUATION));
+ }
+
+
/*
* Minimal implementation to enable other transformations to be tested. It is NOT robust.
*/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 08/16: Fix resource bundle key used by WsFrameClient
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 2d2797f4d05d39ed8eeb78784f819a72d5b1bd2e
Author: opencode <re...@apache.org>
AuthorDate: Sat Oct 17 11:18:12 2026 +0200
Fix resource bundle key used by WsFrameClient
The defensive IllegalStateException thrown by the default branch of
processSocketRead() used the message key
"wsFrameServer.illegalReadState". That key is only defined in the
org.apache.tomcat.websocket.server resource bundle. WsFrameClient's
StringManager loads the bundle of its own package
(org.apache.tomcat.websocket), so the lookup failed and the exception
message degraded to the raw key instead of the intended
"Unexpected read state [X]", making diagnosis of the (rare) read state
race confusing.
Use the key "wsFrame.illegalReadState", which is defined in the
websocket bundle and is already used correctly elsewhere in the same
class. This is a copy-paste leftover from WsFrameServer, which
legitimately uses the server bundle key.
---
java/org/apache/tomcat/websocket/WsFrameClient.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/java/org/apache/tomcat/websocket/WsFrameClient.java b/java/org/apache/tomcat/websocket/WsFrameClient.java
index 29572b1de3..9b639f044b 100644
--- a/java/org/apache/tomcat/websocket/WsFrameClient.java
+++ b/java/org/apache/tomcat/websocket/WsFrameClient.java
@@ -120,7 +120,7 @@ public class WsFrameClient extends WsFrameBase {
}
return;
default:
- throw new IllegalStateException(sm.getString("wsFrameServer.illegalReadState", getReadState()));
+ throw new IllegalStateException(sm.getString("wsFrame.illegalReadState", getReadState()));
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 02/16: Fix WebSocket client digest response calculation for qop auth-int
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 1c978260653c71d3535c56f17464e4c464263fad
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 23:00:22 2026 +0200
Fix WebSocket client digest response calculation for qop auth-int
When the server challenge used qop="auth-int", the WebSocket client's
DigestAuthenticator computed A2 as Method ":" digest-uri-value,
omitting the H(entity-body) term required by RFC 2617 section 3.3.2 and
RFC 7616. A WebSocket handshake has no entity body but the hash of the
empty string still has to be included. Without it, the generated
response value was always rejected by such servers, making the client
unable to authenticate to third-party servers that advertise auth-int.
Append the hash of the empty entity body to A2 when the challenge qop
is auth-int and correct the comment that claimed the term was not
needed.
Add a unit test that checks the generated response value against an
independent, RFC based calculation done the same way a server would, for
both qop=auth and qop=auth-int. The auth-int case fails against the
previous code.
Found by code review.
---
.../tomcat/websocket/DigestAuthenticator.java | 9 ++-
.../tomcat/websocket/TestDigestAuthenticator.java | 93 ++++++++++++++++++++++
webapps/docs/changelog.xml | 6 ++
3 files changed, 106 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/tomcat/websocket/DigestAuthenticator.java b/java/org/apache/tomcat/websocket/DigestAuthenticator.java
index 405d654159..f885b55607 100644
--- a/java/org/apache/tomcat/websocket/DigestAuthenticator.java
+++ b/java/org/apache/tomcat/websocket/DigestAuthenticator.java
@@ -129,10 +129,15 @@ public class DigestAuthenticator extends Authenticator {
}
/*
- * If the "qop" value is "auth-int", then A2 is: A2 = Method ":" digest-uri-value ":" H(entity-body) since we do
- * not have an entity-body, A2 = Method ":" digest-uri-value for auth and auth_int
+ * If the "qop" value is "auth-int", then A2 is:
+ * A2 = Method ":" digest-uri-value ":" H(entity-body)
+ * There is no entity-body for a WebSocket handshake but the hash of
+ * the empty body still has to be included.
*/
String A2 = method + ":" + requestUri;
+ if ("auth-int".equals(qop)) {
+ A2 = A2 + ":" + encode(algorithm, "");
+ }
preDigest.append(encode(algorithm, A1));
preDigest.append(':');
diff --git a/test/org/apache/tomcat/websocket/TestDigestAuthenticator.java b/test/org/apache/tomcat/websocket/TestDigestAuthenticator.java
new file mode 100644
index 0000000000..682c1973f4
--- /dev/null
+++ b/test/org/apache/tomcat/websocket/TestDigestAuthenticator.java
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.websocket;
+
+import java.nio.charset.StandardCharsets;
+import java.security.MessageDigest;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.tomcat.util.buf.HexUtils;
+
+public class TestDigestAuthenticator {
+
+ private static final Pattern RESPONSE_PATTERN = Pattern.compile("response=\"([^\"]+)\"");
+ private static final Pattern CNONCE_PATTERN = Pattern.compile("cnonce=\"([^\"]+)\"");
+ private static final Pattern NC_PATTERN = Pattern.compile("nc=([0-9a-fA-F]{8})");
+
+ @Test
+ public void testAuthorizationQopAuth() throws Exception {
+ doTestAuthorization("auth");
+ }
+
+ @Test
+ public void testAuthorizationQopAuthInt() throws Exception {
+ doTestAuthorization("auth-int");
+ }
+
+ private void doTestAuthorization(String qop) throws Exception {
+ String method = "GET";
+ String requestUri = "/ws/echo";
+ String userName = "user";
+ String password = "password";
+ String realm = "realm";
+ String nonce = "0000f844a605d5c0cb0270f1e2f10ca0";
+ String algorithm = "MD5";
+
+ String authenticateHeader = "Digest realm=\"" + realm + "\", qop=\"" + qop + "\", nonce=\"" + nonce +
+ "\", algorithm=" + algorithm;
+
+ DigestAuthenticator authenticator = new DigestAuthenticator();
+ String authorization =
+ authenticator.getAuthorization(method, requestUri, authenticateHeader, userName, password, null);
+
+ String response = extract(RESPONSE_PATTERN, authorization);
+ String cnonce = extract(CNONCE_PATTERN, authorization);
+ String nc = extract(NC_PATTERN, authorization);
+ Assert.assertEquals("00000001", nc);
+
+ // Calculate the expected response independently of the code under
+ // test, in the same way as a server would, per RFC 2617 section
+ // 3.2.2.1 and RFC 7616 section 3.4.2.
+ String a1 = userName + ":" + realm + ":" + password;
+ String a2 = method + ":" + requestUri;
+ if ("auth-int".equals(qop)) {
+ // RFC 7616: A2 includes H(entity-body). A WebSocket handshake
+ // does not have an entity body so use the hash of the empty
+ // string.
+ a2 = a2 + ":" + encode("");
+ }
+ String expected = encode(encode(a1) + ":" + nonce + ":" + nc + ":" + cnonce + ":" + qop + ":" + encode(a2));
+
+ Assert.assertEquals(expected, response);
+ }
+
+ private static String extract(Pattern pattern, String authorization) {
+ Matcher m = pattern.matcher(authorization);
+ Assert.assertTrue(m.find());
+ return m.group(1);
+ }
+
+ private static String encode(String value) throws Exception {
+ MessageDigest md = MessageDigest.getInstance("MD5");
+ byte[] digest = md.digest(value.getBytes(StandardCharsets.ISO_8859_1));
+ return HexUtils.toHexString(digest);
+ }
+}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 0c0764ad91..980eed0082 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -409,6 +409,12 @@
write operations so a stalled peer cannot block the write thread
indefinitely. (remm)
</fix>
+ <fix>
+ Correct the calculation of the digest <code>response</code> value by
+ the WebSocket client when the server uses <code>qop="auth-int"</code>.
+ The hash of the (empty) entity body was omitted, resulting in an
+ incorrect response that the server always rejected. (remm)
+ </fix>
</changelog>
</subsection>
<subsection name="Web applications">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 13/16: Reject @OnMessage methods with a primitive payload and boolean last flag
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 871f498805cf082ad78af00634a95edcfaa85b00
Author: opencode <re...@apache.org>
AuthorDate: Sat Oct 17 13:50:48 2026 +0200
Reject @OnMessage methods with a primitive payload and boolean last flag
Partial message delivery is only defined for String, byte[] and
ByteBuffer payloads, optionally with a boolean last flag. Tomcat
already rejects Reader, InputStream, PongMessage and decoded object
payloads combined with a boolean last flag at deployment time but did
not reject the same combination for a primitive (or boxed primitive)
payload parameter.
For a method like void onMessage(int payload, boolean last) the method
mapping was accepted, handler creation then fell through to the partial
ByteBuffer branch and constructed PojoMessageHandlerPartialBinary with
an index of -1. The first fragmented binary message caused
parameters[indexPayload] = message in
PojoMessageHandlerPartialBase.onMessage() to throw an
ArrayIndexOutOfBoundsException outside the try/catch, which WsFrameBase
handled as an unexpected error, invoking onError() and closing the
session abnormally (1006). Any client connecting to such an endpoint
could trigger this with a single fragmented binary message.
Reject the parameter combination at deployment time instead, matching
the existing partial parameter validation, with a new
pojoMethodMapping.partialPrimitive message. This also removes the last
route by which the partial message handler creation could be reached
without a valid payload index.
Add a regression test asserting the DeploymentException is raised for
this signature.
---
.../tomcat/websocket/pojo/LocalStrings.properties | 1 +
.../tomcat/websocket/pojo/PojoMethodMapping.java | 5 ++++
.../websocket/pojo/TestPojoMethodMapping.java | 27 ++++++++++++++++++++++
3 files changed, 33 insertions(+)
diff --git a/java/org/apache/tomcat/websocket/pojo/LocalStrings.properties b/java/org/apache/tomcat/websocket/pojo/LocalStrings.properties
index 9945db9ca7..165b4f2bad 100644
--- a/java/org/apache/tomcat/websocket/pojo/LocalStrings.properties
+++ b/java/org/apache/tomcat/websocket/pojo/LocalStrings.properties
@@ -46,6 +46,7 @@ pojoMethodMapping.paramWithoutAnnotation=A parameter of type [{0}] was found on
pojoMethodMapping.partialInputStream=Invalid InputStream and boolean parameters present on the method [{0}] of class [{1}] that was annotated with OnMessage
pojoMethodMapping.partialObject=Invalid Object and boolean parameters present on the method [{0}] of class [{1}] that was annotated with OnMessage
pojoMethodMapping.partialPong=Invalid PongMessage and boolean parameters present on the method [{0}] of class [{1}] that was annotated with OnMessage
+pojoMethodMapping.partialPrimitive=Invalid primitive type and boolean parameters present on the method [{0}] of class [{1}] that was annotated with OnMessage
pojoMethodMapping.partialReader=Invalid Reader and boolean parameters present on the method [{0}] of class [{1}] that was annotated with OnMessage
pojoMethodMapping.pongWithPayload=Invalid PongMessage and Message parameters present on the method [{0}] of class [{1}] that was annotated with OnMessage
diff --git a/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java b/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java
index 9f4defaed3..cb33282475 100644
--- a/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java
+++ b/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java
@@ -607,6 +607,11 @@ public class PojoMethodMapping {
throw new DeploymentException(sm.getString("pojoMethodMapping.partialInputStream", m.getName(),
m.getDeclaringClass().getName()));
}
+ if (indexPrimitive != -1 && indexBoolean != -1) {
+ throw new DeploymentException(
+ sm.getString("pojoMethodMapping.partialPrimitive", m.getName(),
+ m.getDeclaringClass().getName()));
+ }
if (decoderMatch != null && decoderMatch.hasMatches() && indexBoolean != -1) {
throw new DeploymentException(
sm.getString("pojoMethodMapping.partialObject", m.getName(), m.getDeclaringClass().getName()));
diff --git a/test/org/apache/tomcat/websocket/pojo/TestPojoMethodMapping.java b/test/org/apache/tomcat/websocket/pojo/TestPojoMethodMapping.java
index 623da81eb1..b188597d8b 100644
--- a/test/org/apache/tomcat/websocket/pojo/TestPojoMethodMapping.java
+++ b/test/org/apache/tomcat/websocket/pojo/TestPojoMethodMapping.java
@@ -21,6 +21,7 @@ import java.util.ArrayList;
import java.util.List;
import jakarta.websocket.ContainerProvider;
+import jakarta.websocket.DeploymentException;
import jakarta.websocket.OnClose;
import jakarta.websocket.OnMessage;
import jakarta.websocket.OnOpen;
@@ -96,6 +97,32 @@ public class TestPojoMethodMapping extends TomcatBaseTest {
}
+ /*
+ * Only String, byte[] and ByteBuffer payloads (plus the boolean last flag) are permitted for partial messages. A
+ * primitive payload combined with a boolean last flag must be rejected at deployment time rather than resulting in
+ * a partial message handler that fails when the first fragmented message arrives.
+ */
+ @Test
+ public void testPrimitiveAndBooleanParameters() throws Exception {
+ try {
+ new PojoMethodMapping(PrimitiveAndBooleanServer.class, new ArrayList<>(), "/primitiveAndBoolean", null);
+ Assert.fail("Expected a DeploymentException");
+ } catch (DeploymentException de) {
+ Assert.assertTrue("Unexpected message: " + de.getMessage(), de.getMessage().contains("primitive type"));
+ }
+ }
+
+
+ @ServerEndpoint("/primitiveAndBoolean")
+ public static final class PrimitiveAndBooleanServer {
+
+ @OnMessage
+ public void onMessage(@SuppressWarnings("unused") int payload, @SuppressWarnings("unused") boolean last) {
+ // No-Op
+ }
+ }
+
+
@ServerEndpoint(value = "/{one}/{two}/{three}", configurator = SingletonConfigurator.class)
public static final class Server {
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 14/16: Fix covariant return type overrides of annotated POJO lifecycle methods wrongly rejected as duplicates
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 433b97f09814a3c904bee44fbf06eece7da31813
Author: opencode <re...@apache.org>
AuthorDate: Sat Oct 17 14:01:43 2026 +0200
Fix covariant return type overrides of annotated POJO lifecycle methods wrongly rejected as duplicates
A sub-class method that overrides an annotated life cycle method of a
super class is permitted by the JLS to use a covariant return type.
isMethodOverride() in PojoMethodMapping compared return types using
equals() so such overrides were not recognised as overrides. Re-
declaring the annotation on the override therefore triggered a
deployment failure ("Duplicate annotations [...]") even though an
override with an identical return type re-declaring the annotation was
accepted.
isMethodOverride() now checks that the return types are compatible
rather than identical. The methods are passed to isMethodOverride() in
different orders by the various call sites so the check tests
assignability in both directions. Methods with the same name and
parameter types within a single class hierarchy are override
equivalent by definition, since the compiler enforces return type
covariance, so this cannot wrongly match methods that are not related
by overriding.
Adds regression tests covering an annotated covariant override that
re-declares the annotation (now accepted, the sub-class method is the
one mapped), a non-annotated covariant override (the super class
method is ignored) and a sub-class method that is not an override
(different name, same payload type) which continues to be rejected as
a duplicate annotation.
---
.../tomcat/websocket/pojo/PojoMethodMapping.java | 9 +-
.../websocket/pojo/TestPojoMethodMapping.java | 98 ++++++++++++++++++++++
2 files changed, 106 insertions(+), 1 deletion(-)
diff --git a/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java b/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java
index cb33282475..93176ec939 100644
--- a/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java
+++ b/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java
@@ -204,7 +204,14 @@ public class PojoMethodMapping {
private boolean isMethodOverride(Method method1, Method method2) {
- return method1.getName().equals(method2.getName()) && method1.getReturnType().equals(method2.getReturnType()) &&
+ // An overriding method may use a covariant return type so the return
+ // types do not have to be equal. They do need to be compatible. The
+ // methods may be passed in either order so both directions are
+ // checked.
+ Class<?> returnType1 = method1.getReturnType();
+ Class<?> returnType2 = method2.getReturnType();
+ return method1.getName().equals(method2.getName()) &&
+ (returnType1.isAssignableFrom(returnType2) || returnType2.isAssignableFrom(returnType1)) &&
Arrays.equals(method1.getParameterTypes(), method2.getParameterTypes());
}
diff --git a/test/org/apache/tomcat/websocket/pojo/TestPojoMethodMapping.java b/test/org/apache/tomcat/websocket/pojo/TestPojoMethodMapping.java
index b188597d8b..b164836f50 100644
--- a/test/org/apache/tomcat/websocket/pojo/TestPojoMethodMapping.java
+++ b/test/org/apache/tomcat/websocket/pojo/TestPojoMethodMapping.java
@@ -123,6 +123,104 @@ public class TestPojoMethodMapping extends TomcatBaseTest {
}
+ /*
+ * A sub-class method that overrides an annotated super class method using a covariant return
+ * type is still an override. Re-declaring the annotation on the override must not be treated
+ * as a duplicate annotation and the sub-class method must be the one that is mapped.
+ */
+ @Test
+ public void testCovariantReturnOverride() throws Exception {
+ PojoMethodMapping mapping = new PojoMethodMapping(CovariantReturnServer.class, new ArrayList<>(),
+ "/covariantReturn", null);
+ Assert.assertEquals(CovariantReturnServer.class.getMethod("onOpen", Session.class), mapping.getOnOpen());
+ }
+
+
+ /*
+ * A non-annotated sub-class method that overrides an annotated super class method using a
+ * covariant return type should result in the super class method being ignored. Note that a
+ * compiler generated bridge method previously masked this in compiled code.
+ */
+ @Test
+ public void testCovariantReturnOverrideWithoutAnnotation() throws Exception {
+ PojoMethodMapping mapping = new PojoMethodMapping(CovariantReturnOverrideServer.class, new ArrayList<>(),
+ "/covariantReturnOverride", null);
+ Assert.assertNull(mapping.getOnOpen());
+ }
+
+
+ /*
+ * A sub-class method that is not an override of the annotated super class method must still
+ * be rejected as a duplicate annotation if it handles the same message type.
+ */
+ @Test
+ public void testAnnotatedMethodNotAnOverride() throws Exception {
+ try {
+ new PojoMethodMapping(NotAnOverrideServer.class, new ArrayList<>(), "/notAnOverride", null);
+ Assert.fail("Expected a DeploymentException");
+ } catch (DeploymentException de) {
+ Assert.assertTrue("Unexpected message: " + de.getMessage(),
+ de.getMessage().contains("Duplicate annotations"));
+ }
+ }
+
+
+ public static class CovariantReturnBase {
+
+ @OnOpen
+ public Object onOpen(Session session) {
+ return session;
+ }
+
+ @OnMessage
+ public Object onMessage(String message) {
+ return message;
+ }
+ }
+
+
+ @ServerEndpoint("/covariantReturn")
+ public static final class CovariantReturnServer extends CovariantReturnBase {
+
+ @Override
+ @OnOpen
+ public String onOpen(Session session) {
+ return session.toString();
+ }
+
+ @Override
+ @OnMessage
+ public String onMessage(String message) {
+ return message;
+ }
+ }
+
+
+ @ServerEndpoint("/covariantReturnOverride")
+ public static final class CovariantReturnOverrideServer extends CovariantReturnBase {
+
+ @Override
+ public String onOpen(Session session) {
+ return session.toString();
+ }
+
+ @Override
+ public String onMessage(String message) {
+ return message;
+ }
+ }
+
+
+ @ServerEndpoint("/notAnOverride")
+ public static final class NotAnOverrideServer extends CovariantReturnBase {
+
+ @OnMessage
+ public String otherMessage(String message) {
+ return message;
+ }
+ }
+
+
@ServerEndpoint(value = "/{one}/{two}/{three}", configurator = SingletonConfigurator.class)
public static final class Server {
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org