This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new 732744b320 decline invalid permessage-deflate offers rather than fail
handshake
732744b320 is described below
commit 732744b3208956a665074b25c15384e2407b88be
Author: sahvx655-wq <[email protected]>
AuthorDate: Mon Jun 8 20:55:50 2026 +0530
decline invalid permessage-deflate offers rather than fail handshake
---
.../apache/tomcat/websocket/PerMessageDeflate.java | 141 +++++++++++----------
.../tomcat/websocket/TestPerMessageDeflate.java | 25 ++++
webapps/docs/changelog.xml | 6 +
3 files changed, 106 insertions(+), 66 deletions(-)
diff --git a/java/org/apache/tomcat/websocket/PerMessageDeflate.java
b/java/org/apache/tomcat/websocket/PerMessageDeflate.java
index c2889d9bc4..b3c94192a6 100644
--- a/java/org/apache/tomcat/websocket/PerMessageDeflate.java
+++ b/java/org/apache/tomcat/websocket/PerMessageDeflate.java
@@ -87,82 +87,91 @@ public class PerMessageDeflate implements Transformation {
boolean clientContextTakeover = true;
int clientMaxWindowBits = -1;
- for (Parameter param : preference) {
- if (SERVER_NO_CONTEXT_TAKEOVER.equals(param.getName())) {
- if (serverContextTakeover) {
- serverContextTakeover = false;
- } else {
- // Duplicate definition
- throw new IllegalArgumentException(
-
sm.getString("perMessageDeflate.duplicateParameter",
SERVER_NO_CONTEXT_TAKEOVER));
- }
- } else if (CLIENT_NO_CONTEXT_TAKEOVER.equals(param.getName()))
{
- if (clientContextTakeover) {
- clientContextTakeover = false;
- } else {
- // Duplicate definition
- throw new IllegalArgumentException(
-
sm.getString("perMessageDeflate.duplicateParameter",
CLIENT_NO_CONTEXT_TAKEOVER));
- }
- } else if (SERVER_MAX_WINDOW_BITS.equals(param.getName())) {
- if (serverMaxWindowBits == -1) {
- serverMaxWindowBits =
Integer.parseInt(param.getValue());
- if (serverMaxWindowBits < 8 || serverMaxWindowBits >
15) {
- throw new
IllegalArgumentException(sm.getString("perMessageDeflate.invalidWindowSize",
- SERVER_MAX_WINDOW_BITS,
Integer.valueOf(serverMaxWindowBits)));
- }
- // Java SE API (as of Java 11) does not expose the API
to
- // control the Window size. It is effectively
hard-coded
- // to 15
- if (isServer && serverMaxWindowBits != 15) {
- ok = false;
- break;
- // Note server window size is not an issue for the
- // client since the client will assume 15 and if
the
- // server uses a smaller window everything will
- // still work
+ try {
+ for (Parameter param : preference) {
+ if (SERVER_NO_CONTEXT_TAKEOVER.equals(param.getName())) {
+ if (serverContextTakeover) {
+ serverContextTakeover = false;
+ } else {
+ // Duplicate definition
+ throw new IllegalArgumentException(
+
sm.getString("perMessageDeflate.duplicateParameter",
SERVER_NO_CONTEXT_TAKEOVER));
}
- } else {
- // Duplicate definition
- throw new IllegalArgumentException(
-
sm.getString("perMessageDeflate.duplicateParameter", SERVER_MAX_WINDOW_BITS));
- }
- } else if (CLIENT_MAX_WINDOW_BITS.equals(param.getName())) {
- if (clientMaxWindowBits == -1) {
- if (param.getValue() == null) {
- // Hint to server that the client supports this
- // option. Java SE API (as of Java 11) does not
- // expose the API to control the Window size. It is
- // effectively hard-coded to 15
- clientMaxWindowBits = 15;
+ } else if
(CLIENT_NO_CONTEXT_TAKEOVER.equals(param.getName())) {
+ if (clientContextTakeover) {
+ clientContextTakeover = false;
} else {
- clientMaxWindowBits =
Integer.parseInt(param.getValue());
- if (clientMaxWindowBits < 8 || clientMaxWindowBits
> 15) {
+ // Duplicate definition
+ throw new IllegalArgumentException(
+
sm.getString("perMessageDeflate.duplicateParameter",
CLIENT_NO_CONTEXT_TAKEOVER));
+ }
+ } else if (SERVER_MAX_WINDOW_BITS.equals(param.getName()))
{
+ if (serverMaxWindowBits == -1) {
+ serverMaxWindowBits =
Integer.parseInt(param.getValue());
+ if (serverMaxWindowBits < 8 || serverMaxWindowBits
> 15) {
throw new
IllegalArgumentException(sm.getString("perMessageDeflate.invalidWindowSize",
- CLIENT_MAX_WINDOW_BITS,
Integer.valueOf(clientMaxWindowBits)));
+ SERVER_MAX_WINDOW_BITS,
Integer.valueOf(serverMaxWindowBits)));
}
+ // Java SE API (as of Java 11) does not expose the
API to
+ // control the Window size. It is effectively
hard-coded
+ // to 15
+ if (isServer && serverMaxWindowBits != 15) {
+ ok = false;
+ break;
+ // Note server window size is not an issue for
the
+ // client since the client will assume 15 and
if the
+ // server uses a smaller window everything will
+ // still work
+ }
+ } else {
+ // Duplicate definition
+ throw new IllegalArgumentException(
+
sm.getString("perMessageDeflate.duplicateParameter", SERVER_MAX_WINDOW_BITS));
}
- // Java SE API (as of Java 11) does not expose the API
to
- // control the Window size. It is effectively
hard-coded
- // to 15
- if (!isServer && clientMaxWindowBits != 15) {
- ok = false;
- break;
- // Note client window size is not an issue for the
- // server since the server will assume 15 and if
the
- // client uses a smaller window everything will
- // still work
+ } else if (CLIENT_MAX_WINDOW_BITS.equals(param.getName()))
{
+ if (clientMaxWindowBits == -1) {
+ if (param.getValue() == null) {
+ // Hint to server that the client supports this
+ // option. Java SE API (as of Java 11) does not
+ // expose the API to control the Window size.
It is
+ // effectively hard-coded to 15
+ clientMaxWindowBits = 15;
+ } else {
+ clientMaxWindowBits =
Integer.parseInt(param.getValue());
+ if (clientMaxWindowBits < 8 ||
clientMaxWindowBits > 15) {
+ throw new IllegalArgumentException(
+
sm.getString("perMessageDeflate.invalidWindowSize", CLIENT_MAX_WINDOW_BITS,
+
Integer.valueOf(clientMaxWindowBits)));
+ }
+ }
+ // Java SE API (as of Java 11) does not expose the
API to
+ // control the Window size. It is effectively
hard-coded
+ // to 15
+ if (!isServer && clientMaxWindowBits != 15) {
+ ok = false;
+ break;
+ // Note client window size is not an issue for
the
+ // server since the server will assume 15 and
if the
+ // client uses a smaller window everything will
+ // still work
+ }
+ } else {
+ // Duplicate definition
+ throw new IllegalArgumentException(
+
sm.getString("perMessageDeflate.duplicateParameter", CLIENT_MAX_WINDOW_BITS));
}
} else {
- // Duplicate definition
+ // Unknown parameter
throw new IllegalArgumentException(
-
sm.getString("perMessageDeflate.duplicateParameter", CLIENT_MAX_WINDOW_BITS));
+
sm.getString("perMessageDeflate.unknownParameter", param.getName()));
}
- } else {
- // Unknown parameter
- throw new IllegalArgumentException(
- sm.getString("perMessageDeflate.unknownParameter",
param.getName()));
}
+ } catch (IllegalArgumentException iae) {
+ // An invalid extension parameter has been offered. RFC 7692
+ // section 5.1 requires the offer to be declined and the
handshake to
+ // continue (without this extension) rather than failing. Try
the
+ // next offered configuration.
+ ok = false;
}
if (ok) {
return new PerMessageDeflate(serverContextTakeover,
serverMaxWindowBits, clientContextTakeover,
diff --git a/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
b/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
index e3e7df0bbd..45080ade46 100644
--- a/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
+++ b/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
@@ -150,6 +150,31 @@ public class TestPerMessageDeflate {
Assert.assertEquals(mp2, compressedParts.get(1));
}
+
+ /*
+ * RFC 7692 section 5.1 requires a permessage-deflate offer that contains
an invalid extension parameter to be
+ * declined so the handshake continues without compression. The offer must
not fail the handshake.
+ */
+ @Test
+ public void testInvalidParameterDeclinesOffer() {
+ // client_max_window_bits with an out-of-range value
+ assertDeclined("client_max_window_bits", "16");
+ // client_max_window_bits with a non-numeric value
+ assertDeclined("client_max_window_bits", "x");
+ // server_max_window_bits offered without a value
+ assertDeclined("server_max_window_bits", null);
+ }
+
+
+ private static void assertDeclined(String name, String value) {
+ List<Parameter> parameters = new ArrayList<>();
+ parameters.add(new WsExtensionParameter(name, value));
+ List<List<Parameter>> preferences = new ArrayList<>();
+ preferences.add(parameters);
+
+ Assert.assertNull(PerMessageDeflate.build(preferences, true));
+ }
+
/*
* Minimal implementation to enable other transformations to be tested. It
is NOT robust.
*/
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 6bf4ab09cb..8484192eb6 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -450,6 +450,12 @@
<code>Endpoint.onOpen()</code> fails for a programmatic endpoint. Test
case provided by uabdur. (markt)
</fix>
+ <fix>
+ If a client presents invalid parameters when negotiating a WebSocket
+ extension, decline the negotiation offer that includes the invalid
+ parameters rather than failing the connection. Pull request
+ <pr>1019</pr> provided by sahvx655-wq. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Web applications">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
