New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 677001 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Possible BUG in "deflate" algorythm

Reported by egor.iva...@gmail.com, Dec 26 2016

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/55.0.2883.87 Safari/537.36

Example URL:

Steps to reproduce the problem:
  Unfortunately it's quite hard to provide a working test case. I can just try and explain what I have found.
  I had a resource loaded in several chunks of data. First chunk contained all headers with "Content-Encoding:deflate", other two were "deflate encoded" body with one weirdness: the first chunk contained just 1 byte, the second - the rest.
  After some research I found that encoded body did not contain zlib headers so GzipSourceStream in GzipSourceStream::FilterData was trying to do inflate only for one byte and instead of getting en error it succeeded. Hence it was unable to do inflate for a second chunk of data.
  I've made a small test with zlib and discovered that it wont return error for just one byte in input buffer so code in GzipSourceStream wont be able to detect zlib headers absence. The correct sequence of calls should be like: 1) add zlib headers; 2) inflate first chunk with 1only one byte; 3) inflate the rest.
  I also found this behavior in previous versions of Chrome which used GZipFilter.
  I hope it will help you.

What is the expected behavior?
Successfully loaded resource, like: js or css file.

What went wrong?
I got the error: net::ERR_CONTENT_DECODING_FAILED

Did this work before? No 

Chrome version: 57.0.2962.0 canary  Channel: stable
OS Version: 10.0
Flash Version: Shockwave Flash 24.0 r0
 
Cc: kkaluri@chromium.org
Labels: TE-NeedsTriageHelp
.push_deviceid
32 bytes Download
Ghi nhớ mới_11122016_174325.jpg
449 KB View Download

Comment 3 by mmenke@chromium.org, Dec 28 2016

