2017-09-08 12:06 GMT+03:00  <ma...@apache.org>:
> Author: markt
> Date: Fri Sep  8 09:06:47 2017
> New Revision: 1807693
>
> URL: http://svn.apache.org/viewvc?rev=1807693&view=rev
> Log:
> Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=61491
> When using the permessage-deflate extension, correctly handle the sending of 
> empty messages after non-empty messages to avoid the IllegalArgumentException
>
> Added:
>     
> tomcat/tc7.0.x/trunk/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
>       - copied, changed from r1807689, 
> tomcat/tc8.5.x/trunk/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
> Modified:
>     tomcat/tc7.0.x/trunk/   (props changed)
>     
> tomcat/tc7.0.x/trunk/java/org/apache/tomcat/websocket/PerMessageDeflate.java
>     tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml


1. There are local variables for uncompressedPart.getPayload()  and
uncompressedPart.isFin()
several lines below in this method. They can be moved up and used
here. Though it does not matter much as those methods in MessagePart
class are simple getters.

> ByteBuffer uncompressedPayload = uncompressedPart.getPayload();

> boolean fin = uncompressedPart.isFin();


2.  If sendMessagePart() is called with an empty part and this is not
the last part,
does the call have to be delivered to client, like some king of flush
or ping? Or it can be ignored?

If it is not ignored, and compression of (empty part) is not empty
 (I may be wrong, but I expect it to contains some code to initialize
compression table, as gz archive of an empty file is not empty),
then I wonder whether the following works: sending two empty parts:
{empty part, empty part & fin=true}

>From the code I think that the output will be {compressed part},
followed by {passed-through empty "fin" part}? I think it will not be
received correctly.


3. Javadoc for Deflater shows that end of input is signaled by calling
finish();. I do not see such call here.

https://docs.oracle.com/javase/8/docs/api/java/util/zip/Deflater.html

Best regards,
Konstantin Kolinko


> Modified: 
> tomcat/tc7.0.x/trunk/java/org/apache/tomcat/websocket/PerMessageDeflate.java
> URL: 
> http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/tomcat/websocket/PerMessageDeflate.java?rev=1807693&r1=1807692&r2=1807693&view=diff
> ==============================================================================
> --- 
> tomcat/tc7.0.x/trunk/java/org/apache/tomcat/websocket/PerMessageDeflate.java 
> (original)
> +++ 
> tomcat/tc7.0.x/trunk/java/org/apache/tomcat/websocket/PerMessageDeflate.java 
> Fri Sep  8 09:06:47 2017
> @@ -315,16 +315,20 @@ public class PerMessageDeflate implement
>      public List<MessagePart> sendMessagePart(List<MessagePart> 
> uncompressedParts) {
>          List<MessagePart> allCompressedParts = new ArrayList<MessagePart>();
>
> +        // Flag to track if a message is completely empty
> +        boolean emptyMessage = true;
> +
>          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
>                  allCompressedParts.add(uncompressedPart);
> -            } else if (uncompressedPart.getPayload().limit() == 0 && 
> uncompressedPart.isFin() &&
> -                    deflater.getBytesRead() == 0) {
> -                // Zero length messages can't be compressed so pass them
> -                // straight through.
> +            } else if (emptyMessage && uncompressedPart.isFin()) {
> +                // Zero length messages can't be compressed so pass the
> +                // final (empty) part straight through.
>                  allCompressedParts.add(uncompressedPart);
>              } else {
>                  List<MessagePart> compressedParts = new 
> ArrayList<MessagePart>();
>
> Copied: 
> tomcat/tc7.0.x/trunk/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
>  (from r1807689, 
> tomcat/tc8.5.x/trunk/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java)
> URL: 
> http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java?p2=tomcat/tc7.0.x/trunk/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java&p1=tomcat/tc8.5.x/trunk/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java&r1=1807689&r2=1807693&rev=1807693&view=diff
> ==============================================================================
> --- 
> tomcat/tc8.5.x/trunk/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
>  (original)
> +++ 
> tomcat/tc7.0.x/trunk/test/org/apache/tomcat/websocket/TestPerMessageDeflate.java
>  Fri Sep  8 09:06:47 2017
> @@ -18,7 +18,6 @@ package org.apache.tomcat.websocket;
>
>  import java.io.IOException;
>  import java.nio.ByteBuffer;
> -import java.nio.charset.StandardCharsets;
>  import java.util.ArrayList;
>  import java.util.Collections;
>  import java.util.List;
> @@ -38,23 +37,23 @@ public class TestPerMessageDeflate {
>
>          // Set up the extension using defaults
>          List<Parameter> parameters = Collections.emptyList();
> -        List<List<Parameter>> preferences = new ArrayList<>();
> +        List<List<Parameter>> preferences = new ArrayList<List<Parameter>>();
>          preferences.add(parameters);
>
>          PerMessageDeflate perMessageDeflate = 
> PerMessageDeflate.negotiate(preferences, true);
>          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, -1);
> +        MessagePart mp1 = new MessagePart(true, 0, Constants.OPCODE_TEXT, 
> bb1, null, null);
>
> -        List<MessagePart> uncompressedParts1 = new ArrayList<>();
> +        List<MessagePart> uncompressedParts1 = new ArrayList<MessagePart>();
>          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, -1);
> +        MessagePart mp2 = new MessagePart(true, 0, Constants.OPCODE_TEXT, 
> bb2, null, null);
>
> -        List<MessagePart> uncompressedParts2 = new ArrayList<>();
> +        List<MessagePart> uncompressedParts2 = new ArrayList<MessagePart>();
>          uncompressedParts2.add(mp2);
>          perMessageDeflate.sendMessagePart(uncompressedParts2);
>      }
>
> Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
> URL: 
> http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1807693&r1=1807692&r2=1807693&view=diff
> ==============================================================================
> --- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
> +++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Fri Sep  8 09:06:47 2017
> @@ -100,6 +100,16 @@
>        </fix>
>      </changelog>
>    </subsection>
> +  <subsection name="WebSocket">
> +    <changelog>
> +      <fix>
> +        <bug>61491</bug>: When using the <code>permessage-deflate</code>
> +        extension, correctly handle the sending of empty messages after
> +        non-empty messages to avoid the 
> <code>IllegalArgumentException</code>.
> +        (markt)
> +      </fix>
> +    </changelog>
> +  </subsection>
>    <subsection name="Tribes">
>      <changelog>
>        <fix>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>

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

Reply via email to