1211580 - brotli decoder can cancel channel when not desired

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Patrick McManus [:mcmanus]]

The brotli decoder is not fully streamed - it has an internal ring buffer and in most cases when that is not full it will not produce output. This is a bit different than zib which will produce output anytime possible.

That means that the final bits of brotli decompressed content need to be triggered from the compressed onStopRequest which indicates to the brotli lib that the input is complete.

Furthermore if that call generates an error (because the buffered data is not valid brotli) the channel needs to have its status updated to the same error code that will be delivered to the stream listener's onstoprequest (that's a property the test suite checks for).

over in 366559 I did that by having nsHTTPCompressConv::OnStopRequest (which is called from the stack after httpchannel::onstoprequest) call channel->cancel before calling the streamlistener->onstoprequest.

So that seemed to go ok, but it turns out that there can be mutliple uses of the same channel with the decoder.

Specifically, the mime "sniffer" peeks at some of the channel data, and calls onstart/oda/onstop with the first block of it, all independently of the normal content consumer stream. it uses the same request channel however.. 

so in the case of some brotli documents that meant that the sniffer onstop received an error code (truncated invalid brotli because it only used the first block) but that also resulted in the channel being cancel'd, which meant the normal full content stream also received an error incorrectly.

Comment 1

[Patrick McManus [:mcmanus]]

Attachment: cancel brotli failures from channel instead of converter

Comment 2

[Patrick McManus [:mcmanus]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6d0a6fc6bba0

Comment 3

[Patrick McManus [:mcmanus]]

Attachment: cancel brotli failures from channel instead of converter

Comment 4

[Patrick McManus [:mcmanus]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8310bb3cc166 (for comment 3)

Comment 5

[Patrick McManus [:mcmanus]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dd2d80b9b98584c652d61ae08dff290e00cd46f6
bug 1211580 - cancel brotli failures from channel instead of converter r=bagder

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/dd2d80b9b985