Does not decode Brotli (content-encoding: br) on server-push (HTTP/2, H2) when 'br' is listed in 'Accept-Encoding'

RESOLVED FIXED in Firefox 55

Status

()

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

People

(Reporter: skhameneh, Unassigned, NeedInfo)

Tracking

({nightly-community})

50 Branch
mozilla55
nightly-community
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [necko-next][spdy])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
User Agent: Mozilla/5.0 (Linux; Android 6.0.1; E5823 Build/32.2.A.0.224) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/50.0.2661.89 Mobile Safari/537.36

Steps to reproduce:

Attempt a Server-Push with Brotli encoded content over HTTP/2.

Associated initial page request header:
Accept-Encoding: "gzip, deflate, br"

Associated PUSH_PROMISE response header:
content-encoding: "br" 
content-length: "58313"


Actual results:

Firefox receives expected content-length, size shows 0 B.
Push completes from server as expected, no errors shown in client.  Firefox is able to render Brotli compressed content that is requested by the client, as expected. 


Expected results:

Firefox should be able to inflate Brotli (br) content when served over a secure connection. 

Note: The push decodes successfully when gzip is used with the respective headers and content.  The functionality works as expected in another browser that supports Brotli.
(Reporter)

Comment 1

2 years ago
I filed this on my mobile device, the issue reproduces on Firefox 47.0 and Nightly 50.0a1 (2016-06-21)

Updated

2 years ago
Component: Untriaged → Networking
Keywords: nightly-community
Product: Firefox → Core
it would be really nice if you could include an endpoint (i.e. a url) that exhibits this...
Component: Networking → Networking: HTTP
(Reporter)

Comment 3

2 years ago
I'll make a repo on Github with some scripts, the endpoints I'm working with are proprietary.  Does that work for you?
If it repros easily with them that's fine. 

sometimes the root cause isn't as independent of the endpoint as we would all hope for :)
> Associated initial page request header:
> Accept-Encoding: "gzip, deflate, br"

I assume by initial page, you mean a client initiated odd stream?

> Associated PUSH_PROMISE response header:
> content-encoding: "br" 
> content-length: "58313"

and this is a server pushed response HEADERS frame? (i.e an even HEADERS?)

What is the even PUSH_PROMISE request header?

It would need to contain the Accept-Encoding header..
Whiteboard: [necko-next]
blocked on more info
Flags: needinfo?(skhameneh)
Whiteboard: [necko-next] → [necko-next][spdy]
Comment hidden (mozreview-request)
this report is WORKSFORME - but I'll use it to add the test coverage that I used to verify there isn't a problem. I'm guessing that the OP thought the accept-encoding needed to be on the associated resource, where it really needs to be on the PUSH_PROMISE request itself.

this worked in chrome because chrome has not historically enforced br being on the request, but they are changing that right now.

Comment 10

a year ago
mozreview-review
Comment on attachment 8855416 [details]
Bug 1281278 - test coverage for h2 push with brotli

https://reviewboard.mozilla.org/r/127260/#review130004

::: testing/xpcshell/moz-http2/moz-http2.js:355
(Diff revision 1)
> -      'content-length' : 1,
> +      'content-length' : 6,
>        'subresource' : '3',
> +      'content-encoding' : 'br',
>        'X-Connection-Http2': 'yes'
>      });
> -    push3.end('3');
> +    push3.end(Buffer.from("8B0080330A03", 'hex')); // '3\n'

I thought brotli was supposed to compress things! This is clearly longer! :-P
Attachment #8855416 - Flags: review?(hurley) → review+

Comment 11

a year ago
Pushed by mcmanus@ducksong.com:
https://hg.mozilla.org/integration/autoland/rev/f898176c408d
test coverage for h2 push with brotli r=nwgh
Flags: needinfo?(mcmanus)
node bailed on the server with
 8066 22:11:33 INFO - TypeError: hex is not a function 
[task 2017-04-06T22:11:33.943426Z] 22:11:33     INFO -  Process stderr
[task 2017-04-06T22:11:33.945331Z] 22:11:33     INFO -  /home/worker/workspace/build/tests/xpcshell/moz-http2/moz-http2.js:355
[task 2017-04-06T22:11:33.947250Z] 22:11:33     INFO -      push3.end(Buffer.from("8B0080330A03", 'hex')); // '3\n'
[task 2017-04-06T22:11:33.949068Z] 22:11:33     INFO -                       ^
[task 2017-04-06T22:11:33.950887Z] 22:11:33     INFO -  TypeError: hex is not a function
[task 2017-04-06T22:11:33.952761Z] 22:11:33     INFO -      at Function.from (native)
[task 2017-04-06T22:11:33.954569Z] 22:11:33     INFO -      at Function.from (native)
[task 2017-04-06T22:11:33.956551Z] 22:11:33     INFO -      at Server.handleRequest (/home/worker/workspace/build/tests/xpcshell/moz-http2/moz-http2.js:355:22)
[task 2017-04-06T22:11:33.958457Z] 22:11:33     INFO -      at emitTwo (events.js:87:13)
[task 2017-04-06T22:11:33.960306Z] 22:11:33     INFO -      at Server.emit (events.js:172:7)
[task 2017-04-06T22:11:33.962232Z] 22:11:33     INFO -      at IncomingRequest.g (events.js:260:16)
[task 2017-04-06T22:11:33.964126Z] 22:11:33     INFO -      at emitNone (events.js:67:13)
[task 2017-04-06T22:11:33.965993Z] 22:11:33     INFO -      at IncomingRequest.emit (events.js:166:7)
[task 2017-04-06T22:11:33.968029Z] 22:11:33     INFO -      at IncomingRequest._onHeaders (/home/worker/workspace/build/tests/xpcshell/node-http2/lib/http.js:688:8)
[task 2017-04-06T22:11:33.969960Z] 22:11:33     INFO -      at Stream.g (events.js:260:16)
[task 2017-04-06T22:11:33.971770Z] 22:11:33     INFO -  INFO | Result summary:
Comment hidden (mozreview-request)

Comment 16

a year ago
Pushed by mcmanus@ducksong.com:
https://hg.mozilla.org/integration/autoland/rev/ed3412f43074
test coverage for h2 push with brotli r=nwgh
https://hg.mozilla.org/mozilla-central/rev/ed3412f43074
Status: UNCONFIRMED → RESOLVED
Last Resolved: a year ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.