Closed Bug 1281278 Opened 8 years ago Closed 7 years ago

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

Categories

(Core :: Networking: HTTP, defect)

Product:
Core
Component:
Networking: HTTP
Version:
50 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: skhameneh, Unassigned, NeedInfo)

Details

(Keywords: nightly-community, Whiteboard: [necko-next][spdy])

Attachments

(1 file)

Bug 1281278 - test coverage for h2 push with brotli
59 bytes, text/x-review-board-request
u408661
: review+
Details 
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.
I filed this on my mobile device, the issue reproduces on Firefox 47.0 and Nightly 50.0a1 (2016-06-21)
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
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]
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 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+
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:
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
Closed: 7 years 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.

Attachment

General

Creator:
Created:
Updated:
Size: