New issue
Advanced search Search tips

Issue 829141 link

Starred by 5 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

IndexedDB: Backend store migration from version 2 to 3 did not update on-disk version

Project Member Reported by dmu...@chromium.org, Apr 4 2018

Issue description

IndexedDB schema upgrade from 2 to 3 had a bug where it never saved the new schema version. So any databases that started as schema 1 or 2 stayed at schema 2, and were never 'saved' as schema 3 (although they were treated as schema 3 databases by the code).

Because the upgrade code to schema version 3 deleted the blob directory (possibly attempting to get a clean state for the code, as schema 3 added support for blobs in IDB), this meant that all of these v2 databases had their blobs wiped every time they opened.

To correct this problem and make sure the world doesn't have corrupt databases, for all schema 2 databases we need to:

Option A:
Blow away all databases that are schema version 2 as corrupted

Option B:
1. Look for BlobDataKey entries for all databases and object stores (will run in O(databases * object stores) time).
2. If there are no blob entries, then save the new schema version as version 3 and successfully open the database.
3. If there are blob entries, then the database is corrupt, and blow it away as corrupted.
 
Cc: pwnall@chromium.org
Summary: IndexedDB: Backend store migration from version 2 to 3 did not update on-disk version (was: IndexedDB: Correctly upgrade schemas from 2 to 3, and detect blob corruption.)
The bug is in IndexedDBBackingStore::SetUpMetadata in https://cs.chromium.org/chromium/src/content/browser/indexed_db/indexed_db_backing_store.cc under the "if (db_schema_version < 3)" branch.
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 5 2018

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

commit 3c0d175b5584a2e5be9d493aca8ddd46c7d78e12
Author: Daniel Murphy <dmurph@chromium.org>
Date: Thu Apr 05 04:35:38 2018

[IndexedDB] Stop deleting blobs on open for schema version 2 dbs

Bug:  756447 ,  829125 , 829141
Change-Id: I854594ed35fc5a99df1b68534db42bcb65b81a0f
Reviewed-on: https://chromium-review.googlesource.com/996582
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548325}
[modify] https://crrev.com/3c0d175b5584a2e5be9d493aca8ddd46c7d78e12/content/browser/indexed_db/indexed_db_backing_store.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Apr 10 2018

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

commit 705d7330d828101a658d390e8ba0790b8a6d29be
Author: Daniel Murphy <dmurph@chromium.org>
Date: Tue Apr 10 02:53:28 2018

[IndexedDB] Schema upgrade fix, fail for corrupt blobs, fix enums.xml

For the last 4 years we 'saved' the new schema version number, so any v2
databases are never marked as v3. The 'migration' always deleted all
blob files. To correctly upgrade to v3, all v2 databases that have blob
entries are considered corrupt and are wiped.

This also fixes the enums.xml values for IDB setup errors, which have
been missing 'DeleteIndex' and 'ClearObjectStore' (and subsequently have
had incorrect reporting on values > 23) since 2014:
https://crrev.com/b05831ee

Bug:  756447 ,  829125 , 829141
Change-Id: I1d711b5ed4bfb4e7240da56a2fc78e6ebe561ca1
Reviewed-on: https://chromium-review.googlesource.com/1000138
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Joshua Bell <jsbell@chromium.org>
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549395}
[modify] https://crrev.com/705d7330d828101a658d390e8ba0790b8a6d29be/content/browser/indexed_db/indexed_db_backing_store.cc
[modify] https://crrev.com/705d7330d828101a658d390e8ba0790b8a6d29be/content/browser/indexed_db/indexed_db_backing_store.h
[modify] https://crrev.com/705d7330d828101a658d390e8ba0790b8a6d29be/content/browser/indexed_db/indexed_db_backing_store_unittest.cc
[modify] https://crrev.com/705d7330d828101a658d390e8ba0790b8a6d29be/content/browser/indexed_db/indexed_db_leveldb_operations.h
[modify] https://crrev.com/705d7330d828101a658d390e8ba0790b8a6d29be/content/browser/indexed_db/indexed_db_metadata_coding.cc
[modify] https://crrev.com/705d7330d828101a658d390e8ba0790b8a6d29be/content/browser/indexed_db/indexed_db_metadata_coding.h
[modify] https://crrev.com/705d7330d828101a658d390e8ba0790b8a6d29be/content/browser/indexed_db/indexed_db_reporting.h
[modify] https://crrev.com/705d7330d828101a658d390e8ba0790b8a6d29be/tools/metrics/histograms/enums.xml

Project Member

Comment 4 by bugdroid1@chromium.org, Apr 10 2018

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

commit 530acd0a64c89b4ae2f64f6199035db2c66aa5a0
Author: Alan Cutter <alancutter@chromium.org>
Date: Tue Apr 10 04:17:52 2018

Revert "[IndexedDB] Schema upgrade fix, fail for corrupt blobs, fix enums.xml"

This reverts commit 705d7330d828101a658d390e8ba0790b8a6d29be.

