Open Bug 1670501 Opened 4 years ago Updated 4 years ago

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

Categories

(WebExtensions :: Storage, defect, P3)

defect

Tracking

(Not tracked)

People

(Reporter: Oriol, Unassigned)

Details

(Keywords: regressionwindow-wanted)

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.

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

Flags: needinfo?(oriol-bugzilla)

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

Flags: needinfo?(oriol-bugzilla)
Severity: -- → S3
Priority: -- → P3

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.

Assignee: nobody → oriol-bugzilla
Status: NEW → ASSIGNED

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.

Assignee: oriol-bugzilla → nobody
Status: ASSIGNED → NEW
You need to log in before you can comment on or make changes to this bug.