Starting in SQLite 3.38.0, JSON support is enabled by default. Chromium
features shouldn't use JSON support in SQL statements, so we disable
JSON support, which saves binary size.
This CL adds a test ensuring that our SQLite build configuration matches
our intention.
Bug: 1300262
Change-Id: I6380b553cb78d284d03afb2fc18c1c50a11a61bc
Tested: Verified that test fails on SQLite 3.38.0 without SQLITE_OMIT_JSON
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3492744
Auto-Submit: Victor Costan <pwnall@chromium.org>
Reviewed-by: Austin Sullivan <asully@chromium.org>
Commit-Queue: Austin Sullivan <asully@chromium.org>
Cr-Commit-Position: refs/heads/main@{#975823}
macOS and iOS each have OS-supported backup options that automatically
backup app data unless specifically excluded from backup.
Currently, backup exclusion on macOS is handled by SetFileBackupExclusion
in base/mac/mac_util.h, and backup exclusion on iOS is handled by
SetSkipSystemBackupAttributeToItem in
ios/chrome/browser/file_metadata_util.h.
In order to be used outside of ios/chrome, the iOS backup exclusion
function needs to be move be a more accessible location. However, since
files/directories excluded from backup on one of {macOS, iOS} are
generally good candidates to be excluded from backup on the other
platform as well, this CL creates a common function for both platforms
in base/mac/backup_util.h.
Future CLs will use this function to exclude more files from backup,
such as the Safe Browsing database.
Bug: 1265580
Change-Id: I59735c4af908c70880764f961b2869d869edb0d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3300920
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Commit-Queue: Ali Juma <ajuma@chromium.org>
Cr-Commit-Position: refs/heads/main@{#948971}
The Database::Execute() guidance currently points to
ExecuteAndReturnErrorCode() for situations where errors would be
ignored. However, the guidance is ignored [1, 2], and
ExecuteAndReturnErrorCode() is not used outside of //sql.
This CL makes ExecuteAndReturnErrorCode() private to Database, and
updates testing code that relied on it. This is an architectural
improvement, as code outside //sql should not have to know about SQLite
error codes.
[1] https://source.chromium.org/search?q=ignore_result%5C(.*%5C.Execute%5C(
[2] https://source.chromium.org/search?q=ignore_result%5C(.*-%3EExecute%5C(
Change-Id: I8d58180a40094c50e1da12b1445fb94c88531a35
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3043151
Auto-Submit: Victor Costan <pwnall@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#904027}
The use of SQLite virtual tables is discouraged in Chrome feature code.
After this CL lands, accessing virtual tables in SQL statements requires
setting DatabaseOptions::enable_virtual_tables_discouraged to true. This
applies to all Chrome feature code, but not to WebSQL, which uses its
own SQLite abstraction. CREATE VIRTUAL TABLE and DROP VIRTUAL TABLE will
still work, but the virtual tables will not be accessible to SQL
statements such as SELECT and INSERT.
The change will make Chrome feature developers aware when using this
discouraged feature, which is expected to curb usage.
Disabling access to virtual tables in Chrome features is the best we can
do for now, because WebSQL still needs SQLite support for the fts3
virtual table. After we remove WebSQL, we can consider removing all
virtual table support from SQLite.
Bug: 910955
Change-Id: Ib89837d0f874c0f2a0c1a0ca2ebef6b14ada1335
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3034241
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Auto-Submit: Victor Costan <pwnall@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#902751}
This is a reland of 525b30ab9b
This reland removes the call to GetDBOptions() from the DatabaseTest
constructor, which was flagged as undefined behavior by the ubsan-vptr
bot. sql::Database construction is deferred to the SetUp() method by
turning the test's sql::Database member into a
std::unique_ptr<sql::Database>.
Original change's description:
> sql: Remove SQLTestBase.
>
> Change-Id: I87bf9499ef590b006660d3b8ab305b0192ec405c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2866306
> Auto-Submit: Victor Costan <pwnall@chromium.org>
> Commit-Queue: Ayu Ishii <ayui@chromium.org>
> Reviewed-by: Ayu Ishii <ayui@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#879443}
Change-Id: Ie83bf28eaebb88883b9eb37a7d8407e8bfc619ad
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2878638
Reviewed-by: Ayu Ishii <ayui@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#880595}
The last remaining SQLite patch is
0001-Custom-shell.c-helpers-to-load-Chromium-s-ICU-data. This CL adds
tests ensuring that SQLite uses ICU, creating a safety net for the
removal of the SQLite patch.
Bug: 945204
Change-Id: I2b5661d78fcd00e7c088fc234eeef0f981f2bb62
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1621845
Auto-Submit: Victor Costan <pwnall@chromium.org>
Reviewed-by: Chris Mumford <cmumford@google.com>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#661968}
Previously unix-dotlock VFS was used by default on Fuchsia. dotlock
creates .lock directory to lock files. It doesn't work correctly because
the lock is not removed if the process dies before releasing the lock.
On Fuchsia profile directories are not expected to be shared between
processes, so cross-process file locks are not necessary.
This CL switches Chromium's VFS wrapper to use unix-none as a base VFS
(which uses no-op locking) and adds custom in-process locking for
Fuchsia in the wrapper.
Bug: 936672
Change-Id: Ibb0a84ba0e540b21551d5f174ec32f0d9077cf98
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1504048
Commit-Queue: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Auto-Submit: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#639257}
This CL:
1) Re-enables MacUtilTest.TestExcludeFileFromBackups, which was disabled
due to failures on a bot that does not exist anymore.
2) Extracts the logic used to check base::mac::SetFileBackupExclusion()
into base::mac::GetFileBackupExclusion(). This logic was duplicated
in tests.
3) Uses the approach in //sql for converting a base::FilePath into the
CFURLRef needed by CSBackup{Is,Set}ItemExcluded. This approach does
not rely on an autorelease pool. The conversion logic is extracted in
base::mac::FilePathToCFURL().
Change-Id: I6ed2a3ded285e6904dccf7e21ade7ff5f68167a2
Reviewed-on: https://chromium-review.googlesource.com/c/1438035
Reviewed-by: Joshua Bell <jsbell@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Auto-Submit: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626702}
SQLite patch 0007-Allow-auto-vacuum-to-work-with-chunks.patch
introduces a PRAGMA that is not used outside of tests. The PRAGMA was
added for an experiment that was discontinued, referenced on the bug
above.
This CL removes the patch and the associated tests, reducing the total
amount of patches we carry on top of SQLite.
No functional changes are expected, because the patch is inert in
production. Specifically, the newly introduced autoVacuumSlack member of
the BtShared structure is zero-initialized by default, and can only be
changed by issuing the auto_vacuum_slack_pages PRAGMA. The new btree
code is gated by an if (... && pBt->autoVacuumSlack). The PRAGMA is not
used outside of testing.
Bug: 698010
Change-Id: Id2a40badccb96389c6ff9ce1780ffa703dedce6c
Reviewed-on: https://chromium-review.googlesource.com/c/1362222
Reviewed-by: Chris Mumford <cmumford@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613782}
This reverts commit 7202f4dc1f.
Reason for revert: CL is only needed in 72.0.3620.0 Windows Canary to diagnose SQLite crashes.
Original change's description:
> sqlite: Disable mmap usage on Windows.
>
> This is an experiment to see if the crashes in the referenced bug are
> related to mmap changes on Windows. This CL will be reverted after it
> goes live in 1-2 canaries.
>
> TBR=cmumford
>
> Bug: 897576
> Change-Id: Ic44836c4ed848f8713d16b553f038c63119f0aaf
> Reviewed-on: https://chromium-review.googlesource.com/c/1348736
> Commit-Queue: Victor Costan <pwnall@chromium.org>
> Reviewed-by: Victor Costan <pwnall@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#610551}
TBR=cmumford@chromium.org,pwnall@chromium.org
Change-Id: I4d79a63c2a428e1e8ab04143a4d8280e274db1fd
Bug: 897576
Reviewed-on: https://chromium-review.googlesource.com/c/1349789
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610700}
This is an experiment to see if the crashes in the referenced bug are
related to mmap changes on Windows. This CL will be reverted after it
goes live in 1-2 canaries.
TBR=cmumford
Bug: 897576
Change-Id: Ic44836c4ed848f8713d16b553f038c63119f0aaf
Reviewed-on: https://chromium-review.googlesource.com/c/1348736
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610551}
sql::Connection represents an opened SQLite database. Pedantically, the
name is correct. At the same time, it is inconsistent with the naming
convention of similar software -- IndexedDB, LevelDB, and WebSQL all use
a derivative of "Database" (IDBDatabase, DB, WindowDatabase) to name
this concept. Furthermore, most sql::Connection instances have names
like "db" and "database".
This CL renames the class to sql::Database, reducing the cognitive load
caused by dissonances like "sql::Connection db_".
TBR=mpearson
Bug: none
Change-Id: I8abf34016bca236b6e38a772f5848e7ea9bf1c59
Reviewed-on: https://chromium-review.googlesource.com/1157876
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579990}
Chrome's SQLite currently uses permissive (0644) POSIX access controls
for newly created databases, and //sql offers a method for opting into
restrictive (0600) permissions. This method is only used by the login
database. However, all Chrome databases are likely to have private data.
For example, the cookies database may contain long-lived OAuth tokens,
and can be just as valuable as the login database. The same argument
applies for the DOMStorage database.
This CL configures SQLite to use restrictive permissions by default, and
removes the method for opting into the restrictive permissions.
Change-Id: I5f0ce9e7f038081fad515cfc30c45ccccf7ff1b6
Reviewed-on: https://chromium-review.googlesource.com/1146295
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Chris Mumford <cmumford@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577476}
USE_SYSTEM_SQLITE used to be supported on iOS, and now is a no-op.
This CL removes the references to it from //sql, to avoid confusing
readers.
The CL also breaks down an unnecessarily large unit test in
//sql/connection_unittest.cc that happend to be near the changes in this
CL.
Change-Id: I47c324300b54362529364a5ef26ee45b45fe3b76
Reviewed-on: https://chromium-review.googlesource.com/1137921
Reviewed-by: Chris Mumford <cmumford@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576213}
The feature is used by //components/webdata. This test will help make
sure the feature keeps working as we upgrade SQLite.
Bug: 829893
Change-Id: Ie94439e5abd2cad4e0e15bad92ec3af462850b4c
Reviewed-on: https://chromium-review.googlesource.com/1015206
Reviewed-by: Chris Mumford <cmumford@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552419}
This updates //sql to use base::RepeatingCallback instead of legacy
base::Callback. No intended functional change is included.
This reduces the number of 'base::Bind' in //sql from 19 to 1
as tracked at http://goo.gl/LUVhDj
Bug: 714018
Change-Id: I4b6b321e25f2b3db7ab7ac3e09d9ee74761db230
Reviewed-on: https://chromium-review.googlesource.com/952116
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541388}
We currently use the system-supplied SQLite library on iOS.
This is not the end of the world, because we require iOS 10.0+, which
means SQLite 3.14+, which has the features that Chrome uses. iOS is also
special in that we don't ship Blink, so we don't expose SQLite via
WebSQL -- we control all the other SQL queries we send to SQLite, so
SQLite security bugs aren't easily exploitable.
At the same time, USE_SYSTEM_SQLITE means Chrome for iOS uses an
entirely different SQLite build that most developers don't run. This
makes it more likely that we'll introduce bugs or crashes, and makes it
more difficult to investigate existing bugs/crashes.
In theory, USE_SYSTEM_SQLITE seems appealing on Android and Linux. In
practice, the idea isn't workable on either platform.
* On Android, not shipping our own SQLite would cut down APK size by a few
hundred KB. However, Android doesn't have a great upgrade story before
Lollipop (5.0), and we still support KitKat (4.4). SQLite is exposed
via WebSQL on Android, so we must be in control of SQLite upgrades.
* Linux distributions would like to ship Chromium builds linked against
their SQLite copies. However, https://crbug.com/807487 suggests that
USE_SYSTEM_SQLITE on Linux would result in unacceptable stability. The
root cause of the bug was that NSS can load SQLite on Linux, resulting
in SQLite getting initialized outside our control. Even if we could
fix that bug (via a dependency from //crypto to //sql), we'd still
have to chase all the libraries that might potentially initialize
SQLite. Today we believe that NSS is the only library in that set, but
we simply don't have the bandwidth to justify investigating the bug
reports that will come if we miss something.
In conclusion, there is no platform where USE_SYSTEM_SQLITE is desirable
and viable. So, this CL removes USE_SYSTEM_SQLITE from the codebase.
This will cause the iOS build to increase by a few hundred KB. This
price buys us one less difference between iOS and every other platform,
and increased OS stability in the long run, as iOS bugs / crashes will
be easier to diagnose.
Bug: 22208, 299684, 372584, 807093, 807487
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-device
Change-Id: I1bd64bd93527624e197bd082c725a952c8b3c430
Reviewed-on: https://chromium-review.googlesource.com/898223
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Chris Mumford <cmumford@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535148}
Chrome currently only supports iOS10 and above. iOS 10 ships with SQLite
3.14.0+, so we can remove some special case handling (and cognitive
burden) needed to support older iOS versions.
Bug: 807093
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-device
Change-Id: I5d19af5eeaffdf587fb43216fc9cf97e9297d15b
Reviewed-on: https://chromium-review.googlesource.com/898564
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Chris Mumford <cmumford@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534591}
Dot-file locking (which we're currently using on Fuchsia) specifies
"version 1" of VFS. This doesn't include SHM functions, so disable the
mmap-requiring tests.
Bug: 764423
Change-Id: I2bb24f4130bebdb3371aca4ee4169dd573d9cbb1
Reviewed-on: https://chromium-review.googlesource.com/665301
Reviewed-by: Victor Costan <pwnall@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502096}
SQLite WAL mode by default checkpoints the WAL to the main database at
close time. Chromium closes databases at shutdown, which causes
shutdown slowdowns due to buffered data being flushed. WAL
checkpointing would effectively increase the amount of buffered data
being flushed.
[WAL mode is currently not in use by Chromium.]
BUG=675264
Review-Url: https://codereview.chromium.org/2827673006
Cr-Commit-Position: refs/heads/master@{#465772}
SQLITE_FCNTL_CHUNK_SIZE can advise the VFS to resize files in quantum
amounts, to reduce fragmentation from tiny appends. This change allows
a new PRAGMA auto_vacuum_slack_pages to provide auto_vacuum with a hint
to only rearrange pages when an entire quantum can be released.
BUG=698010
Review-Url: https://codereview.chromium.org/2732553002
Cr-Commit-Position: refs/heads/master@{#459847}
Chromium's SQLite was modified to propagate OSX Time-Machine exclusions
from the main database file to any associated journal files.
Re-implement this using a VFS which wraps the default VFS and makes the
check when opening journal files.
BUG=679941
Review-Url: https://codereview.chromium.org/2623083002
Cr-Commit-Position: refs/heads/master@{#445601}
When the bug was logged, SQLite had a flaw where the regular
expression (Y in X REGEXP Y or REGEXP(Y,X)) effectively had the
compiled results cached with the prepared statement. At some point
Chromium pulled a version of SQLite which fixed this, this test
verifies that the SQLite currently in use has the fix.
BUG=248608
TBR=nyquist@chromium.org
Review-Url: https://chromiumcodereview.appspot.com/2440653002
Cr-Commit-Position: refs/heads/master@{#426398}
The mmap mitigation for bug 537742 meant that the test wasn't running in
cases where mmap could potentially be enabled but wasn't enabled by
default. Change the test to instead run when the platform allows SQLite
mmap to be enabled.
Add a test to verify that mmap cannot be enabled in cases where it is
expected not to work, so that platforms must make an explicit decision
about whether to allow mmap.
BUG=537742, 554269
Review URL: https://codereview.chromium.org/1529693002
Cr-Commit-Position: refs/heads/master@{#365904}
This adds a //sql/mojo library which can be linked into preexisting
sqlite3 code which adds a new VFS which transparently proxies filesystem
usage to the mojo:filesystem application.
We create a new sql_apptests.mojo target, which currently has all the
sql connection_unittests.cc (minus 2 hard ones), all statement and
transaction unit tests and refactor the sql testing stuff so that we
have two implementations of an SQLTestBase class: one that uses files
raw and one that proxies to the filesystem process.
Notably, this patch does not implement file locking, which will have to
be implemented before we can safely use this, but will be a large enough
patch in and of itself that I'm punting on it for this patch.
BUG=493311
Review URL: https://codereview.chromium.org/1176653002
Cr-Commit-Position: refs/heads/master@{#335415}
Long ago, Chromium used fts2 for history full-text search. It was
later replaced by fts3, and even later that feature was deleted
entirely. fts2 is no longer used in the browser at all, so stop
compiling it.
Since SQLite is used by WebSQL, in theory this could affect web
authors, but WebSQL uses an authorizer to allow only specific virtual
table types. fts2 is not one of those types, I have verified manually
that fts2 tables cannot be created using WebSQL.
BUG=455817
Review URL: https://codereview.chromium.org/999573003
Cr-Commit-Position: refs/heads/master@{#320135}
Ran through the import script in third_party/sqlite/README.Chromium,
including the SQLite test suite. There are a few pager errors which
are because of a change required for WebDatabase support (documented
in README).
SQLite changes are at http://www.sqlite.org/changes.html , Chromium
previously used 3.7.6.3.
All patches were applied and the results reviewed to make sure
backported patches were safe to remove, and retained patches were still
covering what was necessary.
Keep fts4 disabled, and also the new fts3 virtual table and unicode61
tokenizer. Once enabled, these are very hard to disable, and there
doesn't seem to be any pressure to enable them. Other SQLITE_* flags
were reviewed for applicability, none looked essential.
Fixes to Chromium:
- In recovery.cc, pk_column now follows the documentation.
- Short garbage files now see SQLITE_NOTADB rather than
SQLITE_IOERR_SHORT_READ.
- Adjust to allow clients to use ScopedErrorIgnore without adding
dependencies.
- More-specific SQLITE_CONSTRAINT_* errors aren't necessary.
- Force recovery test to scan table rather than index.
BUG=340757
TEST=*EVERYTHING* continues to work.
R=michaeln@chromium.org
Review URL: https://codereview.chromium.org/901033002
Cr-Commit-Position: refs/heads/master@{#315646}
