Bug 99282 – [WebSocket] WebSocketInflater should handle BFINAL = 1 blocks

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Kenichi Ishibashi]

We received a bug report from a chromium user (http://code.google.com/p/chromium/issues/detail?id=155060).

The draft specs of deflate-frame and permessage-compress extensions allow an endpoint compress messages as arbitrary number of blocks with BFINAL set to 0 and 1.
http://tools.ietf.org/html/draft-tyoshino-hybi-websocket-perframe-deflate-06#section-4.2.1
http://tools.ietf.org/html/draft-ietf-hybi-permessage-compression-01#section-5.2

WebSocketDeflateFramer doesn't accept blocks with BFINAL set to 1. It should accept such blocks.

[Kenichi Ishibashi]

Created attachment 168613 [details]
WIP patch

[Kenichi Ishibashi]

(In reply to comment #1)
> Created an attachment (id=168613) [details]
> WIP patch

This patch requires a new version of pywebsocket which should include https://codereview.appspot.com/6684050/

[Yuta Kitamura]

Comment on attachment 168613 [details]
WIP patch

View in context: https://bugs.webkit.org/attachment.cgi?id=168613&action=review

> Source/WebCore/Modules/websockets/WebSocketDeflater.cpp:172
> +            // Reveived a block with BFINAL set to 1. Reset decompression state.

Reveived -> Received, maybe?

[Kenichi Ishibashi]

Comment on attachment 168613 [details]
WIP patch

View in context: https://bugs.webkit.org/attachment.cgi?id=168613&action=review

>> Source/WebCore/Modules/websockets/WebSocketDeflater.cpp:172
>> +            // Reveived a block with BFINAL set to 1. Reset decompression state.
> 
> Reveived -> Received, maybe?

Yes. Thank you for the correction. Will fix.

[Kenichi Ishibashi]

Created attachment 168808 [details]
Patch

[Kenichi Ishibashi]

Hi Yuta-san,

pywebsocket has been updated. Could you take another look?

[Yuta Kitamura]

Comment on attachment 168808 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=168808&action=review

Sorry that I didn't catch this in the last review.

> Source/WebCore/Modules/websockets/WebSocketDeflater.cpp:181
> +    return consumedSoFar == length;

In what situation does this function return false? Is there any way to test that?

[Kenichi Ishibashi]

Comment on attachment 168808 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=168808&action=review

>> Source/WebCore/Modules/websockets/WebSocketDeflater.cpp:181
>> +    return consumedSoFar == length;
> 
> In what situation does this function return false? Is there any way to test that?

Sorry, that was a leavings of changes when I investigated the cause. I read zlib.h and confirmed that it shouldn't occur. I will restore it.

[Kenichi Ishibashi]

Created attachment 168828 [details]
Patch

[Yuta Kitamura]

Comment on attachment 168828 [details]
Patch

OK.

[Kenichi Ishibashi]

Comment on attachment 168828 [details]
Patch

Thanks!

[WebKit Review Bot]

Comment on attachment 168828 [details]
Patch

Clearing flags on attachment: 168828

Committed r131395: <http://trac.webkit.org/changeset/131395>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.