Reason for revert: Broke WinMSVC64 (dbg) compile.
https://ci.chromium.org/buildbot/chromium.win/WinMSVC64%20%28dbg%29/4682
indexed_db_backing_store_unittest.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) class leveldb::Status __cdecl content::indexed_db::GetInt<class content::LevelDBTransaction>(class content::LevelDBTransaction *,class base::BasicStringPiece<class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > > const &,__int64 *,bool *)" (__imp_??$GetInt@VLevelDBTransaction@content@@@indexed_db@content@@YA?AVStatus@leveldb@@PEAVLevelDBTransaction@1@AEBV?$BasicStringPiece@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@@base@@PEA_JPEA_N@Z) referenced in function "public: void __cdecl <lambda_395e582d16a7cd048cd3868c3a9ede8f>::operator()(class content::IndexedDBBackingStore *,class content::IndexedDBKey,struct content::IndexedDBValue,struct `private: virtual void __cdecl content::indexed_db_backing_store_unittest::IndexedDBBackingStoreTest_SchemaUpgradeWithoutBlobsSurvives_Test::TestBody(void)'::`2'::TestState *)const " (??R<lambda_395e582d16a7cd048cd3868c3a9ede8f>@@QEBAXPEAVIndexedDBBackingStore@content@@VIndexedDBKey@2@UIndexedDBValue@2@PEAUTestState@?1??TestBody@IndexedDBBackingStoreTest_SchemaUpgradeWithoutBlobsSurvives_Test@indexed_db_backing_store_unittest@2@EEAAXXZ@@Z)

Original change's description:
> [IndexedDB] Schema upgrade fix, fail for corrupt blobs, fix enums.xml
> 
> For the last 4 years we 'saved' the new schema version number, so any v2
> databases are never marked as v3. The 'migration' always deleted all
> blob files. To correctly upgrade to v3, all v2 databases that have blob
> entries are considered corrupt and are wiped.
> 
> This also fixes the enums.xml values for IDB setup errors, which have
> been missing 'DeleteIndex' and 'ClearObjectStore' (and subsequently have
> had incorrect reporting on values > 23) since 2014:
> https://crrev.com/b05831ee
> 
> Bug:  756447 ,  829125 , 829141
> Change-Id: I1d711b5ed4bfb4e7240da56a2fc78e6ebe561ca1
> Reviewed-on: https://chromium-review.googlesource.com/1000138
> Reviewed-by: Mark Pearson <mpearson@chromium.org>
> Reviewed-by: Joshua Bell <jsbell@chromium.org>
> Commit-Queue: Daniel Murphy <dmurph@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#549395}

TBR=mpearson@chromium.org,jsbell@chromium.org,dmurph@chromium.org,pwnall@chromium.org

Change-Id: I563e3f3b77f0ff72025d3d398a6e8f5e36a818bc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  756447 ,  829125 , 829141
Reviewed-on: https://chromium-review.googlesource.com/1004133
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549413}
[modify] https://crrev.com/530acd0a64c89b4ae2f64f6199035db2c66aa5a0/content/browser/indexed_db/indexed_db_backing_store.cc
[modify] https://crrev.com/530acd0a64c89b4ae2f64f6199035db2c66aa5a0/content/browser/indexed_db/indexed_db_backing_store.h
[modify] https://crrev.com/530acd0a64c89b4ae2f64f6199035db2c66aa5a0/content/browser/indexed_db/indexed_db_backing_store_unittest.cc
[modify] https://crrev.com/530acd0a64c89b4ae2f64f6199035db2c66aa5a0/content/browser/indexed_db/indexed_db_leveldb_operations.h
[modify] https://crrev.com/530acd0a64c89b4ae2f64f6199035db2c66aa5a0/content/browser/indexed_db/indexed_db_metadata_coding.cc
[modify] https://crrev.com/530acd0a64c89b4ae2f64f6199035db2c66aa5a0/content/browser/indexed_db/indexed_db_metadata_coding.h
[modify] https://crrev.com/530acd0a64c89b4ae2f64f6199035db2c66aa5a0/content/browser/indexed_db/indexed_db_reporting.h
[modify] https://crrev.com/530acd0a64c89b4ae2f64f6199035db2c66aa5a0/tools/metrics/histograms/enums.xml

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 10 2018

Labels: merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0d1aacb9239aae9ab57b1035cf62ceb1889e8c54

commit 0d1aacb9239aae9ab57b1035cf62ceb1889e8c54
Author: Daniel Murphy <dmurph@chromium.org>
Date: Tue Apr 10 18:23:18 2018

[IndexedDB] Stop deleting blobs on open for schema version 2 dbs

Bug:  756447 ,  829125 , 829141
Change-Id: I854594ed35fc5a99df1b68534db42bcb65b81a0f
Reviewed-on: https://chromium-review.googlesource.com/996582
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#548325}(cherry picked from commit 3c0d175b5584a2e5be9d493aca8ddd46c7d78e12)
Reviewed-on: https://chromium-review.googlesource.com/1005694
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#655}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/0d1aacb9239aae9ab57b1035cf62ceb1889e8c54/content/browser/indexed_db/indexed_db_backing_store.cc

