This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/11.0.x by this push:
new 4fef25fe2a Fix per-message-deflate with non-final blocks that have
BFINAL set
4fef25fe2a is described below
commit 4fef25fe2ab7509e697af093280b9daadb515615
Author: Mark Thomas <[email protected]>
AuthorDate: Tue Sep 8 19:32:47 2026 +0100
Fix per-message-deflate with non-final blocks that have BFINAL set
---
.../apache/tomcat/websocket/PerMessageDeflate.java | 69 +++++-
.../tomcat/websocket/TestPerMessageDeflate.java | 253 +++++++++++++++++++--
webapps/docs/changelog.xml | 5 +
3 files changed, 308 insertions(+), 19 deletions(-)
diff --git a/java/org/apache/tomcat/websocket/PerMessageDeflate.java
b/java/org/apache/tomcat/websocket/PerMessageDeflate.java
index 10ffa1dbbc..502b5d0b91 100644
--- a/java/org/apache/tomcat/websocket/PerMessageDeflate.java
+++ b/java/org/apache/tomcat/websocket/PerMessageDeflate.java
@@ -76,6 +76,16 @@ public class PerMessageDeflate implements Transformation {
private volatile boolean skipDecompression = false;
private volatile boolean eomBytesInserted = false;
private volatile boolean eomOverflowWritten = false;
+ /*
+ * Offset and length, within readBuffer's backing array, of the compressed
bytes most recently passed to
+ * inflater.setInput(). Used to work out where the unconsumed tail starts
if inflater.finished() becomes true before
+ * all of those bytes have been consumed (see getMoreData()). Both fields
must be kept in sync with whatever the
+ * most recent setInput() call actually used - lastInputOffset is not
always readBuffer.arrayOffset(): after the
+ * first such recovery, the next input segment starts wherever the
previous one left off, not at the start of
+ * readBuffer's backing array.
+ */
+ private volatile int lastInputOffset;
+ private volatile int lastInputLength;
private volatile ByteBuffer writeBuffer =
ByteBuffer.allocate(Constants.DEFAULT_BUFFER_SIZE);
private volatile boolean firstCompressedFrameWritten = false;
// Flag to track if a message is completely empty
@@ -238,6 +248,8 @@ public class PerMessageDeflate implements Transformation {
if (inflater.needsInput() && !eomBytesInserted) {
readBuffer.clear();
TransformationResult nextResult = next.getMoreData(opCode,
fin, (rsv ^ RSV_BITMASK), readBuffer);
+ lastInputOffset = readBuffer.arrayOffset();
+ lastInputLength = readBuffer.position();
inflater.setInput(readBuffer.array(),
readBuffer.arrayOffset(), readBuffer.position());
if (dest.hasRemaining()) {
if (TransformationResult.UNDERFLOW.equals(nextResult)) {
@@ -267,7 +279,37 @@ public class PerMessageDeflate implements Transformation {
sm.getString("perMessageDeflate.next.ise",
next.getClass().getName()));
}
} else if (written == 0) {
- return endFrame(fin);
+ if (!eomBytesInserted && inflater.finished() &&
inflater.getRemaining() > 0) {
+ /*
+ * RFC 7692 section 7.2.1 permits an endpoint to compress
a single message using multiple DEFLATE
+ * blocks with any mix of BFINAL values, including a block
with BFINAL=1 that is not the last block
+ * of the message.
+ *
+ * If inflater is finished without EOM bytes being
inserted and with data still to process this
+ * indicates there is at least one more block to process.
Inflater has no API to continue once it
+ * has finished. From this point on, it silently ignores
any further input. The only way to process
+ * the remaining, still-unconsumed bytes belonging to this
same message is to reset() the Inflater
+ * (clearing the finished state) and feed it just the
unconsumed tail.
+ */
+ int remaining = inflater.getRemaining();
+ /*
+ * The unconsumed tail starts wherever the *current* input
segment started, not necessarily at
+ * readBuffer.arrayOffset(): if this is the second (or
later) recovery for the same message, the
+ * current segment already starts partway into readBuffer.
+ */
+ int newOffset = lastInputOffset + lastInputLength -
remaining;
+ try {
+ inflater.reset();
+ } catch (NullPointerException e) {
+ throw new
IOException(sm.getString("perMessageDeflate.alreadyClosed"), e);
+ }
+ inflater.setInput(readBuffer.array(), newOffset,
remaining);
+ lastInputOffset = newOffset;
+ lastInputLength = remaining;
+ // Continue decompression loop
+ } else {
+ return endFrame(fin);
+ }
}
}
@@ -313,11 +355,26 @@ public class PerMessageDeflate implements Transformation {
private TransformationResult endFrame(boolean fin) throws IOException {
eomBytesInserted = false;
eomOverflowWritten = false;
- if (fin && (isServer && !clientContextTakeover || !isServer &&
!serverContextTakeover)) {
- try {
- inflater.reset();
- } catch (NullPointerException e) {
- throw new
IOException(sm.getString("perMessageDeflate.alreadyClosed"), e);
+ if (fin) {
+ boolean contextTakeover = isServer ? clientContextTakeover :
serverContextTakeover;
+ /*
+ * If the message's final block was itself an independently
BFINAL=1 terminated block (see the recovery
+ * in getMoreData()), the EOM_BYTES appended to complete the
message per RFC 7692 section 7.2.2 were fed
+ * to an already-finished Inflater and were never consumed:
inflater.finished() stays true with those 4
+ * bytes still reported by getRemaining(). Left in that state, the
next message would compute its first
+ * recovery offset from this stale, unrelated leftover count,
which can go negative. There is no way to
+ * continue decompressing past a finished Inflater in place, so it
has to be reset here too - even though
+ * context takeover is enabled - at the cost of losing the window
for the *next* message in this,
+ * otherwise rare, case.
+ */
+ if (!contextTakeover || inflater.finished()) {
+ try {
+ inflater.reset();
+ } catch (NullPointerException e) {
+ throw new
IOException(sm.getString("perMessageDeflate.alreadyClosed"), e);
+ }
+ lastInputOffset = 0;
+ lastInputLength = 0;
}
}
return TransformationResult.END_OF_FRAME;
diff --git a/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
b/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
index 74a77d112e..93a7ede4a3 100644
--- a/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
+++ b/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
@@ -16,6 +16,7 @@
*/
package org.apache.tomcat.websocket;
+import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
@@ -23,6 +24,7 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
+import java.util.zip.Deflater;
import jakarta.websocket.Extension;
import jakarta.websocket.Extension.Parameter;
@@ -47,16 +49,14 @@ public class TestPerMessageDeflate {
perMessageDeflate.setNext(new TesterTransformation());
ByteBuffer bb1 = ByteBuffer.wrap("A".getBytes(StandardCharsets.UTF_8));
- MessagePart mp1 =
- new MessagePart(true, 0, Constants.OPCODE_TEXT, bb1, null,
null, false, Long.MAX_VALUE);
+ MessagePart mp1 = new MessagePart(true, 0, Constants.OPCODE_TEXT, bb1,
null, null, false, Long.MAX_VALUE);
List<MessagePart> uncompressedParts1 = new ArrayList<>();
uncompressedParts1.add(mp1);
perMessageDeflate.sendMessagePart(uncompressedParts1);
ByteBuffer bb2 = ByteBuffer.wrap("".getBytes(StandardCharsets.UTF_8));
- MessagePart mp2 =
- new MessagePart(true, 0, Constants.OPCODE_TEXT, bb2, null,
null, false, Long.MAX_VALUE);
+ MessagePart mp2 = new MessagePart(true, 0, Constants.OPCODE_TEXT, bb2,
null, null, false, Long.MAX_VALUE);
List<MessagePart> uncompressedParts2 = new ArrayList<>();
uncompressedParts2.add(mp2);
@@ -83,8 +83,7 @@ public class TestPerMessageDeflate {
ByteBuffer bb = ByteBuffer.wrap(data);
long writeTimeoutExpiry = System.currentTimeMillis() + 10000;
- MessagePart mp =
- new MessagePart(true, 0, Constants.OPCODE_BINARY, bb, null,
null, true, writeTimeoutExpiry);
+ MessagePart mp = new MessagePart(true, 0, Constants.OPCODE_BINARY, bb,
null, null, true, writeTimeoutExpiry);
List<MessagePart> uncompressedParts = new ArrayList<>();
uncompressedParts.add(mp);
@@ -125,8 +124,8 @@ public class TestPerMessageDeflate {
byte[] data = new byte[size];
Arrays.fill(data, (byte) 0x80);
List<MessagePart> uncompressedParts = new ArrayList<>();
- uncompressedParts.add(new MessagePart(true, 0,
Constants.OPCODE_BINARY,
- ByteBuffer.wrap(data), null, null, false, Long.MAX_VALUE));
+ uncompressedParts.add(new MessagePart(true, 0,
Constants.OPCODE_BINARY, ByteBuffer.wrap(data), null, null,
+ false, Long.MAX_VALUE));
MessagePart compressedPart =
perMessageDeflateTx.sendMessagePart(uncompressedParts).get(0);
// Decompress the way WsFrameBase.processDataBinary does: fill an
8192
@@ -172,18 +171,17 @@ public class TestPerMessageDeflate {
// First message part
byte[] data = new byte[1024];
ByteBuffer bb = ByteBuffer.wrap(data);
- MessagePart mp1 =
- new MessagePart(true, 0, Constants.OPCODE_BINARY, bb, null,
null, false, Long.MAX_VALUE);
+ MessagePart mp1 = new MessagePart(true, 0, Constants.OPCODE_BINARY,
bb, null, null, false, Long.MAX_VALUE);
uncompressedParts.add(mp1);
// Flush message (replicates result of calling flushBatch()
- MessagePart mp2 = new MessagePart(true, 0,
Constants.INTERNAL_OPCODE_FLUSH, null, null, null, false,
- Long.MAX_VALUE);
+ MessagePart mp2 =
+ new MessagePart(true, 0, Constants.INTERNAL_OPCODE_FLUSH,
null, null, null, false, Long.MAX_VALUE);
uncompressedParts.add(mp2);
List<MessagePart> compressedParts =
perMessageDeflateTx.sendMessagePart(uncompressedParts);
- Assert.assertEquals(2, compressedParts.size());
+ Assert.assertEquals(2, compressedParts.size());
// Check the first compressed part
MessagePart compressedPart1 = compressedParts.get(0);
@@ -205,6 +203,181 @@ public class TestPerMessageDeflate {
}
+ /*
+ * RFC 7692 section 7.2.1 explicitly permits an endpoint to compress a
single message using multiple DEFLATE blocks,
+ * of any type, with any mix of BFINAL values - including multiple,
independently terminated (BFINAL=1) DEFLATE
+ * blocks/streams concatenated together. This is not something
PerMessageDeflate's own compressor produces (it
+ * always uses a single, sync-flush terminated stream), so the payload has
to be constructed by hand to reproduce
+ * it. A compliant receiver must still recover the full, correctly
ordered, decompressed content of the message.
+ */
+ @Test
+ public void testMultipleFinalDeflateBlocksInOneMessage() throws
IOException {
+
+ byte[] part1 = "Hello, this is the FIRST part of the
message.".getBytes(StandardCharsets.UTF_8);
+ byte[] part2 = "And this is the SECOND, independently terminated
part.".getBytes(StandardCharsets.UTF_8);
+
+ // Two separate raw DEFLATE streams, each terminated with BFINAL=1,
concatenated together.
+ byte[] compressedPayload = concat(rawDeflateFinished(part1),
rawDeflateFinished(part2));
+
+ List<Parameter> parameters = Collections.emptyList();
+ List<List<Parameter>> preferences = new ArrayList<>();
+ preferences.add(parameters);
+
+ PerMessageDeflate perMessageDeflateRx =
PerMessageDeflate.build(preferences, true);
+ perMessageDeflateRx.setNext(new
TesterTransformation(ByteBuffer.wrap(compressedPayload)));
+
+ // RSV1 (the permessage-deflate compression bit) set, RSV2/RSV3 clear.
+ int rsv = 0b100;
+
+ ByteArrayOutputStream received = new ByteArrayOutputStream();
+ ByteBuffer buf = ByteBuffer.allocate(8192);
+ TransformationResult tr;
+ do {
+ buf.clear();
+ tr = perMessageDeflateRx.getMoreData(Constants.OPCODE_BINARY,
true, rsv, buf);
+ received.write(buf.array(), 0, buf.position());
+ } while (tr == TransformationResult.OVERFLOW);
+
+ Assert.assertEquals(TransformationResult.END_OF_FRAME, tr);
+
+ byte[] expected = concat(part1, part2);
+
+ Assert.assertArrayEquals("Expected the concatenation of both DEFLATE
blocks' decompressed content, got: " +
+ received.size() + " bytes: [" + received.toString("UTF-8") +
"]", expected, received.toByteArray());
+ }
+
+
+ /*
+ * As testMultipleFinalDeflateBlocksInOneMessage, but with three
independently terminated (BFINAL=1) DEFLATE blocks
+ * concatenated together. This requires two recoveries in getMoreData()
back to back, with no intervening
+ * readBuffer.clear()-based fetch between them - which is what exposes an
offset bug that a single-recovery
+ * (two-block) message cannot: computing the second recovery's offset from
readBuffer.arrayOffset() (fixed) rather
+ * than from where the first recovery's input segment actually started.
+ */
+ @Test
+ public void testThreeFinalDeflateBlocksInOneMessage() throws IOException {
+
+ byte[] part1 = "First part.".getBytes(StandardCharsets.UTF_8);
+ byte[] part2 = "Second, somewhat longer
part.".getBytes(StandardCharsets.UTF_8);
+ byte[] part3 = "Third and final
part.".getBytes(StandardCharsets.UTF_8);
+
+ byte[] compressedPayload =
+ concat(concat(rawDeflateFinished(part1),
rawDeflateFinished(part2)), rawDeflateFinished(part3));
+
+ List<Parameter> parameters = Collections.emptyList();
+ List<List<Parameter>> preferences = new ArrayList<>();
+ preferences.add(parameters);
+
+ PerMessageDeflate perMessageDeflateRx =
PerMessageDeflate.build(preferences, true);
+ perMessageDeflateRx.setNext(new
TesterTransformation(ByteBuffer.wrap(compressedPayload)));
+
+ int rsv = 0b100;
+
+ ByteArrayOutputStream received = new ByteArrayOutputStream();
+ ByteBuffer buf = ByteBuffer.allocate(8192);
+ TransformationResult tr;
+ do {
+ buf.clear();
+ tr = perMessageDeflateRx.getMoreData(Constants.OPCODE_BINARY,
true, rsv, buf);
+ received.write(buf.array(), 0, buf.position());
+ } while (tr == TransformationResult.OVERFLOW);
+
+ Assert.assertEquals(TransformationResult.END_OF_FRAME, tr);
+
+ byte[] expected = concat(concat(part1, part2), part3);
+
+ Assert.assertArrayEquals(
+ "Expected the concatenation of all three DEFLATE blocks'
decompressed content, " + "got: " +
+ received.size() + " bytes: [" +
received.toString("UTF-8") + "]",
+ expected, received.toByteArray());
+ }
+
+
+ /*
+ * A message whose real content is a single, exactly-fitting,
independently BFINAL=1 terminated block (nothing
+ * else) finishes cleanly via the normal needsInput()==true path and, as
part of that, has the RFC 7692 section
+ * 7.2.2 EOM_BYTES fed to an already-finished Inflater - which silently
ignores them, leaving finished()==true
+ * with those 4 bytes stuck in getRemaining(). With context takeover
enabled, endFrame() must not leave the
+ * Inflater in that state: otherwise the next message's recovery logic
computes its first offset from this
+ * stale, unrelated leftover count.
+ */
+ @Test
+ public void testMessageEndingInCleanBfinalBlockDoesNotPoisonNextMessage()
throws IOException {
+ byte[] message1 = "First message, a single clean BFINAL
block.".getBytes(StandardCharsets.UTF_8);
+ byte[] compressed1 = rawDeflateFinished(message1);
+
+ List<Parameter> parameters = Collections.emptyList();
+ List<List<Parameter>> preferences = new ArrayList<>();
+ preferences.add(parameters);
+
+ /*
+ * Context takeover is enabled by default (no *_no_context_takeover
parameter) - the same PerMessageDeflate
+ * instance, and therefore the same Inflater, must be reused across
messages, exactly as it would be for a
+ * real connection. setNext() on PerMessageDeflate delegates to the
existing next's setNext() once next is
+ * already set, so a single mutable source (rather than two separate
TesterTransformation instances) is used
+ * to supply both messages' bytes in turn.
+ */
+ PerMessageDeflate perMessageDeflateRx =
PerMessageDeflate.build(preferences, true);
+ MutableTesterTransformation source = new
MutableTesterTransformation(ByteBuffer.wrap(compressed1));
+ perMessageDeflateRx.setNext(source);
+ int rsv = 0b100;
+
+ ByteArrayOutputStream received1 = new ByteArrayOutputStream();
+ ByteBuffer buf = ByteBuffer.allocate(8192);
+ TransformationResult tr;
+ do {
+ buf.clear();
+ tr = perMessageDeflateRx.getMoreData(Constants.OPCODE_BINARY,
true, rsv, buf);
+ received1.write(buf.array(), 0, buf.position());
+ } while (tr == TransformationResult.OVERFLOW);
+ Assert.assertEquals(TransformationResult.END_OF_FRAME, tr);
+ Assert.assertArrayEquals(message1, received1.toByteArray());
+
+ // A completely separate, ordinary message on the same
connection/Transformation instance.
+ byte[] message2 = "Second message on the same
connection.".getBytes(StandardCharsets.UTF_8);
+ byte[] compressed2 = rawDeflateFinished(message2);
+ source.data = ByteBuffer.wrap(compressed2);
+ source.delivered = false;
+
+ ByteArrayOutputStream received2 = new ByteArrayOutputStream();
+ do {
+ buf.clear();
+ tr = perMessageDeflateRx.getMoreData(Constants.OPCODE_BINARY,
true, rsv, buf);
+ received2.write(buf.array(), 0, buf.position());
+ } while (tr == TransformationResult.OVERFLOW);
+ Assert.assertEquals(TransformationResult.END_OF_FRAME, tr);
+ Assert.assertArrayEquals("Second message must decompress correctly;
the first message's clean BFINAL "
+ + "ending must not poison the shared Inflater's state",
message2, received2.toByteArray());
+ }
+
+
+ private static byte[] rawDeflateFinished(byte[] data) {
+ @SuppressWarnings("resource") // False positive
+ Deflater deflater = new Deflater(Deflater.DEFAULT_COMPRESSION, true);
+ try {
+ deflater.setInput(data);
+ deflater.finish();
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ byte[] buf = new byte[4096];
+ while (!deflater.finished()) {
+ int n = deflater.deflate(buf);
+ baos.write(buf, 0, n);
+ }
+ return baos.toByteArray();
+ } finally {
+ deflater.end();
+ }
+ }
+
+
+ private static byte[] concat(byte[] a, byte[] b) {
+ byte[] result = new byte[a.length + b.length];
+ System.arraycopy(a, 0, result, 0, a.length);
+ System.arraycopy(b, 0, result, a.length, b.length);
+ return result;
+ }
+
+
/*
* 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.
@@ -281,4 +454,58 @@ public class TestPerMessageDeflate {
public void close() {
}
}
+
+
+ /*
+ * Like TesterTransformation, but the source ByteBuffer can be swapped out
between messages, to exercise reuse of
+ * a single PerMessageDeflate instance (and therefore its Inflater) across
multiple messages, as happens on a
+ * real connection with context takeover enabled.
+ */
+ private static class MutableTesterTransformation implements Transformation
{
+
+ ByteBuffer data;
+ boolean delivered;
+
+ MutableTesterTransformation(ByteBuffer data) {
+ this.data = data;
+ }
+
+ @Override
+ public boolean validateRsvBits(int i) {
+ return false;
+ }
+
+ @Override
+ public boolean validateRsv(int rsv, byte opCode) {
+ return false;
+ }
+
+ @Override
+ public void setNext(Transformation t) {
+ }
+
+ @Override
+ public List<MessagePart> sendMessagePart(List<MessagePart>
messageParts) {
+ return messageParts;
+ }
+
+ @Override
+ public TransformationResult getMoreData(byte opCode, boolean fin, int
rsv, ByteBuffer dest) {
+ if (delivered) {
+ return TransformationResult.END_OF_FRAME;
+ }
+ dest.put(data);
+ delivered = true;
+ return TransformationResult.END_OF_FRAME;
+ }
+
+ @Override
+ public Extension getExtensionResponse() {
+ return null;
+ }
+
+ @Override
+ public void close() {
+ }
+ }
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index f7cf4ceea7..fd28d64963 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -330,6 +330,11 @@
<code>{</code> and <code>}</code> characters from being mapped to
WebSocket end points. (markt)
</fix>
+ <fix>
+ Fix handling of WebSocket messages with compressed payloads using
+ per-message-deflate that have one or more non-final blocks where the
+ <code>BFINAL</code> bit is set. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Web applications">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