Cc: rdsmith@chromium.org mmenke@chromium.org xunji...@chromium.org
Components: -Internals>Network Internals>Network>Filters
Labels: -OS-Windows OS-All
Status: Untriaged (was: Unconfirmed)
Note that GzipSourceStream is actually a new class, and isn't being used in Chrome 55, though it's possible both the old and completely refactored versions of the filter code had the same bug.  Could you see if Chrome Canary has the same issue?  You can safely install Chrome Canary alongside Chrome stable, and then uninstall it at your leisure, without losing settings on stable (That's not the case with other pre-release channels).

If Chrome Canary has the issue, it would be great if you could provide us a net-internals log with byte logging enabled (Instructions:  https://sites.google.com/a/chromium.org/dev/for-testers/providing-network-details - byte logging option is under the capture tab.  Note that the log will include cookies and credentials sent to/received from sites visited while net-internals is open.  If you use Canary, and don't log in to any sites with it, you shouldn't have anything to worry about).
Actually, I've checked this error in my custom Chrome 51 build, Chrome 55 and Canary. All of them had a similar error.
Sadly, I've found this thing using our client's private site so it could be impossible to provide whole net log due to security reasons.
Here are two files with some data from chrome://net-internals from events menu. it's URL_REQUEST data, one from a broken attempt the other from a good one. I hope it will help.
URL_REQUEST_jquery.blockUI.js(broken).txt
45.9 KB View Download
URL_REQUEST_jquery.blockUI.js.txt
154 KB View Download

Comment 5 by mmenke@chromium.org, Dec 29 2016

Cc: -mmenke@chromium.org
Owner: mmenke@chromium.org
Status: Assigned (was: Untriaged)
Thanks for that, should be all we need to try and repro.

Random comment:  Be careful of the use of "chunked" in reference to HTTP, I had assumed you meant chunked transfer-encoding, which, admittedly, shouldn't matter if this is indeed an issue at the URLRequestJob layer.

Comment 6 by mmenke@chromium.org, Dec 29 2016

Looks like I can repro.  The issue looks to only affect the path where we're getting a deflate response without a deflate header, so if you just attach a deflate header (Assumed to be 0x78 0x01), you'll avoid the issue.  Obviously we need to fix that (Though it would be nice if we could just force everyone to either send or not send a gzip header...), but it won't make it to stable for a couple months.

Comment 7 by mmenke@chromium.org, Dec 29 2016

Seems like deflate over HTTP is fundamentally flawed.  We try decompressing without adding a deflate header, which works if there's already one in the stream.  If that fails, we try deflating with adding a header.  The problem is if we've already consumed data when we get the error (Or worse, already passed data on upstream), we aren't correcting adding the deflate header.

The problem is that there doesn't seem to actually be any guarantee that the first bytes after the (Possibly not present) deflate header are not a valid deflate header, so however many bytes we require before continuing to process the response, there's no guarantee we have enough data.  So any "fix" involving buffering data is really only a partially functional hack, unless we buffer the entire partial response in memory holding it back from the renderer until it's all received, which we're obviously not going to do.

I'm going to arbitrarily buffer up to 100 bytes until we either get a decompression error, or start decoding bytes, but it doesn't seem like we can do better than best-effort here.  You're much better off either sending the deflate header, or using gzip.
  Wow, I haven't thought about "chunks" in that meaning, sorry for that. 
  Concerning this issue the bad thing is I'm not quite sure what leads to this strange fractured body. I think it could be my proxy server's fault. It happens only when using HTTPS over TLS 1.0 protocol. So for me I've find a temporary solution just to switch to TLS 1.2.
  Anyway, thanks for you help.

Comment 9 by mmenke@chromium.org, Dec 29 2016

Independent of that, you should be sending the 2-byte deflate header.  Per above comment, decompressing a response without that header is best-effort at best, and it's not really a good idea to have a production service depend on best-effort behavior.
  I don't have access to the client's server where the response was originally encrypted so I can't add headers needed. All that I can do is to figure out why only one first byte was sent and fix it. Since it seems that it's a rare case and this issue was in Chrome for years I think it should be ok for me right now just to avoid sending such a bad byte sequence. At least it's all that I can do.
Cc: mmenke@chromium.org
 Issue 649339  has been merged into this issue.
Project Member

Comment 12 by bugdroid1@chromium.org, Jan 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e66c81aea915be21bb56cb458f5efe8703aea78e

commit e66c81aea915be21bb56cb458f5efe8703aea78e
Author: mmenke <mmenke@chromium.org>
Date: Tue Jan 03 20:07:58 2017

Fix bug in deflate code.

For historical reasons, deflate-encoded streams may or may not have zlib
headers. In the case zlib headers appear to be missing, we add the
headers and then re-send the data we've received to zlib. If we
discovered this problem on a read other than the first zlib body read,
however, we would only re-send the result of the most recent read,
rather than the entire body, resulting in an error.

This CL adds some buffering of the body data until we're (somewhat)
confident the original stream had a zlib header. Since there's no
gaurantee that streams with and without zlib headers are actually
distinguishable from each other, this doesn't resolve all potential
issues, but should at least fix some cases.

BUG= 677001 

Review-Url: https://codereview.chromium.org/2604233002
Cr-Commit-Position: refs/heads/master@{#441197}

[modify] https://crrev.com/e66c81aea915be21bb56cb458f5efe8703aea78e/net/filter/gzip_source_stream.cc
[modify] https://crrev.com/e66c81aea915be21bb56cb458f5efe8703aea78e/net/filter/gzip_source_stream.h
[modify] https://crrev.com/e66c81aea915be21bb56cb458f5efe8703aea78e/net/filter/gzip_source_stream_unittest.cc
[modify] https://crrev.com/e66c81aea915be21bb56cb458f5efe8703aea78e/net/filter/mock_source_stream.cc
[modify] https://crrev.com/e66c81aea915be21bb56cb458f5efe8703aea78e/net/filter/mock_source_stream.h

Status: Fixed (was: Assigned)
Let's hope that sticks, but don't think we should spend too much time on making zlib-header-sniffing work better.

Sign in to add a comment