diff --git a/java/org/apache/tomcat/websocket/PerMessageDeflate.java b/java/org/apache/tomcat/websocket/PerMessageDeflate.java index b6125f14f553..3789abb69fe8 100644 --- a/java/org/apache/tomcat/websocket/PerMessageDeflate.java +++ b/java/org/apache/tomcat/websocket/PerMessageDeflate.java @@ -87,82 +87,91 @@ static PerMessageDeflate build(List> preferences, boolean isServ 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 36c7d1872375..c74684b54713 100644 --- a/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java +++ b/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java @@ -150,6 +150,31 @@ public void testFlushBatchMessagePart() throws IOException { 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 parameters = new ArrayList<>(); + parameters.add(new WsExtensionParameter(name, value)); + List> 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. */