1005821 - http2 GOAWAY frame sends ID field wrongly!

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Daniel Stenberg [:bagder]]

Attachment: suggested patch for the problem

Http2Session::GenerateGoAway() inserts the 'mOutgoingGoAwayID' at index 7 while the comment (and the current http2 spec draft-12) says it should be at index 8.

Comment 1

[u408661]

Comment on attachment 8417254 [details] [diff] [review]
suggested patch for the problem

Review of attachment 8417254 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/protocol/http/Http2Session.cpp
@@ +756,5 @@
>    CreateFrameHeader(packet, 8, FRAME_TYPE_GOAWAY, 0, 0);
>  
>    // last-good-stream-id are bytes 8-11 reflecting pushes
>    uint32_t goAway = PR_htonl(mOutgoingGoAwayID);
> +  memcpy (packet + 8, &goAway, 4);

While you're at it, get rid of the space before the open paren, too. r=hurley with that change.

Comment 2

[Daniel Stenberg [:bagder]]

Attachment: v2-0001-http2-store-streamID-on-correct-index-within-fram.patch

Fixed the space from the review

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Please provide a Try link. Suggestions for what to run if you haven't yet can be found here:
https://wiki.mozilla.org/Sheriffing/How:To:Recommended_Try_Practices

Comment 4

[u408661]

Daniel - just do an xpcshell try run to hit the http/2 unit tests. No other tests will be using http/2 at all, unless someone is doing something very weird we don't know about (hint - they're not. none of the protocol updates have broken anything.)

Comment 5

[Daniel Stenberg [:bagder]]

https://tbpl.mozilla.org/?tree=Try&rev=9783c267a41a

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d0f820f3f4fd

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d0f820f3f4fd