On 08/09/17 13:23, Konstantin Kolinko wrote:
> 2017-09-08 12:06 GMT+03:00 <[email protected]>:
>> 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?
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}
permessage-deflate does not include the header information in the
payload since that is negotiated already.
> 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.
The first (non-fin) empty part is ignored.
The second (fin) empty part is sent uncompressed (since uncompressed
empty is smaller than compressed empty).
> 3. Javadoc for Deflater shows that end of input is signaled by calling
> finish();. I do not see such call here.
Correct. That would prevent the use of the compression context for the
next message.
Thanks for looking at this.
Mark
>
> 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: [email protected]
>> For additional commands, e-mail: [email protected]
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> For additional commands, e-mail: [email protected]
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
