Bug 1752045 Comment 2 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

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](https://searchfox.org/mozilla-central/rev/7fb9750a14e99267a77328559c2fd022e360d99b/dom/indexedDB/Key.cpp#786-826) that [ends up as code here](https://searchfox.org/mozilla-central/source/dom/indexedDB/Key.cpp#786-826) 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 data irrevocably will 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` 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 crazy mitigations, 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!
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](https://searchfox.org/mozilla-central/rev/7fb9750a14e99267a77328559c2fd022e360d99b/dom/indexedDB/Key.cpp#786-826) that [ends up as code here](https://searchfox.org/mozilla-central/source/dom/indexedDB/Key.cpp#786-826) 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` 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 crazy mitigations, 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!
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](https://searchfox.org/mozilla-central/rev/7fb9750a14e99267a77328559c2fd022e360d99b/dom/indexedDB/Key.cpp#786-826) that [ends up as code here](https://searchfox.org/mozilla-central/source/dom/indexedDB/Key.cpp#786-826) 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!

Back to Bug 1752045 Comment 2