Note: There are a few cases of duplicates in user autocompletion which are being worked on.

brotli decoder can cancel channel when not desired

RESOLVED FIXED in Firefox 44

Status

()

Core
Networking: HTTP
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mcmanus, Assigned: mcmanus)

Tracking

32 Branch
mozilla44
Points:
---

Firefox Tracking Flags

(firefox44 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

2 years ago
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.
(Assignee)

Comment 1

2 years ago
Created attachment 8669829 [details] [diff] [review]
cancel brotli failures from channel instead of converter
Attachment #8669829 - Flags: review?(daniel)
(Assignee)

Updated

2 years ago
Assignee: nobody → mcmanus
Status: NEW → ASSIGNED
(Assignee)

Comment 2

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6d0a6fc6bba0
(Assignee)

Comment 3

2 years ago
Created attachment 8670288 [details] [diff] [review]
cancel brotli failures from channel instead of converter
Attachment #8670288 - Flags: review?(daniel)
(Assignee)

Updated

2 years ago
Attachment #8669829 - Attachment is obsolete: true
Attachment #8669829 - Flags: review?(daniel)
(Assignee)

Comment 4

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8310bb3cc166 (for comment 3)
Attachment #8670288 - Flags: review?(daniel) → review+
(Assignee)

Comment 5

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd2d80b9b98584c652d61ae08dff290e00cd46f6
bug 1211580 - cancel brotli failures from channel instead of converter r=bagder
https://hg.mozilla.org/mozilla-central/rev/dd2d80b9b985
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.