Project

General

Profile

Actions

Bug #7034

closed

RequestParser incorrectly processes some ws13 compressed fragments as uncompressed data

Added by Bruce Toll almost 5 years ago. Updated over 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
Start date:
05/03/2019
Due date:
% Done:

0%

Estimated time:

Description

With github wt 4.0.5-31-ga05a7c5d, applications with websockets enabled (using the default per-message compression) can fail for clients that send multi-fragment compressed ws13 messages where not all message fragments are flagged as compressed. In particular, this issue has been observed with Chromium 72 and a test application that generates a large client request (170K+).

The attached test application can be used to demonstrate this issue. Test notes:

1. The test program was built with WT_DEBUG_JS and DEBUG logging enabled (also CMAKE_BUILD_TYPE of Debug).

  1. The test program was tested against two browsers under Linux: Chromium 72 and Firefox 65.
  2. The intended output of the test is a 5x5 grid of WTableViews positioned at row 400.
  3. A wt_config.xml is attached. It is needed to enable web-sockets and also to increase max-request-size to 256.
    NOTE: Without the larger max-request-size, the application fails without logging a problem (with default logging).

Observations, starting from the above:

  1. With websockets disabled, both Firefox and Chromium work correctly.
  2. With websockets enabled and compression disabled, both browsers work (run server with ---no-compression ---max-memory-request-size=256000)
  3. With websockets enabled, Firefox works correctly and Chromium fails (stays Loading).
  4. Reducing the number of rows or columns will result in both browsers working properly with default websockets (compressed).

The issue appears to be due to Chromium only setting the per-message compress flag on the first fragment of a message (consistent with RFC 7692), while the RequestParser seems to assume that the compression flag will be present on each fragment. The mismatch results in compressed data being handled as if it were uncompressed.

The attached patch includes more details on the problem, as well as a proposed fix for your review (only lightly tested).

It may also be worth considering ways to split up these large client-side requests to avoid hitting server limits. I will submit a second issue report with that suggestion so that it can be tracked separately.


Files

test_websocket_decompression_20190503a.C (2.06 KB) test_websocket_decompression_20190503a.C Bruce Toll, 05/03/2019 03:34 PM
wt_config.xml (420 Bytes) wt_config.xml Bruce Toll, 05/03/2019 03:34 PM
0001-Only-check-first-fragment-for-ws13-decompression.patch (1.76 KB) 0001-Only-check-first-fragment-for-ws13-decompression.patch Bruce Toll, 05/03/2019 03:34 PM
issue_7034_2.cpp (1.26 KB) issue_7034_2.cpp Roel Standaert, 07/09/2019 05:44 PM
Actions #1

Updated by Bruce Toll almost 5 years ago

As a follow-up, I noticed an additional issue when testing under valgrind. I don't think it is specific to valgrind, but rather may be related to a protocol interaction from the extended processing time. It appears that under some circumstances, the last payload in a frame may get dropped in a multi-frame message (observed with chromium). With web-socket compression disabled, the gap is visible in the debug-level output of queryString in CgiParser.C. With compression enabled, it seems to cause corrupt output in the queryString.

This problem should probably be tracked as a separate issue. However, I do not yet understand it well enough to file a good issue report. I'm adding a note here because it may be encountered in the course of reviewing this issue.

Actions #2

Updated by Bruce Toll almost 5 years ago

There is a follow-up with additional information on the related issue in #7039.

Actions #3

Updated by Roel Standaert almost 5 years ago

This should be resolved with my latest push. I also made another test case that only tests this issue and #7039, and doesn't also involve WTableView. (max-request-size needs to be fairly large, though, and Firefox's frames are rejected, because they're too big. Not sure if this means we should reconsider our hard coded frame length limit).

Actions #4

Updated by Roel Standaert over 4 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF