From 90bde82eedc0533110c2431222d623d50ad905b8 Mon Sep 17 00:00:00 2001 From: Robert Schlabbach Date: Fri, 19 Dec 2025 03:37:04 +0100 Subject: [PATCH 1/4] Fix Per-Message Compression Extension support The per-message compression extension was badly broken, leading to exceptions and loss of communication between client and server. Fix the extension negotiation to correctly send the current state of the client_no_context_takeover and server_no_context_takeover configuration variables, and ORing the received extensions with the received ones, to ensure client and server end up with the same configuration. Fix the application of these configuration variables, which must be swapped on the client vs. the server: The configuration of the deflater on the client side must affect the inflater on the server side, and vice versa. Remove the requestedParameters map as there is no need for it. This fixes issue #1496 --- .../PerMessageDeflateExtension.java | 52 ++++++++++++------- 1 file changed, 34 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/java_websocket/extensions/permessage_deflate/PerMessageDeflateExtension.java b/src/main/java/org/java_websocket/extensions/permessage_deflate/PerMessageDeflateExtension.java index 9eb16ca1..9956eb09 100644 --- a/src/main/java/org/java_websocket/extensions/permessage_deflate/PerMessageDeflateExtension.java +++ b/src/main/java/org/java_websocket/extensions/permessage_deflate/PerMessageDeflateExtension.java @@ -2,7 +2,6 @@ import java.io.ByteArrayOutputStream; import java.nio.ByteBuffer; -import java.util.LinkedHashMap; import java.util.Map; import java.util.zip.DataFormatException; import java.util.zip.Deflater; @@ -47,15 +46,14 @@ public class PerMessageDeflateExtension extends CompressionExtension { private boolean serverNoContextTakeover = true; private boolean clientNoContextTakeover = false; - // For WebSocketServers, this variable holds the extension parameters that the peer client has requested. - // For WebSocketClients, this variable holds the extension parameters that client himself has requested. - private Map requestedParameters = new LinkedHashMap<>(); - private final int compressionLevel; private final Inflater inflater; private final Deflater deflater; + private boolean resetDeflater = false; + private boolean resetInflater = false; + /** * Constructor for the PerMessage Deflate Extension (7. Thepermessage-deflate" Extension) * @@ -184,8 +182,8 @@ We can check the getRemaining() method to see whether the data we supplied has b if (inputFrame.isFin()) { decompress(TAIL_BYTES, output); - // If context takeover is disabled, inflater can be reset. - if (clientNoContextTakeover) { + // If context takeover is disabled for the other side, inflater must be reset. + if (resetInflater) { inflater.reset(); } } @@ -254,8 +252,8 @@ public void encodeFrame(Framedata inputFrame) { if (endsWithTail(outputBytes)) { outputLength -= TAIL_BYTES.length; } - - if (serverNoContextTakeover) { + // If context takeover is disabled for this side, deflater must be reset. + if (resetDeflater) { deflater.reset(); } } @@ -294,11 +292,19 @@ public boolean acceptProvidedExtensionAsServer(String inputExtension) { // Holds parameters that peer client has sent. Map headers = extensionData.getExtensionParameters(); - requestedParameters.putAll(headers); - if (requestedParameters.containsKey(CLIENT_NO_CONTEXT_TAKEOVER)) { + if (headers.containsKey(SERVER_NO_CONTEXT_TAKEOVER)) { + serverNoContextTakeover = true; + } + if (headers.containsKey(CLIENT_NO_CONTEXT_TAKEOVER)) { clientNoContextTakeover = true; } + // RFC 7692: + // server_ prefix parameters configure the server compressor (deflater) + // client_ prefix parameters configure the server decompressor (inflater) + resetDeflater = serverNoContextTakeover; + resetInflater = clientNoContextTakeover; + return true; } @@ -316,7 +322,19 @@ public boolean acceptProvidedExtensionAsClient(String inputExtension) { // Holds parameters that are sent by the server, as a response to our initial extension request. Map headers = extensionData.getExtensionParameters(); - // After this point, parameters that the server sent back can be configured, but we don't use them for now. + if (headers.containsKey(SERVER_NO_CONTEXT_TAKEOVER)) { + serverNoContextTakeover = true; + } + if (headers.containsKey(CLIENT_NO_CONTEXT_TAKEOVER)) { + clientNoContextTakeover = true; + } + + // RFC 7692: + // client_ prefix parameters configure the client compressor (deflater) + // server_ prefix parameters configure the client decompressor (inflater) + resetDeflater = clientNoContextTakeover; + resetInflater = serverNoContextTakeover; + return true; } @@ -325,17 +343,15 @@ public boolean acceptProvidedExtensionAsClient(String inputExtension) { @Override public String getProvidedExtensionAsClient() { - requestedParameters.put(CLIENT_NO_CONTEXT_TAKEOVER, ExtensionRequestData.EMPTY_VALUE); - requestedParameters.put(SERVER_NO_CONTEXT_TAKEOVER, ExtensionRequestData.EMPTY_VALUE); - - return EXTENSION_REGISTERED_NAME + "; " + SERVER_NO_CONTEXT_TAKEOVER + "; " - + CLIENT_NO_CONTEXT_TAKEOVER; + return EXTENSION_REGISTERED_NAME + + (serverNoContextTakeover ? "; " + SERVER_NO_CONTEXT_TAKEOVER : "") + + (clientNoContextTakeover ? "; " + CLIENT_NO_CONTEXT_TAKEOVER : ""); } @Override public String getProvidedExtensionAsServer() { return EXTENSION_REGISTERED_NAME - + "; " + SERVER_NO_CONTEXT_TAKEOVER + + (serverNoContextTakeover ? "; " + SERVER_NO_CONTEXT_TAKEOVER : "") + (clientNoContextTakeover ? "; " + CLIENT_NO_CONTEXT_TAKEOVER : ""); } From 8359b78ab41b64c5ed194a95974553308eaac641 Mon Sep 17 00:00:00 2001 From: Robert Schlabbach Date: Fri, 19 Dec 2025 04:07:35 +0100 Subject: [PATCH 2/4] Adapt tests to corrected extension negotiation --- .../extensions/PerMessageDeflateExtensionTest.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/java_websocket/extensions/PerMessageDeflateExtensionTest.java b/src/test/java/org/java_websocket/extensions/PerMessageDeflateExtensionTest.java index 5a86648f..f7e7da56 100644 --- a/src/test/java/org/java_websocket/extensions/PerMessageDeflateExtensionTest.java +++ b/src/test/java/org/java_websocket/extensions/PerMessageDeflateExtensionTest.java @@ -214,7 +214,7 @@ public void testIsFrameValid() { @Test public void testGetProvidedExtensionAsClient() { PerMessageDeflateExtension deflateExtension = new PerMessageDeflateExtension(); - assertEquals("permessage-deflate; server_no_context_takeover; client_no_context_takeover", + assertEquals("permessage-deflate; server_no_context_takeover", deflateExtension.getProvidedExtensionAsClient()); } @@ -242,6 +242,8 @@ public void testSetServerNoContextTakeover() { PerMessageDeflateExtension deflateExtension = new PerMessageDeflateExtension(); deflateExtension.setServerNoContextTakeover(false); assertFalse(deflateExtension.isServerNoContextTakeover()); + assertEquals("permessage-deflate", + deflateExtension.getProvidedExtensionAsServer()); } @Test @@ -255,6 +257,8 @@ public void testSetClientNoContextTakeover() { PerMessageDeflateExtension deflateExtension = new PerMessageDeflateExtension(); deflateExtension.setClientNoContextTakeover(true); assertTrue(deflateExtension.isClientNoContextTakeover()); + assertEquals("permessage-deflate; server_no_context_takeover; client_no_context_takeover", + deflateExtension.getProvidedExtensionAsClient()); } @Test From 1fd84a976d50fba89b056f1d5cc633c39d67534a Mon Sep 17 00:00:00 2001 From: Robert Schlabbach Date: Fri, 19 Dec 2025 19:00:04 +0100 Subject: [PATCH 3/4] Ensure compatibility with servers not supporting server_no_context_takeover --- .../permessage_deflate/PerMessageDeflateExtension.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/main/java/org/java_websocket/extensions/permessage_deflate/PerMessageDeflateExtension.java b/src/main/java/org/java_websocket/extensions/permessage_deflate/PerMessageDeflateExtension.java index 9956eb09..5c4481e2 100644 --- a/src/main/java/org/java_websocket/extensions/permessage_deflate/PerMessageDeflateExtension.java +++ b/src/main/java/org/java_websocket/extensions/permessage_deflate/PerMessageDeflateExtension.java @@ -324,6 +324,12 @@ public boolean acceptProvidedExtensionAsClient(String inputExtension) { Map headers = extensionData.getExtensionParameters(); if (headers.containsKey(SERVER_NO_CONTEXT_TAKEOVER)) { serverNoContextTakeover = true; + } else { + // If the server does not return server_no_context_takeover, the client must not reset the + // decompressor (inflater) because that would break communication. Note that in contrast, + // the client can reset the compressor (deflater) even if the server does not reset the + // decompressor (inflater), so this is not required for client_no_context_takeover below. + serverNoContextTakeover = false; } if (headers.containsKey(CLIENT_NO_CONTEXT_TAKEOVER)) { clientNoContextTakeover = true; From 4e61adf184c99c7bef3f89f87c0f4e4807e61c8b Mon Sep 17 00:00:00 2001 From: Robert Schlabbach Date: Fri, 19 Dec 2025 19:06:46 +0100 Subject: [PATCH 4/4] Make wording more precise Resetting the inflater/deflater is never a "must". Only _not_ resetting the inflater is a "must" if the deflater is not being reset. --- .../permessage_deflate/PerMessageDeflateExtension.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/java_websocket/extensions/permessage_deflate/PerMessageDeflateExtension.java b/src/main/java/org/java_websocket/extensions/permessage_deflate/PerMessageDeflateExtension.java index 5c4481e2..a04054cb 100644 --- a/src/main/java/org/java_websocket/extensions/permessage_deflate/PerMessageDeflateExtension.java +++ b/src/main/java/org/java_websocket/extensions/permessage_deflate/PerMessageDeflateExtension.java @@ -182,7 +182,7 @@ We can check the getRemaining() method to see whether the data we supplied has b if (inputFrame.isFin()) { decompress(TAIL_BYTES, output); - // If context takeover is disabled for the other side, inflater must be reset. + // If context takeover is disabled on the other end, inflater can be reset. if (resetInflater) { inflater.reset(); } @@ -252,7 +252,7 @@ public void encodeFrame(Framedata inputFrame) { if (endsWithTail(outputBytes)) { outputLength -= TAIL_BYTES.length; } - // If context takeover is disabled for this side, deflater must be reset. + // If context takeover is disabled on this end, deflater can be reset. if (resetDeflater) { deflater.reset(); }