Bug 65317 - PerMessageDeflate getMoreData doesn't return correct TransformationResult if the payload is inflated to exact 8192 bytes
Summary: PerMessageDeflate getMoreData doesn't return correct TransformationResult if ...
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 9
Classification: Unclassified
Component: WebSocket (show other bugs)
Version: 9.0.31
Hardware: PC All
: P2 normal (vote)
Target Milestone: -----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-05-19 15:08 UTC by Saksham Verma
Modified: 2021-05-24 15:15 UTC (History)
1 user (show)



Attachments
A probable fix for the issue. Needs confirmation though. (22.90 KB, text/x-csrc)
2021-05-19 15:08 UTC, Saksham Verma
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Saksham Verma 2021-05-19 15:08:55 UTC
Created attachment 37868 [details]
A probable fix for the issue. Needs confirmation though.

If the compression is enabled and inflated size is exactly 8192, then tomcat closes the websocket connection with Close code 1009 below message:

No async message support and buffer too small. Buffer size: [8,192], Message size: [1,202]] 


Root cause:

PerMessageDeflate.getMoreData() finds that there's no space in the buffer but doesn't check if at that time, the complete message is written or not. 

And thus, instead of returning the END_OF_FRAME it returns OVERFLOW.
It causes to close the connection with a message that doesn't make much sense (buffer size 8192 is small for message size 1202).

With the attached file containing the changes, I was able to get around the problem and my connection was stable, but not sure if this is the most elegant way to do it as there's already some complicated logic to find the TransformationResut.
Comment 1 Mark Thomas 2021-05-20 16:15:28 UTC
Do you have a test case that reproduces this?
Comment 2 Mark Thomas 2021-05-20 16:41:21 UTC
No need for a test case. I've been able to build a unit test that demonstrates this.
Comment 3 Mark Thomas 2021-05-20 18:31:20 UTC
Test case now passes with the proposed patch applied.

There are a lot of edge cases here so I want to run the Autobahn test suite before committing this.
Comment 4 Mark Thomas 2021-05-21 10:45:59 UTC
All looks good. Thanks for the report and the patch.

Fixed in:
- 10.0.x for 10.0.7 onwards
- 9.0.x for 9.0.47 onwards
- 8.5.x for 8.5.67 onwards
Comment 5 Saksham Verma 2021-05-24 15:15:27 UTC
Thanks a lot for taking quick action on this.