Files for test case. Note that server.js is a Node.js script, and testcase.html should be placed in the same directory.

Deleted: server.js 2.0 KB

server.js 2.0 KB View Download

Deleted: testcase.html 2.2 KB

testcase.html 2.2 KB View Download

The cache seems very unreliable when range requests are used.
Additional tests below show that the responses for the same request can be different, and that seemingly unrelated byte range requests affect the response of other requests.

I ran more tests, and reused the test framework from the report.
3. Open the DevTools before executing step 3 of the report.
4. When the test finishes, go to the Network tab and copy the URL.
In the console, run the following code (with the URL replaced):

await doFetch('http://localhost:8080/bigfile?1520634101835', new Headers({'range':'bytes=0-50000'}));

Expected: "50001" (the content-length of the response).
Actual  : "10000"

I have repeated the above multiple times (after waiting for the response, to make sure that there are no concurrent requests).

Then I change the start offset by one:
await doFetch('http://localhost:8080/bigfile?1520634101835', new Headers({'range':'bytes=1-50000'}));

Expected: "50000"
Actual  : "9999" (this is one lower than the previous bad case, but still not 50000...)
(I also repeated this multiple times to see if there would be any difference)

Then I repeated the original range request again (starting from 0):
await doFetch('http://localhost:8080/bigfile?1520634101835', new Headers({'range':'bytes=0-50000'}));

Expected: "50001" (the content-length of the response).
Actual  : "9999" (This is different)



PS. If someone starts working on this bug, consider looking at bug 770694 too. Perhaps the two bugs are related.

I'm not terribly familiar with the cache, but it's been suggested in the past that we stop caching range request responses - we're the only browser that does (Or at least we were at one point, not sure if it's still the case), and it adds a lot of complexity to our cache implementation.  We could also consider just disabling it until this current set of issues is resolved.

@reporter: Can you please provide the net internals logs for the issue as per the following instructions:
https://sites.google.com/a/chromium.org/dev/for-testers/providing-network-details

See attachment. I followed the STR from the report, but opened the devtools before executing step 3.

Deleted: chrome-net-export-log.json 68.3 KB

chrome-net-export-log.json 68.3 KB View Download

re: comment #3: they are rather important for things like short looping videos 
(or at least multiple people complained about backend size limits breaking caching for their not-quite-short videos); though I definitely agree with you on complexity impact.
Also I think our PDF plugin might use it, much like pdf.js, which it sounds like what this was discovered with?

re: comment #5: so I peeked at the log to see if the backend screws up, but it's uninteresting in that way; dumping what I noticed here in case it saves Shivani some time. The first requests is a regular GET, with no range funkyness, though we only get partway through before we doom the cache entry due to the 2nd request 
coming in (note that our story with concurrent range request is not great even discounting outright bugs, I think):

t= 11 [st= 10]    SIMPLE_CACHE_ENTRY_DOOM_CALL
t= 11 [st= 10]    SIMPLE_CACHE_ENTRY_DOOM_BEGIN
t= 12 [st= 11]    SIMPLE_CACHE_ENTRY_DOOM_END
t=105 [st=104]    SIMPLE_CACHE_ENTRY_WRITE_CALL
                  --> buf_len = 10000
                  --> index = 1
                  --> offset = 10000
                  --> truncate = true

(10K, this may be important)

Then on second request:

