You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2019/11/12 09:32:42 UTC

[tomcat] branch 7.0.x updated: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63913 Wrap NPEs

This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/7.0.x by this push:
     new 32a0405  Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63913 Wrap NPEs
32a0405 is described below

commit 32a04051988982698f6d55f929545fb2689423b9
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Nov 12 09:11:05 2019 +0000

    Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63913 Wrap NPEs
    
    Wrap any NullPointerExceptions throw by the Inflater or Deflater used by
    the PerMessageDeflate extension in an IOException so that the error can
    be caught and handled by the WebSocket error handling mechanism.
---
 .../tomcat/websocket/LocalStrings.properties       |  1 +
 .../apache/tomcat/websocket/PerMessageDeflate.java | 37 ++++++++++++++++------
 .../apache/tomcat/websocket/Transformation.java    |  5 ++-
 .../tomcat/websocket/WsRemoteEndpointImplBase.java |  7 +++-
 .../tomcat/websocket/TestPerMessageDeflate.java    |  2 +-
 webapps/docs/changelog.xml                         | 11 +++++++
 6 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/java/org/apache/tomcat/websocket/LocalStrings.properties b/java/org/apache/tomcat/websocket/LocalStrings.properties
index 8507c1a..273920d 100644
--- a/java/org/apache/tomcat/websocket/LocalStrings.properties
+++ b/java/org/apache/tomcat/websocket/LocalStrings.properties
@@ -32,6 +32,7 @@ caseInsensitiveKeyMap.nullKey=Null keys are not permitted
 
 futureToSendHandler.timeout=Operation timed out after waiting [{0}] [{1}] to complete
 
+perMessageDeflate.alreadyClosed=The transformer has been closed and may no longer be used
 perMessageDeflate.deflateFailed=Failed to decompress a compressed WebSocket frame
 perMessageDeflate.duplicateParameter=Duplicate definition of the [{0}] extension parameter
 perMessageDeflate.invalidWindowSize=An invalid windows of [{1}] size was specified for [{0}]. Valid values are whole numbers from 8 to 15 inclusive.
diff --git a/java/org/apache/tomcat/websocket/PerMessageDeflate.java b/java/org/apache/tomcat/websocket/PerMessageDeflate.java
index c4fe1c4..a6f241d 100644
--- a/java/org/apache/tomcat/websocket/PerMessageDeflate.java
+++ b/java/org/apache/tomcat/websocket/PerMessageDeflate.java
@@ -204,6 +204,8 @@ public class PerMessageDeflate implements Transformation {
                         dest.array(), dest.arrayOffset() + dest.position(), dest.remaining());
             } catch (DataFormatException e) {
                 throw new IOException(sm.getString("perMessageDeflate.deflateFailed"), e);
+            } catch (NullPointerException e) {
+                throw new IOException(sm.getString("perMessageDeflate.alreadyClosed"), e);
             }
             dest.position(dest.position() + written);
 
@@ -229,7 +231,11 @@ public class PerMessageDeflate implements Transformation {
             } else if (written == 0) {
                 if (fin && (isServer && !clientContextTakeover ||
                         !isServer && !serverContextTakeover)) {
-                    inflater.reset();
+                    try {
+                        inflater.reset();
+                    } catch (NullPointerException e) {
+                        throw new IOException(sm.getString("perMessageDeflate.alreadyClosed"), e);
+                    }
                 }
                 return TransformationResult.END_OF_FRAME;
             }
