Closed Bug 1054173 Opened 10 years ago Closed 10 years ago

Push_Promise error when accept-encoding header is included

Categories

(Core :: Networking: HTTP, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: ohtsu, Assigned: u408661)

Details

(Whiteboard: [spdy])

Attachments

(2 files)

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0 (Beta/Release) Build ID: 20140716183446 Steps to reproduce: I made a server push test between Firefox and iij-http2 and found that it was failed because the pushed contents are not displayed at Firefox. In server push, iij-http2 encodes headers of pushed contents in Push_Promise by changing :path and the rest of headers are copied from the request headers sent by Firefox. The all headers encoded in Push Promise are Push Promise Header: [ { name: ':method', value: 'GET' }, { name: ':path', value: '/push/flower1.jpg' }, { name: ':authority', value: 'demo-int.iijplus.jp:8443' }, { name: ':scheme', value: 'https' }, { name: 'user-agent', value: 'Mozilla/5.0 (X11; Linux x86_64; rv:34.0) Gecko/20100101 Firefox/34.0' }, { name: 'accept', value: 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8' }, { name: 'accept-language', value: 'en-US,en;q=0.5' }, { name: 'accept-encoding', value: 'gzip, deflate' } ] Actual results: The following debug log of Firefox shows that the error is caused by "illegal response header found : accept-encoding". 2014-08-15 04:26:25.714000 UTC - 7032[280f530]: Http2Session::RecvPushPromise 1726dc00 ID 0x2 assoc ID 0x9 paddingLength 0 padded 0 2014-08-15 04:26:25.714000 UTC - 7032[280f530]: Http2Stream::Http2Stream 159c7010 2014-08-15 04:26:25.714000 UTC - 7032[280f530]: Http2PushedStream ctor this=159c7010 0x2 (snip) 2014-08-15 04:26:25.718000 UTC - 7032[280f530]: HTTP decompressor indexed entry 16 2014-08-15 04:26:25.718000 UTC - 7032[280f530]: HTTP Decompressor illegal response header found : accept-encoding 2014-08-15 04:26:25.718000 UTC - 7032[280f530]: Decompressor state after indexe (snip) 2014-08-15 04:26:25.719000 UTC - 7032[280f530]: Http2Stream::ConvertPushHeaders 159c7010 Error 2014-08-15 04:26:25.719000 UTC - 7032[280f530]: nsHttpConnection::CloseTransaction[this=15afe860 trans=1726dc00 reason=80070057] 2014-08-15 04:26:25.719000 UTC - 7032[280f530]: Http2Session::Close 1726dc00 80070057 Expected results: The accept-encoding header is allowed to encode in the request header by https://hg.mozilla.org/mozilla-central/rev/dc9960d759e4 so the server may have it in Push Promise. The attached patch is just removing accept-encoding from the excluded list in header output. After applying this, my test of server push works fine.
Component: Untriaged → Networking: HTTP
OS: Windows 8.1 → All
Product: Firefox → Core
Hardware: x86_64 → All
Whiteboard: [spdy]
I think the problem is really that the function is enforcing response header rules (because that is its the decompressor, but in this case it is being used for push_promise)
Yeah, we're seeing this same behavior with webtide.com now, too. We should really allow those headers in the case of a push (since they're *supposed* to be request headers in that case), and not hard-fail when it's not a push, but rather just not send them on to the HTTP/1.1 layer.
Assignee: nobody → hurley
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached patch patchSplinter Review
Here's the patch discussed on irc. Still can't talk to webtide.com for other reasons, but we get past this hurdle. https://tbpl.mozilla.org/?tree=Try&rev=2fb3264f58b7
Attachment #8474666 - Flags: review?(mcmanus)
Comment on attachment 8474666 [details] [diff] [review] patch Review of attachment 8474666 [details] [diff] [review]: ----------------------------------------------------------------- txs
Attachment #8474666 - Flags: review?(mcmanus) → review+
The patch works well with my push promise tests but I wonder if Firefox needs not check connection-specific headers such as transfer-encoding in Push_Promise when mIsPush is true.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: