1670501 - Inconsistencies with storage.StorageArea.get({prop: undefined})

Status: NEW

(WebExtensions :: Storage, defect, P3)

Description

[Oriol Brufau [:Oriol]]

While writing a webext mockup I noticed some inconsistencies when calling get() on a StorageArea and passing an object with an undefined value.

chrome.storage.local.set({a: 1}, () => {
  chrome.storage.local.get({a: undefined, b: undefined}, console.log);
});

Firefox: {a: 1}
Chrome: {}

chrome.storage.sync.set({a: 1}, () => {
  chrome.storage.sync.get({a: undefined, b: undefined}, console.log);
});

Firefox: {a: 1, b: null}
Chrome: {}

I think it makes more sense to return {a: 1} instead of {} like Chrome, but the b: null in the sync storage makes no sense.

Comment 1

[Tomislav Jovanovic :zombie]

Thanks Oriol, is this a regression, did this change with the new Sync implementation from earlier this year?

Comment 2

[Oriol Brufau [:Oriol]]

Doesn't seem so, I can reproduce in 66.0a1 (2019-01-17), maybe earlier.

Comment 3

[Oriol Brufau [:Oriol]]

Seems the problem is https://searchfox.org/mozilla-central/rev/b757a23d312f5b154887b870447ddfeae8cfcc36/toolkit/components/extensions/ExtensionStorage.jsm#171-179

  sanitize(value, context) {
    let json = context.jsonStringify(value === undefined ? null : value);
    if (json == undefined) {
      throw new ExtensionUtils.ExtensionError(
        "DataCloneError: The object could not be cloned."
      );
    }
    return JSON.parse(json);
  },

It converts undefined values to null. Should be easy to fix.

Comment 4

[Oriol Brufau [:Oriol]]

OK, so the problem is more complex than what I thought. It's not just undefined, it involves all values that are not JSON-serializable.

• For reference, there is interoperability with serializable primitives: you get the same exact value.

  var number = 123;
  chrome.storage.local.get({number}, data => console.log(data, data.number, data.number === number));
  

  Firefox: Object { number: 123 } 123 true. Chromium: {number: 123} 123 true.

  var number = 123;
  chrome.storage.sync.get({number}, data => console.log(data, data.number, data.number === number));
  

  Firefox: Object { number: 123 } 123 true. Chromium: {number: 123} 123 true.

• Also for non-cyclic objects: you get a clone

  var object = {a:1};
  chrome.storage.local.get({object}, data => console.log(data, data.object, data.object === object));
  

  Firefox: Object { object: {…} } Object { a: 1 } false. Chromium: {object: {…}} {a: 1} false

  var object = {a:1};
  chrome.storage.sync.get({object}, data => console.log(data, data.object, data.object === object));
  

  Firefox: Object { object: {…} } Object { a: 1 } false. Chromium: {object: {…}} {a: 1} false

• But things get interesting with non-serializable primitives. With big ints:

  var bigint = 1n;
  chrome.storage.local.get({bigint}, data => console.log(data, data.bigint, data.bigint === bigint));
  

  Firefox: Object { bigint: 1n } 1n true. Chromium: {} undefined false

  var bigint = 1n;
  chrome.storage.sync.get({bigint}, data => console.log(data, data.bigint, data.bigint === bigint));
  

  Firefox: Error: can't access property "bigint", data is undefined + Error: BigInt value can't be serialized in JSON
  Chromium: {} undefined false

  Basically, Chromium just excludes the bigint property. Firefox included it for local storage, but sync storage fails completely.

• It's similar for symbols:

  var symbol = Symbol('s');
  chrome.storage.local.get({symbol}, data => console.log(data, data.symbol, data.symbol === symbol));
  

  Firefox: Object { symbol: Symbol(s) } undefined false + NS_ERROR_FAILURE [nsIXPCComponents_Utils.cloneInto]
  Chromium: {} undefined false

  var symbol = Symbol('s');
  chrome.storage.sync.get({symbol}, data => console.log(data, data.symbol, data.symbol === symbol));
  

  Firefox: Error: can't access property "symbol", data is undefined + Error: DataCloneError: The object could not be cloned
  Chromium: {} undefined false

  Again, Chromium just excluded the symbol property. In Firefox, local storage kind of works, but Cu.cloneInto fails and then the extension gets an object from another compartment, with security wrappers, and can't read the properties. Sync storage fails completely.

• Finally, with cyclic objects:

  var object = {foo: {}};
  object.foo.bar = object;
  chrome.storage.local.get({object}, data => console.log(data, data.object, data.object === object, data.object.foo.bar === data.object));
  

  Firefox: Object { object: {…} } Object { foo: {…} } false true
  Chromium: {object: {…}} {foo: {…}} false false

  var object = {foo: {}};
  object.foo.bar = object;
  chrome.storage.sync.get({object}, data => console.log(data, data.object, data.object === object, data.object.foo.bar === data.object));
  

  Firefox: Error: can't access property "object", data is undefined + Error: cyclic object value
  Chromium: {object: {…}} {foo: {…}} false false

  So Chromium kind of clones the object, but replacing cyclic references with null, i.e. {foo: {bar: null}}.
  In Firefox, local storage clones the object, preserving the cyclic structure. Sync storage completely fails.

I don't know how all of this is supposed to behave, so unassigning myself.