780104 - Incorrect dictionary in SPDY response causes infinite loop

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[HttpWatch]

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]]

thanks

Comment 2

[Patrick McManus [:mcmanus]]

Attachment: patch 0

Comment 3

[Honza Bambas (:mayhemer)]

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]]

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

Comment 5

[Ed Morley [:emorley]]

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