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)

31 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: ohtsu, Assigned: ohtsu)

Details

(Whiteboard: [spdy])

Attachments

(1 file)

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;
 }
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 :)
Flags: needinfo?(ohtsu)
Flags: needinfo?(ohtsu)
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
https://hg.mozilla.org/mozilla-central/rev/b0e8399d6b27
Assignee: nobody → ohtsu
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
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.