This is an automated email from the ASF dual-hosted git repository.

markt-asf pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/main by this push:
     new 8e6b4fd436 decline invalid permessage-deflate offers rather than fail 
handshake
8e6b4fd436 is described below

commit 8e6b4fd4368dc337d78aff1c6beb12ed09d90db4
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 ++++
 2 files changed, 100 insertions(+), 66 deletions(-)

diff --git a/java/org/apache/tomcat/websocket/PerMessageDeflate.java 
b/java/org/apache/tomcat/websocket/PerMessageDeflate.java
index b6125f14f5..3789abb69f 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 36c7d18723..c74684b547 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.
      */


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to