New issue
Advanced search Search tips

Issue 698564 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Structured cloning should fail with WeakMap, WeakSet, and Promises

Reported by bret...@gmail.com, Mar 5 2017

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.87 Safari/537.36

Steps to reproduce the problem:
1. Try the following:

var caught = false;
try {
    postMessage(new Promise(() => {}), '*');
    postMessage(new WeakMap(), '*');
    postMessage(new WeakSet(), '*');
} catch (err) {caught = true;}
console.log(caught); // Gives false

What is the expected behavior?
The console should report `true` for any of the items but does not fail with any of them.

What went wrong?
The StructuredClone algorithm at https://html.spec.whatwg.org/multipage/infrastructure.html#structuredclone mentions in step 18 that 

"...if input has any internal slot other than [[Prototype]] or [[Extensible]], then throw a "DataCloneError" DOMException.
For instance, a [[PromiseState]] or [[WeakMapData]] internal slot."

However, as per the above example, `postMessage` (which uses this algorithm) does not throw an exception in Chrome for `Promise` objects (which have an extra slot ([[PromiseState]]) nor for `WeakMap` objects (which have an extra slot, [[WeakMapData]]) or `WeakSet` objects (which have an extra slot, [[WeakSetData]]).

Did this work before? N/A 

Does this work in other browsers? Yes

Chrome version: 56.0.2924.87  Channel: n/a
OS Version: 10.0
Flash Version: 

Firefox 51.0.1 correctly throws with `WeakMap` and `WeakSet` but mistakenly does not throw with a `Promise`.

 

Comment 1 by bret...@gmail.com, Mar 5 2017

Oh, and also, passing `Object.prototype` as in the following:

    postMessage(Object.prototype, '*');

...should fail (though it incorrectly does not, though neither does it do so in Firefox) as it is a (non-array) exotic object (arrays are permitted) and these are to fail per step 19 of the above-mentioned algorithm:

> Otherwise, if input is an exotic object, then throw a "DataCloneError" DOMException."
>
> For instance, a proxy object.
Cc: jbroman@chromium.org
Components: Blink>Bindings
Labels: -Type-Bug -Pri-2 M-57 OS-Linux OS-Mac Pri-1 Type-Bug-Regression
Owner: jbroman@chromium.org
Status: Assigned (was: Unconfirmed)
Able to reproduce the issue on Windows 10, Mac 10.12.3 and Ubuntu 14.04 using reported version #56.0.2924.87 but the same is not reproducible in the latest canary #59.0.3033.0.

Reverse Bisect Information:
=====================
Good build: 57.0.2936.0  Revision(434845)

Bad Build : 57.0.2935.0  Revision(434592)

Change Log URL: 
https://chromium.googlesource.com/chromium/src/+log/ce1c7996be64c3af7243256c5032f11aef6fe7cd..19b847a9301f7139b7ff85cb32eb13e514fa1ab1

From the above change log possible CL that fixed this issue:

Review-Url: https://codereview.chromium.org/2517813002

jbroman@ - Could you please check and merge the fix to M57 if it is a valid candidate.

Thanks...!!

Comment 4 Deleted

Labels: -Type-Bug-Regression -M-57 Type-Bug
Status: Fixed (was: Assigned)
Regarding the original report: this isn't a regression, but a longstanding compat bug. It was fixed in M57 (along with performance improvements) as part of an update to this feature. I share your interest in these objects throwing exceptions, and as of Chrome 57, they do.

#1: I don't believe Object.prototype is exotic; AFAIK it's an ordinary object. Though it might be an odd thing to serialize, I don't think the specification requires it to fail. If the spec or another vendor implements this otherwise, we can look at the possibility of a compat issue here, but this seems okay to me.

#3: Yes, that's the CL that fixed the issue. No merge to M57 is needed as that CL is already included in M57. No merge to M56 is needed because this is not a critical security/stability fix.

This fix should reach the stable channel with M57, in the next few weeks.

Comment 6 by bret...@gmail.com, Mar 9 2017

From 19.1.3 at https://tc39.github.io/ecma262/#sec-properties-of-the-object-prototype-object :

"The Object prototype object is an immutable prototype exotic object."

where an "immutable prototype exotic object" (which is only used, at least within the spec, to refer to `Object.prototype`) is defined in section 9.4.7 at https://tc39.github.io/ecma262/#sec-immutable-prototype-exotic-objects as follows:

"An immutable prototype exotic object is an exotic object that has a [[Prototype]] internal slot that will not change once it is initialized."
Ah, interesting. I'd been looking at ECMA-262, 6th edition, so it seems that this was a change since then (and was presumably not contemplated by the author of the HTML spec wording).

The particular way it's exotic doesn't cause any problems for cloning (__proto__ isn't serialized anyhow), but I'll look into it. For prioitization, I assume this isn't breaking any applications, and is just a spec-vs-implementation concern?
Filed the immutable prototype case as https://bugs.chromium.org/p/chromium/issues/detail?id=699831. You may star that if you wish to follow its progress. The fix for that is unlikely to be high priority, so it will ship in M59 at the earliest.

Comment 9 by bret...@gmail.com, Mar 9 2017

Yes, only a spec-vs-implementation concern, not breaking any applications except that we'd like to know whether our ES-based implementation of the cloning algorithm ought to throw for such.

(Our ES-based structured cloning implementation is within `typeson-registry` at https://github.com/dfahlander/typeson-registry and our browser/Node shim for IndexedDB (IndexedDBShim at https://github.com/axemclion/IndexedDBShim ) is built on top of this, and it might conceivably be used for the likes of `postMessage` polyfills, within jsdom or such.)

Comment 10 Deleted

Comment 11 by bret...@gmail.com, Mar 9 2017

FWIW, any ES-implementation such as ours is currently inherently limited as it is anyways (e.g., no apparent way in the browser to distinguish a Proxy for rejection by the algorithm; if sync XHR is abandoned, we may not be able to properly throw synchronously for Blob/File failures as required by IndexedDB methods; `Object.prototype.toString` is not fail-safe for "class" detection nor can we directly get at slots like [[Clone]] or programmatically find those with additional slots; etc.).

Sign in to add a comment