Fix BZ 66681 - NPE on batch flush when permessage-deflate enabled

https://bz.apache.org/bugzilla/show_bug.cgi?id=66681
apache · Jul 24, 2023 · a2615d1 · a2615d1
1 parent 728dc8d
commit a2615d1
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 4 deletions.
diff --git a/java/org/apache/tomcat/websocket/PerMessageDeflate.java b/java/org/apache/tomcat/websocket/PerMessageDeflate.java
@@ -314,13 +314,16 @@ public List<MessagePart> sendMessagePart(List<MessagePart> uncompressedParts) th
 
         for (MessagePart uncompressedPart : uncompressedParts) {
             byte opCode = uncompressedPart.getOpCode();
-            boolean emptyPart = uncompressedPart.getPayload().limit() == 0;
-            emptyMessage = emptyMessage && emptyPart;
             if (Util.isControl(opCode)) {
                 // Control messages can appear in the middle of other messages
-                // and must not be compressed. Pass it straight through
+                // and must not be compressed. Pass it straight through.
                 allCompressedParts.add(uncompressedPart);
-            } else if (emptyMessage && uncompressedPart.isFin()) {
+                continue;
+            }
+
+            boolean emptyPart = uncompressedPart.getPayload().limit() == 0;
+            emptyMessage = emptyMessage && emptyPart;
+            if (emptyMessage && uncompressedPart.isFin()) {
                 // Zero length messages can't be compressed so pass the
                 // final (empty) part straight through.
                 allCompressedParts.add(uncompressedPart);

diff --git a/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java b/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
@@ -101,6 +101,55 @@ public void testMessagePartThatFillsBufffer() throws IOException {
     }
 
 
+    /*
+     * https://bz.apache.org/bugzilla/show_bug.cgi?id=66681
+     */
+    @Test
+    public void testFlushBatchMessagePart() throws IOException {
+        // Set up the extension using defaults
+        List<Parameter> parameters = Collections.emptyList();
+        List<List<Parameter>> preferences = new ArrayList<>();
+        preferences.add(parameters);
+
+        // Set up the compression and sending of the message.
+        PerMessageDeflate perMessageDeflateTx = PerMessageDeflate.negotiate(preferences, true);
+        perMessageDeflateTx.setNext(new TesterTransformation());
+
+        List<MessagePart> uncompressedParts = new ArrayList<>();
+
+        // 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, -1);
+        uncompressedParts.add(mp1);
+
+        // Flush message (replicates result of calling flushBatch()
+        MessagePart mp2 = new MessagePart(true, 0, Constants.INTERNAL_OPCODE_FLUSH, null, null, null, -1);
+        uncompressedParts.add(mp2);
+
+        List<MessagePart> compressedParts = perMessageDeflateTx.sendMessagePart(uncompressedParts);
+
+        Assert.assertEquals(2,  compressedParts.size());
+
+        // Check the first compressed part
+        MessagePart compressedPart1 = compressedParts.get(0);
+
+        // Set up the decompression and process the received message
+        PerMessageDeflate perMessageDeflateRx = PerMessageDeflate.negotiate(preferences, true);
+        perMessageDeflateRx.setNext(new TesterTransformation(compressedPart1.getPayload()));
+
+        ByteBuffer received = ByteBuffer.allocate(8192);
+
+        TransformationResult tr = perMessageDeflateRx.getMoreData(compressedPart1.getOpCode(), compressedPart1.isFin(),
+                compressedPart1.getRsv(), received);
+
+        Assert.assertEquals(1024, received.position());
+        Assert.assertEquals(TransformationResult.END_OF_FRAME, tr);
+
+        // Check the second compressed part (should be passed through unchanged)
+        Assert.assertEquals(mp2, compressedParts.get(1));
+    }
+
     /*
      * Minimal implementation to enable other transformations to be tested. It is NOT robust.
      */

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
@@ -149,6 +149,15 @@
       </fix>
     </changelog>
   </subsection>
+  <subsection name="WebSocket">
+    <changelog>
+      <fix>
+        <bug>66681</bug>: Fix a <code>NullPointerException</code> when flushing
+        batched messages with compression enabled using
+        <code>permessage-deflate</code>. (markt)
+      </fix>
+    </changelog>
+  </subsection>
 </section>
 <section name="Tomcat 11.0.0-M9 (markt)" rtext="2023-07-10">
   <subsection name="Other">