New issue
Advanced search Search tips

Issue 691754 link

Starred by 2 users
Status: Fixed
Owner:
jsb...@chromium.org
Closed: Mar 2017
Cc:
pwnall@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Key generator behavior for values above 2^53 is inconsistent

Project Member Reported by jsb...@chromium.org, Feb 13 2017 
Spec bug:

https://github.com/w3c/IndexedDB/issues/147

Test cases:

https://github.com/w3c/web-platform-tests/pull/4814

Spec has inconsistencies here, but Chrome's impl is reliant on undefined behavior:

https://cs.chromium.org/chromium/src/content/browser/indexed_db/indexed_db_database.cc?l=1204

The incoming key - a number which is a double and therefore could be Infinity or > 2^63 - is coerced via;

static_cast<int64_t>(floor(key.number())) + 1

I have no idea what Infinity ends up doing here (it's undefined behavior per C++). We end up ignoring it so it probably becomes 0 or something which is safe but we need to bounds check and figure out what the desired behavior is. 

Fun cases:

* Anything about the generator maximum value (2^53)
* Anything above that but that fits into int64_t
* Anything above that but that fits into uint64_t
* Anything above that but finite
* Infinity
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 7 2017 

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

commit c4c73fb74bfeedbc41e0fc41f14feed642c6a86b
Author: jsbell <jsbell@chromium.org>
Date: Tue Mar 07 22:31:34 2017

Indexed DB: Ensure large explicit keys consistently max out generator

Background: Stores can have a key generator which generates successive
numeric keys. Storing a record with an explicit numeric key adjusts
they key generator to produce values above the explicit key. Once the
generator hits 2^53 it stops generating new keys (since that's the
maximum integer uniquely representable as a JS number).

Chrome's logic for certain values above this limit was "wonky". Values
above 2^53 would max out the generator. Values above 2^63 and Infinity
would be ignored and not adjust the generator, due to relying on
undefined double->int64_t casting behavior.

Fix to always max out the generator for large values. Also adds
web-platform-tests - other implementations are wonky too. :(

Also adds some missing test coverage for key injection cases.

Spec discussion: https://github.com/w3c/IndexedDB/issues/147

BUG= 691754 

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

[modify] https://crrev.com/c4c73fb74bfeedbc41e0fc41f14feed642c6a86b/content/browser/indexed_db/indexed_db_database.cc
[add] https://crrev.com/c4c73fb74bfeedbc41e0fc41f14feed642c6a86b/third_party/WebKit/LayoutTests/external/wpt/IndexedDB/keygenerator-explicit.html
[add] https://crrev.com/c4c73fb74bfeedbc41e0fc41f14feed642c6a86b/third_party/WebKit/LayoutTests/external/wpt/IndexedDB/keygenerator-inject.html

Comment 2 by jsb...@chromium.org, Mar 8 2017

Status: Fixed (was: ExternalDependency)
►

Sign in to add a comment