@@ -314,7 +320,7 @@ public class PerMessageDeflate implements Transformation {
 
 
     @Override
-    public List<MessagePart> sendMessagePart(List<MessagePart> uncompressedParts) {
+    public List<MessagePart> sendMessagePart(List<MessagePart> uncompressedParts) throws IOException {
         List<MessagePart> allCompressedParts = new ArrayList<MessagePart>();
 
         for (MessagePart uncompressedPart : uncompressedParts) {
@@ -345,10 +351,14 @@ public class PerMessageDeflate implements Transformation {
                 while (deflateRequired) {
                     ByteBuffer compressedPayload = writeBuffer;
 
-                    int written = deflater.deflate(compressedPayload.array(),
-                            compressedPayload.arrayOffset() + compressedPayload.position(),
-                            compressedPayload.remaining(), flush);
-                    compressedPayload.position(compressedPayload.position() + written);
+                    try {
+                        int written = deflater.deflate(compressedPayload.array(),
+                                compressedPayload.arrayOffset() + compressedPayload.position(),
+                                compressedPayload.remaining(), flush);
+                        compressedPayload.position(compressedPayload.position() + written);
+                    } catch (NullPointerException e) {
+                        throw new IOException(sm.getString("perMessageDeflate.alreadyClosed"), e);
+                    }
 
                     if (!uncompressedPart.isFin() && compressedPayload.hasRemaining() && deflater.needsInput()) {
                         // This message part has been fully processed by the
@@ -400,7 +410,12 @@ public class PerMessageDeflate implements Transformation {
                         // - in middle of EOM bytes
                         // - about to write EOM bytes
                         // - more data to write
-                        int eomBufferWritten = deflater.deflate(EOM_BUFFER, 0, EOM_BUFFER.length, Deflater.SYNC_FLUSH);
+                        int eomBufferWritten;
+                        try {
+                            eomBufferWritten = deflater.deflate(EOM_BUFFER, 0, EOM_BUFFER.length, Deflater.SYNC_FLUSH);
+                        } catch (NullPointerException e) {
+                            throw new IOException(sm.getString("perMessageDeflate.alreadyClosed"), e);
+                        }
                         if (eomBufferWritten < EOM_BUFFER.length) {
                             // EOM has just been completed
                             compressedPayload.limit(compressedPayload.limit() - EOM_BYTES.length + eomBufferWritten);
@@ -444,11 +459,15 @@ public class PerMessageDeflate implements Transformation {
     }
 
 
-    private void startNewMessage() {
+    private void startNewMessage() throws IOException {
         firstCompressedFrameWritten = false;
         emptyMessage = true;
         if (isServer && !serverContextTakeover || !isServer && !clientContextTakeover) {
-            deflater.reset();
+            try {
+                deflater.reset();
+            } catch (NullPointerException e) {
+                throw new IOException(sm.getString("perMessageDeflate.alreadyClosed"), e);
+            }
         }
     }
 
diff --git a/java/org/apache/tomcat/websocket/Transformation.java b/java/org/apache/tomcat/websocket/Transformation.java
index 5186181..14d8093 100644
--- a/java/org/apache/tomcat/websocket/Transformation.java
+++ b/java/org/apache/tomcat/websocket/Transformation.java
@@ -101,8 +101,11 @@ public interface Transformation {
      * @return  The list of messages after this any any subsequent
      *          transformations have been applied. The size of the returned list
      *          may be bigger or smaller than the size of the input list
+     *
+     * @throws IOException If an error occurs during the transformation of the
+     *                     message parts
      */
-    List<MessagePart> sendMessagePart(List<MessagePart> messageParts);
+    List<MessagePart> sendMessagePart(List<MessagePart> messageParts) throws IOException;
 
     /**
      * Clean-up any resources that were used by the transformation.
diff --git a/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java b/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java
index 427f68c..ef4505d 100644
--- a/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java
+++ b/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java
@@ -300,7 +300,12 @@ public abstract class WsRemoteEndpointImplBase implements RemoteEndpoint {
                 intermediateMessageHandler,
                 new EndMessageHandler(this, handler)));
 
-        messageParts = transformation.sendMessagePart(messageParts);
+        try {
+            messageParts = transformation.sendMessagePart(messageParts);
+        } catch (IOException ioe) {
+            handler.onResult(new SendResult(ioe));
+            return;
+        }
 
         // Some extensions/transformations may buffer messages so it is possible
         // that no message parts will be returned. If this is the case the
diff --git a/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java b/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
index 9e30827..ac5fdc7 100644
--- a/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
+++ b/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
@@ -41,7 +41,7 @@ public class TestPerMessageDeflate {
      * https://bz.apache.org/bugzilla/show_bug.cgi?id=61491
      */
     @Test
-    public void testSendEmptyMessagePartWithContextTakeover() {
+    public void testSendEmptyMessagePartWithContextTakeover() throws IOException {
 
         // Set up the extension using defaults
         List<Parameter> parameters = Collections.emptyList();
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 3c1af27..200d248 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -130,6 +130,17 @@
       </fix>
     </changelog>
   </subsection>
+  <subsection name="WebSocket">
+    <changelog>
+      <fix>
+        <bug>63913</bug>: Wrap any <code>NullPointerException</code>s throw by
+        the <code>Inflater</code> or <code>Deflater</code> used by the
+        <code>PerMessageDeflate</code> extension in an <code>IOException</code>
+        so that the error can be caught and handled by the WebSocket error
+        handling mechanism. (markt)
+      </fix>
+    </changelog>
+  </subsection>
   <subsection name="Web applications">
     <changelog>
       <fix>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org