1752045 - -0 is replaced with +0 in IndexedDB keys

Status: NEW

(Core :: Storage: IndexedDB, defect, P3)

Description

[Evgeny Kapun]

Steps to reproduce:

Run the following code:

let openRequest = indexedDB.open("test");
openRequest.onupgradeneeded = function() {
  let objectStore = this.result.createObjectStore("test");
  objectStore.add(-0, -0);
  objectStore.getAllKeys().onsuccess = function() {
    console.log("Key is +0:", Object.is(this.result[0], +0));
  };
};

Actual results:

"Key is +0: true" is logged, which contradicts IndexedDB spec.

Expected results:

"Key is +0: false" should be logged instead. This is what happens in Chromium, and is consistent with the spec.

Comment 1

[Evgeny Kapun]

See also: https://github.com/w3c/IndexedDB/issues/375

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

Thank you for filing this and raising a spec issue, and extra thanks for the concise reproduction steps!

We intentionally do some "clever" encoding for sort purposes per the docs that ends up as code here but we aren't handling -0 correctly.

Because we're doing 0 - negativeNumber on -0 (which has a float rep of 0x80_00_00_00_00_00_00_00) to attempt to invert the double's bits for encoding on 0x80_00_00_00_00_00_00_00 instead of adjusting it to 0x7f_ff_ff_ff_ff_ff_ff_ff we're ending up with the same value as when we adjust +0 (and all positive numbers) by or-ing them with 0x80_00_00_00_00_00_00_00. Note that because we omit trailing 0's in our keys means that both +0 and -0 end up encoded as 0x80 (unless the zeroes are in an array and then followed by other stuff).

It seems like we can fix this without too much trouble, but all existing stored key data (not value data!) will irrevocably have had -0 converted to +0. It's not clear there's any sane mitigation for existing code that was trying to store -0 and now would experience inconsistencies. That said, we could do an internal schema version bump and clear the origin for any origin where we see an attempt to use -0 as a key in a database that was created prior to us supporting -0. (That is, we can detect use of -0 in a put/add key and know that a database being used previously could not have represented that cleanly and then choose the nuclear option of clearing all offline data for the site.) I think it's probably better to not have mitigations that do things like that, though.

We definitely want to augment our gtest for keys at https://searchfox.org/mozilla-central/source/dom/indexedDB/test/gtest/TestKey.cpp for sure!

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

Before we start to fix this, we should note, however, that normalization is being considered in https://github.com/w3c/IndexedDB/issues/375. Depending on how that goes, this may be more about adding appropriate comments and tests and/or cleaning up the clever encoding, rather than making us support -0.