Last Comment Bug 780104 - Incorrect dictionary in SPDY response causes infinite loop
: Incorrect dictionary in SPDY response causes infinite loop
Status: RESOLVED FIXED
[spdy]
:
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: 15 Branch
: All All
: -- normal (vote)
: mozilla17
Assigned To: Patrick McManus [:mcmanus]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-03 03:14 PDT by HttpWatch
Modified: 2012-08-08 09:30 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch 0 (4.34 KB, patch)
2012-08-06 19:59 PDT, Patrick McManus [:mcmanus]
honzab.moz: review+
Details | Diff | Splinter Review

Description HttpWatch 2012-08-03 03:14:28 PDT
I returned a SPDY_CONTROL_TYPE_SYN_REPLY to Firefox with a compression dictionary that did not match the SPDY spec,

Firefox went into an infinite loop continually failing to set the correct dictionary.

The inifinite loop occurred in FF 15 beta occurred in SpdySession2::DownstreamUncompress, SpdySession3::UncompressAndDiscard and SpdyStream3::Uncompress

The same also applies to the SPDY implementations in FF 13 and FF14.

The problem is code like this:

  do {
    mDownstreamZlib.next_out = trash;
    mDownstreamZlib.avail_out = sizeof(trash);
    int zlib_rv = inflate(&mDownstreamZlib, Z_NO_FLUSH);

    if (zlib_rv == Z_NEED_DICT)
      inflateSetDictionary(&mDownstreamZlib, SpdyStream3::kDictionary,
                           sizeof(SpdyStream3::kDictionary));

    if (zlib_rv == Z_DATA_ERROR || zlib_rv == Z_MEM_ERROR)
      return NS_ERROR_FAILURE;
  }
  while (mDownstreamZlib.avail_in);

The call inflateSetDictionary fails with Z_NEED_DICT if SpdyStream3::kDictionary does not match the dictionary that was actually set in the stream.

The solution is to re-attempt the inflate and break out of the loop if the dictionary is still not correct:

  do {
    mDownstreamZlib.next_out = trash;
    mDownstreamZlib.avail_out = sizeof(trash);
    int zlib_rv = inflate(&mDownstreamZlib, Z_NO_FLUSH);

    if (zlib_rv == Z_NEED_DICT)
	{
      inflateSetDictionary(&mDownstreamZlib, SpdyStream3::kDictionary,
                           sizeof(SpdyStream3::kDictionary));

	  // Re-attemp the inflate with new dictionary to ensure correct
	  // dictionary was set in the stream
	  zlib_rv = inflate(&mDownstreamZlib, Z_NO_FLUSH);

	  // Return an error rather than going into an infinite loop
	  if ( zlib_rv == Z_NEED_DICT )
	    return NS_ERROR_FAILURE;
	}

    if (zlib_rv == Z_DATA_ERROR || zlib_rv == Z_MEM_ERROR)
      return NS_ERROR_FAILURE;
  }
  while (mDownstreamZlib.avail_in);
Comment 1 Patrick McManus [:mcmanus] 2012-08-03 08:28:35 PDT
thanks
Comment 2 Patrick McManus [:mcmanus] 2012-08-06 19:59:52 PDT
Created attachment 649526 [details] [diff] [review]
patch 0
Comment 3 Honza Bambas (:mayhemer) 2012-08-07 15:12:39 PDT
Comment on attachment 649526 [details] [diff] [review]
patch 0

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

r=honzab

Please just check added trailing white spaces in splinter before landing.
Comment 4 Patrick McManus [:mcmanus] 2012-08-07 18:59:50 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/d1ed1834ee75
Comment 5 Ed Morley [:emorley] 2012-08-08 09:30:05 PDT
https://hg.mozilla.org/mozilla-central/rev/d1ed1834ee75

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