Closed
Bug 1051982
Opened 10 years ago
Closed 10 years ago
Header Table Size Update of HPACK is encoded with wrong integer representation
Categories
(Core :: Networking: HTTP, defect)
Tracking
()
RESOLVED
FIXED
mozilla34
People
(Reporter: ohtsu, Assigned: ohtsu)
Details
(Whiteboard: [spdy])
Attachments
(1 file)
985 bytes,
patch
|
u408661
:
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: When Firefox accessed to my HTTP/2 server and received SETTINGS_HEADER_TABLE_SIZE, Firefox sent a wrong encoded data of HPACK Header Table Size Update. Actual results: Here is a debug log of firefox when it received SETTINGS_HEADER_TABLE_SIZE of 4096 2014-08-11 08:19:28.118000 UTC - 9068[2b0f530]: Http2Session::RecvSettings 15498400 SETTINGS Control Frame with 3 entries ack=0 2014-08-11 08:19:28.118000 UTC - 9068[2b0f530]: Settings ID 1, Value 4096 2014-08-11 08:19:28.118000 UTC - 9068[2b0f530]: Compression header table setting received: 4096 2014-08-11 08:19:28.118000 UTC - 9068[2b0f530]: Http2Compressor::SetMaxBufferSizeInternal 4096 called Then the following debug log shows that Header Table Size Update is encoded to 2F F1. It seems to be represented as an integer with a 4-bit prefix. 2014-08-11 08:19:28.223000 UTC - 9068[2b0f530]: Http2Stream::TransmitFrame 16aaf530 inline=48 stream=0 2014-08-11 08:19:28.223000 UTC - 9068[2b0f530]: Http2Stream::TransmitFrame for inline BufferOutput session=15498400 stream=16aaf530 result 0 len=48 2014-08-11 08:19:28.223000 UTC - 9068[2b0f530]: Http2Session::LogIO 15498400 stream=16aaf530 id=0x5 [Writing from Inline Buffer] 2014-08-11 08:19:28.223000 UTC - 9068[2b0f530]: 00000000: 00 00 1A 01 25 00 00 00 05 00 00 00 00 15 2F F1 2014-08-11 08:19:28.223000 UTC - 9068[2b0f530]: 00000010: 1F 82 05 89 62 51 F7 31 0F 52 E6 21 FF C1 87 C0 2014-08-11 08:19:28.223000 UTC - 9068[2b0f530]: 00000020: BF BE 90 00 00 04 08 00 00 00 00 05 0F FE 00 00 Expected results: In 7.3 Header Table Size Update of draft-ietf-httpbis-header-compression-09, it is defined as 5-bit prefix. After applying the patch, firefox works fine with my HTTP/2 server. diff -r 70be728521e3 netwerk/protocol/http/Http2Compression.cpp --- a/netwerk/protocol/http/Http2Compression.cpp Sat Aug 09 17:00:59 2014 -0400 +++ b/netwerk/protocol/http/Http2Compression.cpp Tue Aug 12 01:06:06 2014 +0900 @@ -1194,7 +1194,7 @@ Http2Compressor::EncodeTableSizeChange(uint32_t newMaxSize) { uint32_t offset = mOutput->Length(); - EncodeInteger(4, newMaxSize); + EncodeInteger(5, newMaxSize); uint8_t *startByte = reinterpret_cast<uint8_t *>(mOutput->BeginWriting()) + offset; *startByte = *startByte | 0x20; }
Updated•10 years ago
|
Component: Untriaged → Networking: HTTP
Product: Firefox → Core
Whiteboard: [spdy]
Man... I could've sworn I made that change. Guess I just got the decode side of things, and not so much the encode side. Nice catch! r=me. Would it be possible to get you to upload a patch in the appropriate mercurial format (https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F has instructions) for us to land? If not, I'll just make up a patch for you with your email as the committer and come up with a good commit message on my own :)
Assignee | ||
Comment 2•10 years ago
|
||
Flags: needinfo?(ohtsu)
Assignee | ||
Comment 3•10 years ago
|
||
Here attached is the patch as instructed. If it has something wrong, please fix by yourself.
Comment on attachment 8471239 [details] [diff] [review] fixHeaderTableSizeUpdate.patch Review of attachment 8471239 [details] [diff] [review]: ----------------------------------------------------------------- Great! I'll fix up the commit message to match what our hook requires, and we'll get this landed. Thanks for the patch!
Attachment #8471239 -
Flags: review+
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 8.1 → All
Hardware: x86_64 → All
Comment 6•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b0e8399d6b27
Assignee: nobody → ohtsu
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Assignee | ||
Comment 7•10 years ago
|
||
I've just confirmed that the issue is resolved in Firefox Nightly 34.0a1 (2014-08-14). Thanks Nick for applying my patch.
You need to log in
before you can comment on or make changes to this bug.
Description
•