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)
Core
Networking: HTTP
Tracking
()
RESOLVED
FIXED
mozilla34
People
(Reporter: ohtsu, Assigned: u408661)
Details
(Whiteboard: [spdy])
Attachments
(2 files)
1.19 KB,
text/x-patch
|
Details | |
6.67 KB,
patch
|
mcmanus
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Updated•10 years ago
|
Component: Untriaged → Networking: HTTP
OS: Windows 8.1 → All
Product: Firefox → Core
Hardware: x86_64 → All
Whiteboard: [spdy]
Comment 1•10 years ago
|
||
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
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 4•10 years ago
|
||
Comment on attachment 8474666 [details] [diff] [review]
patch
Review of attachment 8474666 [details] [diff] [review]:
-----------------------------------------------------------------
txs
Attachment #8474666 -
Flags: review?(mcmanus) → review+
Reporter | ||
Comment 6•10 years ago
|
||
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.
Comment 7•10 years ago
|
||
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.
Description
•