Last Comment Bug 718210 - spdy hdr decompress context screwed up by server push
: spdy hdr decompress context screwed up by server push
Status: RESOLVED FIXED
[qa?]
:
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: mozilla12
Assigned To: Patrick McManus [:mcmanus]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-14 11:12 PST by Patrick McManus [:mcmanus]
Modified: 2012-03-29 15:17 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
affected
fixed


Attachments
patch 0 (3.21 KB, patch)
2012-01-14 11:15 PST, Patrick McManus [:mcmanus]
honzab.moz: review+
asa: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Patrick McManus [:mcmanus] 2012-01-14 11:12:07 PST
node-spdy is the first backend to implement server push. right now we rst any pushed streams, but we need to decompress the headers in those push frames so that the decompress context stays in sync with the sender. confirmed that interop continues with the patch attached to this bug.
Comment 1 Patrick McManus [:mcmanus] 2012-01-14 11:15:17 PST
Created attachment 588668 [details] [diff] [review]
patch 0

Hi honza, this is an interop issue so I'd like to expedite it to whatever extent possible.. might even nom for aurora.
Comment 2 Fedor Indutny 2012-01-14 11:18:34 PST
Btw, supporting push streams would be cool feature too :)
Comment 3 Patrick McManus [:mcmanus] 2012-01-14 11:22:39 PST
(In reply to Fedor Indutny from comment #2)
> Btw, supporting push streams would be cool feature too :)

yes - just a few things to sort out to make sure there isn't room for cache poisoning attacks and so on..
Comment 4 Honza Bambas (:mayhemer) 2012-01-14 12:10:44 PST
Comment on attachment 588668 [details] [diff] [review]
patch 0

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

Ups.. I didn't catch this even during the review (almost done by this time, btw).

r=honzab

For channel drivers: this is patch for a bug in a feature that is by default pref'ed off but people out there are testing it by manually pref'ing it on (including my self).  So it would be helpful and harmless to land it on aurora as well.

::: netwerk/protocol/http/SpdySession.cpp
@@ +776,5 @@
>  {
>    NS_ABORT_IF_FALSE(self->mFrameControlType == CONTROL_TYPE_SYN_STREAM,
>                      "wrong control type");
>    
> +  if (self->mFrameDataSize < 18) {

Hmm.. a bug in the spec apparently.
Comment 5 Patrick McManus [:mcmanus] 2012-01-14 20:03:00 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/59cb54c6dfe1
Comment 6 Jonathan Kew (:jfkthame) 2012-01-16 04:47:36 PST
https://hg.mozilla.org/mozilla-central/rev/59cb54c6dfe1
Comment 7 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-03-29 15:17:10 PDT
Is this something QA can verify?

Note You need to log in before you can comment on or make changes to this bug.