Fix BrotliEncoder bug that does not mark ByteBuf it encodes as read (#13497)

vietj · normanmaurer · web-flow · commit 61d75db27778 · 2023-07-19T16:02:09.000+02:00
Motivation:
 
The contract between HttpContentCompressor and compression encoders assumes that encoders will mark the encoded ByteBuf as read after they have been encoded, otherwise the HttpContentCompressor will send encoded chunk twice (one time decoded and one time encoded). The BrotliEncoder peeks the ByteBuf nio buffer and pass it to the BrotliChannelEncoder without marking the ByteBuf and does not respect the implicit contract with HttpContentCompressor.

Modifications:

The BrotliEncoder now will set skip bytes of encoded ByteBuf, in addition the test generic AbstractEncoderTest has been modified to ensure that the readable bytes of a ByteBuf is equals to 0 after it has been encoded.

Result:

This fixes the bug.

---------

Co-authored-by: Norman Maurer &lt;norman_maurer@apple.com&gt;
diff --git a/codec/src/main/java/io/netty/handler/codec/compression/BrotliEncoder.java b/codec/src/main/java/io/netty/handler/codec/compression/BrotliEncoder.java
@@ -205,7 +205,10 @@ private void encode(ByteBuf msg, boolean preferDirect) throws Exception {
                 //
                 // A race condition will not arise because one flush call to encoder will result
                 // in only 1 call at `write(ByteBuffer)`.
-                brotliEncoderChannel.write(msg.nioBuffer());
+                ByteBuffer nioBuffer = msg.nioBuffer();
+                int position = nioBuffer.position();
+                brotliEncoderChannel.write(nioBuffer);
+                msg.skipBytes(nioBuffer.position() - position);
                 brotliEncoderChannel.flush();
             } catch (Exception e) {
                 ReferenceCountUtil.release(msg);
diff --git a/codec/src/test/java/io/netty/handler/codec/compression/AbstractEncoderTest.java b/codec/src/test/java/io/netty/handler/codec/compression/AbstractEncoderTest.java
@@ -87,6 +87,7 @@ protected void testCompression(final ByteBuf data) throws Exception {
         final int dataLength = data.readableBytes();
         assertTrue(channel.writeOutbound(data.retain()));
         assertTrue(channel.finish());
+        assertEquals(0, data.readableBytes());
 
         ByteBuf decompressed = readDecompressed(dataLength);
         assertEquals(data.resetReaderIndex(), decompressed);
@@ -101,12 +102,14 @@ protected void testCompressionOfBatchedFlow(final ByteBuf data) throws Exception
         while (written + length < dataLength) {
             ByteBuf in = data.retainedSlice(written, length);
             assertTrue(channel.writeOutbound(in));
+            assertEquals(0, in.readableBytes());
             written += length;
             length = rand.nextInt(100);
         }
         ByteBuf in = data.retainedSlice(written, dataLength - written);
         assertTrue(channel.writeOutbound(in));
         assertTrue(channel.finish());
+        assertEquals(0, in.readableBytes());
 
         ByteBuf decompressed = readDecompressed(dataLength);
         assertEquals(data, decompressed);