Comment 6 by dmu...@chromium.org, Apr 10 2018

Labels: -merge-merged-3359
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 11 2018

Labels: merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9ab30dca6ad03d9dc70f4ca9868a38f1c9b97768

commit 9ab30dca6ad03d9dc70f4ca9868a38f1c9b97768
Author: Daniel Murphy <dmurph@chromium.org>
Date: Wed Apr 11 18:08:57 2018

[IndexedDB] Log schema version

Bug:  830909 ,  756447 ,  829125 , 829141
Change-Id: Iaf467fbe9cf2c836bfcd761d558c3f7c0e7379a8
Reviewed-on: https://chromium-review.googlesource.com/1003281
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Joshua Bell <jsbell@chromium.org>
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#549653}(cherry picked from commit c757238fa2717fa08178def1faf522ad3c876416)
Reviewed-on: https://chromium-review.googlesource.com/1007942
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#684}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/9ab30dca6ad03d9dc70f4ca9868a38f1c9b97768/content/browser/indexed_db/indexed_db_backing_store.cc
[modify] https://crrev.com/9ab30dca6ad03d9dc70f4ca9868a38f1c9b97768/content/browser/indexed_db/indexed_db_leveldb_coding.h
[modify] https://crrev.com/9ab30dca6ad03d9dc70f4ca9868a38f1c9b97768/content/browser/indexed_db/indexed_db_reporting.cc
[modify] https://crrev.com/9ab30dca6ad03d9dc70f4ca9868a38f1c9b97768/content/browser/indexed_db/indexed_db_reporting.h
[modify] https://crrev.com/9ab30dca6ad03d9dc70f4ca9868a38f1c9b97768/tools/metrics/histograms/histograms.xml

Project Member

Comment 10 by bugdroid1@chromium.org, May 23 2018

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

commit bee79acd3a8ea85cf5a316df8e5a88ffdb5e9db5
Author: Daniel Murphy <dmurph@chromium.org>
Date: Wed May 23 00:13:01 2018

[IndexedDB] Treat v2 schemas w/ blobs as corrupt, behind flag.

The flag is present so we can finch this feature on stable to mitigate
risk for first-party customers like Docs Offline.

Bug: 829141
Change-Id: Ibfed20dfe45c4a96445001f721a499e08e2bcb7e
Reviewed-on: https://chromium-review.googlesource.com/1069601
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560846}
[modify] https://crrev.com/bee79acd3a8ea85cf5a316df8e5a88ffdb5e9db5/content/browser/indexed_db/indexed_db_backing_store.cc
[modify] https://crrev.com/bee79acd3a8ea85cf5a316df8e5a88ffdb5e9db5/content/browser/indexed_db/indexed_db_backing_store_unittest.cc
[modify] https://crrev.com/bee79acd3a8ea85cf5a316df8e5a88ffdb5e9db5/content/public/common/content_features.cc
[modify] https://crrev.com/bee79acd3a8ea85cf5a316df8e5a88ffdb5e9db5/content/public/common/content_features.h

Project Member

Comment 11 by bugdroid1@chromium.org, May 30 2018

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

commit 10cc9570a11e9428b1db8c9acf9108a8198f143e
Author: Daniel Murphy <dmurph@chromium.org>
Date: Wed May 30 02:57:43 2018

[IndexedDB] Handle corruption in GetDatabaseNames

Bug: 829141,  847681 
Change-Id: I6679e700049fb13d94322a4e8b0f87bc1310fe9e
Reviewed-on: https://chromium-review.googlesource.com/1077677
Reviewed-by: Victor Costan <pwnall@chromium.org>
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562708}
[modify] https://crrev.com/10cc9570a11e9428b1db8c9acf9108a8198f143e/content/browser/indexed_db/indexed_db_factory_impl.cc

Project Member

Comment 12 by bugdroid1@chromium.org, May 31 2018

Labels: merge-merged-3440
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/498cfcc952b3cb1b2975e4ffc2d60b326ff64105

commit 498cfcc952b3cb1b2975e4ffc2d60b326ff64105
Author: Daniel Murphy <dmurph@chromium.org>
Date: Thu May 31 19:48:01 2018

[IndexedDB] Handle corruption in GetDatabaseNames

Bug: 829141,  847681 
Change-Id: I6679e700049fb13d94322a4e8b0f87bc1310fe9e
Reviewed-on: https://chromium-review.googlesource.com/1077677
Reviewed-by: Victor Costan <pwnall@chromium.org>
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#562708}(cherry picked from commit 10cc9570a11e9428b1db8c9acf9108a8198f143e)
Reviewed-on: https://chromium-review.googlesource.com/1081169
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#64}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/498cfcc952b3cb1b2975e4ffc2d60b326ff64105/content/browser/indexed_db/indexed_db_factory_impl.cc

Comment 13 Deleted

Sign in to add a comment