New issue
Advanced search Search tips

Issue 791056 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows , Mac
Pri: 2
Type: Bug



Sign in to add a comment

SparseControl::DoGetAvailableRange() returns negative byte range

Reported by fval...@connected-labs.com, Dec 1 2017

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64; rv:50.0) Gecko/20100101 Firefox/50.0

Example URL:

Steps to reproduce the problem:
I can reproduce a crash with following stack loading ressource from cache (http, not https)

FATAL:cert_status_flags.cc(99) Check failed: false .
#0 0x0000019d02de base::debug::StackTrace::StackTrace()
#1 0x0000019e6c5e logging::LogMessage::~LogMessage()
#2 0x0000023528a8 net::MapCertStatusToNetError()
#3 0x000001800974 content::SSLManager::OnSSLCertificateError()
#4 0x00000163a559 content::ResourceLoader::OnSSLCertificateError()
#5 0x0000025c42cb net::URLRequestHttpJob::OnStartCompleted()
#6 0x0000023f30c1 net::HttpCache::Transaction::DoLoop()
#7 0x00000241caff net::PartialData::GetAvailableRangeCompleted()
#8 0x0000025496ca disk_cache::InFlightBackendIO::OnOperationComplete()
#9 0x00000254c011 disk_cache::InFlightIO::InvokeCallback()

I reproduce it using dash.js and a private http server.
I'm using a chromium build based on 55.0.2883.105 using dcheck_always_on but the related code in src/net didn't change so version should not matter I think.

net::URLRequestHttpJob::OnStartCompleted() is invoked with result = -200

-200 is ERR_CERT_COMMON_NAME_INVALID but it makes any sense because in my case, the resource is using http. There is no https involved.

Tracking down the -200 value, I found that it's a number of range coming from SparseControl::DoGetAvailableRange
https://cs.chromium.org/chromium/src/net/disk_cache/blockfile/sparse_control.cc?type=cs&q=SparseControl::DoGetAvailableRange()&l=823

In my case partial_start_bytes = 612 and block_offset = 812
so result_ is set to result_ = partial_start_bytes - block_offset ==> -200.

Any help / comment would be helpful

What is the expected behavior?

What went wrong?
browser process crashed.

Did this work before? N/A 

Chrome version: ALL  Channel: n/a
OS Version: 
Flash Version:
 
Components: -Internals>Network Internals>Network>Cache
Owner: morlovich@chromium.org
Status: Available (was: Unconfirmed)
Thanks for the investigation; that's pretty interesting...

Note: switching the cache backend might be a workaround that suits your purposes; it's "--use-simple-cache-backend=on" for Chrome itself, but I don't know how your things are wired up. Could probably hack it in in disk_cache.cc since you seem to be just building from source, too...

Hi thanks for feedback. I will try to build chromium head and see if I can find an easy / shareable way to reproduce.
I've been able to reproduce the issue using chromium head (build from this morning, commit 09c21aa99af3ec) and a minimum test page.

I'm building on OpenSuse Tumbleweed using following gn args (I don't think it really matter as long as dcheck are activated, i.e. debug build or dcheck_always_on=true)

use_jumbo_build=true
enable_nacl=false
linux_use_bundled_binutils=false
use_debug_fission=false
is_clang=false
use_sysroot=false
proprietary_codecs=true
debug_devtools=false
enable_google_now=false
ffmpeg_branding="ChromeOS"
is_component_build=false
media_use_ffmpeg=true media_use_libvpx=true
remove_webcore_debug_symbols=true
symbol_level=0
treat_warnings_as_errors=false
use_cups=false

steps to reproduce:

./out/Default/content_shell --no-sandbox https://release.voxtok.com/chromium_791056/crashNetwork.html

when video is playing, refresh the page, you should get (hopefully) FATAL:cert_status_flags.cc(99)] Check failed: false

