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

Issue 621295 link

Starred by 4 users

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

Downloading files bigger than 4GB over QUIC fails

Reported by tmora...@akamai.com, Jun 18 2016

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.84 Safari/537.36

Example URL:

Steps to reproduce the problem:
1. Attempt to download a file that's bigger than 4GB using QUIC.

/Applications/Google\ Chrome\ Canary.app/Contents/MacOS/Google\ Chrome\ Canary --enable-logging --log-level=0 --v=2 --incognito --log-net-log=netlog.json --quic-version=QUIC_VERSION_31 --origin-to-force-quic-on=www.example.org:443 https://www.example.org/5GB.bin

What is the expected behavior?
Download the file. With smaller files (tested with 1MB) it behaves as expected.

What went wrong?
The download fails. The server receives RST_STREAM with QUIC_BAD_APPLICATION_PAYLOAD error.

Did this work before? N/A 

Chrome version: 53.0.2771.0   Channel: canary
OS Version: OS X 10.11.5
Flash Version: Shockwave Flash 22,0,0,192

It's reproducible using quic_client tool, built in Debug mode to get detailed logs. Logs attached. In quic_client.log we can see 
[0618/121221:VERBOSE1:quic_packet_creator.cc(527)] Adding frame: type { RST_STREAM_FRAME } { stream_id: 5, error_code: 3 }

From a quick examination of quic_client source code it looks like:
1) QuicSpdyClientStream::OnInitialHeadersComplete calls SpdyUtils::ParseHeaders. If that returns false, onInitialHeadersComplete resets the stream with QUIC_BAD_APPLICATION_PAYLOAD.

2) SpdyUtils::ParseHeaders tries to convert content-length value to int (the plain old C int). This apparently fails for values bigger than 32-bit MAXINT. Hence the failure in OnInitialHeadersComplete and QUIC_BAD_APPLICATION_PAYLOAD.
 
netlog.json
505 KB View Download
quic_client.log
48.2 KB View Download

Comment 1 by ianswett@google.com, Jun 18 2016

Owner: ianswett@chromium.org
Components: -Internals>Network Internals>Network>QUIC
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 20 2016

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

commit a387ce79f9344d421c2f03caf39869bd7f4f7b2a
Author: ianswett <ianswett@chromium.org>
Date: Mon Jun 20 17:35:18 2016

Fix a bug in QUIC's SpdyUtils where Content-Length was being parsed as an int, not an int64, causing >4GB downloads to fail.  Not flag protected.

Merge internal change: 125208956
Fixes crbug/621295

R=rch@chromium.org
BUG= 621295 

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

[modify] https://crrev.com/a387ce79f9344d421c2f03caf39869bd7f4f7b2a/net/quic/spdy_utils.cc
[modify] https://crrev.com/a387ce79f9344d421c2f03caf39869bd7f4f7b2a/net/quic/spdy_utils_test.cc

Comment 4 by ianswett@google.com, Jun 21 2016

The newest version of Chrome Canary should have the patch to fix this bug.  Can you try verifying with your example script?

I tried verifying with Google drive and discovered they do not include Content-Length, so it works correctly with and without the fix.

Comment 5 by tmora...@akamai.com, Jun 22 2016

The current version of Canary is 53.0.2774.3 and this still has the same problem. Should we wait for a newer version?

Netlog attached and it does contain this line, which I assume shows this problem:

{"params":{"quic_rst_stream_error":3,"stream_id":5},"phase":0,"source":{"id":28,"type":6},"time":"53047761","type":226},

netlog.json
168 KB View Download

Comment 6 by ianswett@google.com, Jun 22 2016

My mistake, I fixed one spot in SpdyUtils that parses content-length, but not the other(ValidateAndCopyHeaders).

I'll land a fix for that as well, and tell you when Canary has that fix.
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 22 2016

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

commit 90c79f0d55ef287f9739ed78841aecf9cb426b4a
Author: ianswett <ianswett@chromium.org>
Date: Wed Jun 22 19:58:42 2016

gfe-relnote: Fix an error in QUIC's SpdyUtils where content-length was
being parsed as an int in CopyAndValidateHeaders.  Not flag protected.

Merge internal change: 125563088

BUG= 621295 

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

[modify] https://crrev.com/90c79f0d55ef287f9739ed78841aecf9cb426b4a/net/quic/spdy_utils.cc
[modify] https://crrev.com/90c79f0d55ef287f9739ed78841aecf9cb426b4a/net/quic/spdy_utils_test.cc

Comment 8 by ianswett@google.com, Jun 24 2016

The second patch is now in Chrome Canary and should have properly fixed the issue.  If you can try to verify the fix, that would be very helpful.

Comment 9 by tmora...@akamai.com, Jun 27 2016

I can confirm that it works with this version of Chrome Canary:
Version 53.0.2777.0 canary (64-bit)

Thank you Ian!
Status: Verified (was: Unconfirmed)

Comment 11 by rch@chromium.org, Jun 28 2016

Labels: Merge-Request-52

Comment 12 by dimu@google.com, Jun 28 2016

Labels: -Merge-Request-52 Merge-Approved-52 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M52 (branch: 2743)
Project Member

Comment 13 by bugdroid1@chromium.org, Jun 28 2016

Labels: -merge-approved-52 merge-merged-2743
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d51432a92256f40c5a5559f16c60c27a596ed795

commit d51432a92256f40c5a5559f16c60c27a596ed795
Author: Ryan Hamilton <rch@chromium.org>
Date: Tue Jun 28 20:50:49 2016

m[52] relnote: Fix an error in QUIC's SpdyUtils where content-length was being parsed as an int in CopyAndValidateHeaders.  Not flag protected.

Merge internal change: 125563088

BUG= 621295 

Review-Url: https://codereview.chromium.org/2081713006
Cr-Commit-Position: refs/heads/master@{#401388}
(cherry picked from commit 90c79f0d55ef287f9739ed78841aecf9cb426b4a)

Review URL: https://codereview.chromium.org/2107923002 .

Cr-Commit-Position: refs/branch-heads/2743@{#510}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/d51432a92256f40c5a5559f16c60c27a596ed795/net/quic/spdy_utils.cc
[modify] https://crrev.com/d51432a92256f40c5a5559f16c60c27a596ed795/net/quic/spdy_utils_test.cc

Cc: pbomm...@chromium.org
Labels: Needs-Feedback
Tested the issue on Mac 10.11.5 using 52.0.2743.60 with below steps:

1.Opened chrome by enabling QUIC indicator through QUIC command and opened URL: https://www.example.org/5GB.bin.
2.Unable to access the site.
3.Opened chrome through command given in comment #0.
4.Observed the error 'The site can't be reached'.

Please find attached screencasts of two ways of checking the issue and update if anything missed here in triaging the issue.

ianswett@Could you please provide another sample URL or file to verify the fix from test team end.

621295_1.mp4
1.3 MB View Download
621295_2.mp4
1.4 MB View Download
In this case, tmoravec@akamai.com was using example.org as a local testing domain.

I don't have a public URL that reproduces this bug, because Google Drive does not appear to supply content-length for very large files.

Sign in to add a comment