t=10 [st=1]      HTTP_CACHE_CALLER_REQUEST_HEADERS
                 --> X-DevTools-Emulate-Network-Conditions-Client-Id: C93601E469E7CA04FBA40CFA3F2BF19E
                     User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/67.0.3367.0 Safari/537.36
                     range: bytes=0-65535
                     Accept: */*
                     Referer: http://localhost:8080/
                     Accept-Encoding: gzip, deflate, br
                     Accept-Language: en-US,en;q=0.9
                 --> line = ""
t=11 [st=2]      HTTP_CACHE_GET_BACKEND  [dt=0]
t=11 [st=2]      HTTP_CACHE_OPEN_ENTRY  [dt=0]
t=11 [st=2]      HTTP_CACHE_ADD_TO_ENTRY  [dt=0]
t=11 [st=2]      HTTP_CACHE_READ_INFO  [dt=0]
t=11 [st=2]      HTTP_CACHE_RESTART_PARTIAL_REQUEST
t=11 [st=2]      HTTP_CACHE_CREATE_ENTRY  [dt=0]
t=11 [st=2]      HTTP_CACHE_ADD_TO_ENTRY  [dt=1]
t=12 [st=3]     +HTTP_STREAM_REQUEST  [dt=2]
t=12 [st=3]        HTTP_STREAM_JOB_CONTROLLER_BOUND
                   --> source_dependency = 386 (HTTP_STREAM_JOB_CONTROLLER)
t=14 [st=5]        HTTP_STREAM_REQUEST_BOUND_TO_JOB
                   --> source_dependency = 387 (HTTP_STREAM_JOB)
t=14 [st=5]     -HTTP_STREAM_REQUEST
t=14 [st=5]     +HTTP_TRANSACTION_SEND_REQUEST  [dt=0]
t=14 [st=5]        HTTP_TRANSACTION_SEND_REQUEST_HEADERS
                   --> GET /bigfile?1520764865483 HTTP/1.1
                       Host: localhost:8080
                       Connection: keep-alive
                       User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/67.0.3367.0 Safari/537.36
                       Accept: */*
                       Referer: http://localhost:8080/
                       Accept-Encoding: gzip, deflate, br
                       Accept-Language: en-US,en;q=0.9
                       Range: bytes=0-9999

... Notice the change in range.

Initial investigation reveals this:
While the 1st request is still pending completion, the 2nd request comes in, due to parallel validation , starts looking at the headers written in the cache by the 1st request and hits this !ConditionalizeRequest() condition (https://cs.chromium.org/chromium/src/net/http/http_cache_transaction.cc?dr&l=2470). This leads to dooming the existing entry. This seems wrong to doom the existing entry.

Also noticed while debugging that if there is a timing gap between the 2 requests, the second succeeds. It might help to understand how does it get past the !ConditionalizeRequest() condition in that case?

Created a CL for the fix here: https://chromium-review.googlesource.com/c/chromium/src/+/959450

The issue is this:
HttpCache::Transaction::ResetPartialState dooms the current entry, and
goes back to creating a new entry for this request. In case of parallel
validation, if the first request is a full request and has written some
part of the response to the cache, then this second transaction would
create the range header with as many bytes as the written response even
on dooming the current entry which is incorrect.
This CL fixes it by restoring the original range header on this path.

(Removing needs-feedback label, looks like it was provided.)

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

commit 4fe20e106bcb130bea7cbffdca6bc6a192e7b1fa
Author: Shivani Sharma <shivanisha@chromium.org>
Date: Thu Mar 15 22:36:08 2018

[Http Cache]: Reset the range headers to original when restarting

HttpCache::Transaction::ResetPartialState dooms the current entry, and
goes back to creating a new entry for this request. In case of parallel
validation, if the first request is a full request and has written some
part of the response to the cache, then this second transaction would
create the range header with as many bytes as the written response even
on dooming the current entry which is incorrect.
This CL fixes it by restoring the original range header on this path.

RangeGET_ParallelValidationCouldntConditionalize
Also manually verified the test case reported in the bug.

TEST: net_unittests --gtest_filter=HttpCache.
Bug:  820599 
Change-Id: I4e57003193873e433c3b503b9fc20a843f28c4da
Reviewed-on: https://chromium-review.googlesource.com/959450
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Commit-Queue: Shivani Sharma <shivanisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543521}
[modify] https://crrev.com/4fe20e106bcb130bea7cbffdca6bc6a192e7b1fa/net/http/http_cache_transaction.cc
[modify] https://crrev.com/4fe20e106bcb130bea7cbffdca6bc6a192e7b1fa/net/http/http_cache_unittest.cc

Verified fixed in 67.0.3376.0 with the test case from my initial report.