The test page contains garbage content, but it's just used to reproduce the issue so I didn't clean it.
I don't own the dash content, so we will remove the test page after a few days. Please email me if needed.
The URL is using https here, but it's not changing the behavior (the cert issue coming from disk_cache/blockfile/sparse_control.cc	


Also, "--use-simple-cache-backend=on" workaround is working. 

Thanks.


Status: Assigned (was: Available)
Confirmed. Thanks for the testcase --- seeing if I can reduce it to something code-level/unit-test-like...

Testcase:
--- a/net/disk_cache/entry_unittest.cc
+++ b/net/disk_cache/entry_unittest.cc
@@ -1835,6 +1835,42 @@ TEST_F(DiskCacheEntryTest, MemoryOnlyGetAvailableRange) {
   GetAvailableRange();
 }
 
+TEST_F(DiskCacheEntryTest, GetAvailableRangeBlockFileDiscontinuous) {
+  //  crbug.com/791056  --- blockfile problem when there is a sub-KiB write before
+  // a bunch of full 1KiB blocks, and a GetAvailableRange is issued to which
+  // both are a potentially relevant.
+  InitCache();
+
+  std::string key("the first key");
+  disk_cache::Entry* entry;
+  ASSERT_THAT(CreateEntry(key, &entry), IsOk());
+
+  scoped_refptr<net::IOBuffer> buf_2k(new net::IOBuffer(2 * 1024));
+  CacheTestFillBuffer(buf_2k->data(), 2 * 1024, false);
+
+  const int kSmallSize = 612;  // sub-1k
+  scoped_refptr<net::IOBuffer> buf_small(new net::IOBuffer(kSmallSize));
+  CacheTestFillBuffer(buf_small->data(), kSmallSize, false);
+
+  // Sets some bits for blocks representing 1K ranges including [1024, 2048),
+  // whic will be relevant for the GetAvailableRange call.
+  EXPECT_EQ(2 * 1024, WriteSparseData(entry, /* offset = */ 1024,
+                                      buf_2k.get(), /* size = */ 2 * 1024));
+
+  // Now record a partial write from start of the first kb.
+  EXPECT_EQ(kSmallSize, WriteSparseData(entry, /* offset = */ 0,
+                                        buf_small.get(), kSmallSize));
+
+  // Try to query a range starting from that block 0. Even a 0 won't be
+  // bad here, but a negative number is right out...
+  int64_t start;
+  net::TestCompletionCallback cb;
+  int rv = entry->GetAvailableRange(812, 1247, &start, cb.callback());
+  EXPECT_GE(cb.GetResult(rv), 0);
+
+  entry->Close();
+}
+
 // Tests that non-sequential writes that are not aligned with the minimum sparse
 // data granularity (1024 bytes) do in fact result in dropped data.
 TEST_F(DiskCacheEntryTest, SparseWriteDropped) {

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 12 2017

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

commit d5f7284fcdd30a23fcb4263efb2842a3a487d041
Author: Maks Orlovich <morlovich@chromium.org>
Date: Tue Dec 12 19:59:42 2017

Blockfile cache: fix DoGetAvailableRange in some discontinuous cases

In particular the motivating case was when the partial write was before
the bitmap...

... however this is done by rewriting the function entirely to be
formulated in terms of net::Interval, since I found the details of
the original pretty much inscrutable, but believe to have understood
both the data representation and the interface.

Bug:  791056 
Change-Id: I297ee4cbcfc204df8ccab9c45145eaf0a82eb0b7
Reviewed-on: https://chromium-review.googlesource.com/806687
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Maks Orlovich <morlovich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523523}
[modify] https://crrev.com/d5f7284fcdd30a23fcb4263efb2842a3a487d041/net/disk_cache/blockfile/sparse_control.cc
[modify] https://crrev.com/d5f7284fcdd30a23fcb4263efb2842a3a487d041/net/disk_cache/entry_unittest.cc

Labels: -OS-Linux OS-Mac OS-Windows
Status: Fixed (was: Assigned)
Fixed... for M65, I think?
Fix confirmed. Thanks a lot.

Sign in to add a comment