Looks like they're not enumerable in Blink/WebKit, but are enumerable in Gecko/Edge. Other indexed objects have the indexed props enumerable, so why not WindowProxy?
Note that I was going to add a wpt for the behavior here initially, but holding off on that for now because I think the spec is actually wrong here.
https://www.w3.org/Bugs/Public/show_bug.cgi?id=20701#c144 ctrl+F enumerable says
Yep, that's the reason. This stuff is normally enumerable, but we decided to
make it non-enumerable for cross-origin objects. We still have to list them
for [[GetOwnPropertyKeys]] though, so we could also just make them
enumerable. It doesn't matter much.
So that at least explains things, I guess.
It does seem nicer to be consistent. Any thoughts, @yuki3 / @cdumez?
Non-enumerable in the cross-origin case makes sense. But why are they non-enumerable in the same-origin case?
I mean, presumably for consistency along that axis...
I should note, by the way, that the quote above is talking about the cross-origin-exposed IDL properties, not the indexed ones.
Could you guys check my understanding?
LegacyPlatformObjectGetOwnProperty defines non-WindowProxy case and it says that indexed properties must have {enumerable: true}.
WindowProxy's [[OwnPropertyKeys]] lists up indexed properties as same as OrdinaryOwnPropertyKeys, so it looks like enumerable from this point of view.
IIUC, it's quite natural, consistent and intuitive that WindowProxy's indexed properties are enumerable. I support this change.
Just FYI, (maybe this is not so relevant) Blink has another bug about WindowProxy's indexed properties:
https://bugs.chromium.org/p/chromium/issues/detail?id=695069
Probably, enumerable is something we can fix along with the above bug.
WindowProxy's [[OwnPropertyKeys]] lists up indexed properties as same as OrdinaryOwnPropertyKeys, so it looks like enumerable from this point of view.
These two things are pretty unrelated. [[OwnPropertyKeys]] just lets you know the result of Object.getOwnPropertyNames(); both enumerable and un-enumerable properties show up there.
So there are several things that are currently in conflict:
If we make the change proposed here, of switching same-origin indexed properties to enumerable, this becomes:
This seems like a downgrade to me.
What would make things very consistent is making both named and indexed properties always enumerable. Alternately we could make named properties for WindowProxy enumerable same-origin too, which still would leave cross-origin WindowProxy objects as inconsistent, but at least confine that.
Consistency of WindowProxy vs. other platform objects (non-enumerable for named, enumerable for indexed, vs. enumerable)
@domenic I'm not sure what you mean here. When you say "named", do you mean IDL operations/attributes, or the properties on the NamedPropertiesObject (not the WindowProxy)?
If you mean the former, they are enumerable on same-origin windows (just like on all other platform objects) and non-enumerable on cross-origin ones. So your consistency descriptions don't look correct to me at all, for either the "before" or the "after" case, if these are the things you're talking about.
If you mean the things on the NamedPropertiesObject, those don't exist at all in the cross-origin case. In the same-origin case they are non-enumerable because the interface uses LegacyUnenumerableNamedProperties, but note that this is all about the NamedPropertiesObject, not the WindowProxy itself.
Alternately we could make named properties for WindowProxy enumerable same-origin too
Again, I'd like to understand which properties you mean by "named properties" here. The WindowProxy does not have any default non-enumerable properties with non-integer names in the spec right now, in the same-origin case.
Sorry, that definitely was confusing. I was referring to step 6 of https://html.spec.whatwg.org/multipage/window-object.html#windowproxy-getownproperty, i.e. the named browsing context properties.
Oh, we actually expose some subset of the NamedPropertiesObject, in the cross-origin case? I had forgotten about that...
I think those were supposed to be non-enumerable to avoid a cross-origin information leak. But we have that leak anyway, because ES added ways to get lists of non-enumerable properties. :(
I'm now catching up... Then, we need to consider a) WindowProxy versus other platform objects, b) same-origin case versus cross-origin case, and c) indexed properties versus named properties (including cross-origin's subset case).
If I'm correctly understanding the current status,
platform object, (same-origin), indexed property = enumerable
WindowProxy, same-origin, named property = enumerable
Are these correct?
And are we aiming the following state?
If so, it looks reasonable.
And are we aiming the following state?
I think that is the idea @bzbarsky proposes. I don't quite know why being cross-origin should make something non-enumerable, so I offered the alternative that everything should be enumerable, cross- or same-origin. How do people feel about that?
(I don't feel strongly.)
Again, the original intent of non-enumerability was to avoid cross-origin information leaks... Then ES added other ways that information leaks. :(
Again, the original intent of non-enumerability was to avoid cross-origin information leaks...
The document-tree child browsing context name property set is not leak, I guess? It must be either of a) same-origin case, or b) case that browsing context container's name matches with child browsing context name. So, either way, we already knew the names.
Then ES added other ways that information leaks.
I don't know exactly which part of ES leaks, however, assuming that it's okay to leak the document-tree child browsing context name property set because the call site already knew their names, it seems okay to me anyway.
As long as no security risk, @domenic's proposal looks good to me.
So, either way, we already knew the names.
No, we didn't. The relevant case is window A getting the "document-tree child browsing context name property set" of window B, where A and B are different-origin but all the subframes of B are same-origin with B.
I don't know exactly which part of ES leaks
Object.getOwnPropertyNames.
Hmm, I see. Then, what do we want to do? I don't have a strong opinion, but @domenic's proposal seems still acceptable given that Object.GetOwnPropertyNames tells the names anyway.
If we could figure out some way to have the names not show up in getOwnPropertyNames but still work via [[Get]] that would be ideal from a security point of view. Assuming that they do need to work from [[Get]], that is.
Got it. For the meanwhile (until we find the ideal solution), do we want to make the indexed properties enumerable only when it's same-origin? I'm fine with it.
If we could figure out some way to have the names not show up in getOwnPropertyNames but still work via [[Get]] that would be ideal from a security point of view.
I mean, we can define [[OwnPropertyKeys]]() to do whatever we want. (It appears it would not violate any JS invariants since the properties are configurable.)
Is that web-compatible? Is it worth the churn?
And what about the indexed property names; should we make those enumerable (both same- and cross-origin)?
Is that web-compatible?
Unknown. What do UAs actually do? For example, do they all even expose the named stuff in getOwnPropertyNames on cross-origin windows?
Is it worth the churn?
If we can manage to do it, then imo yes. Cross-origin information leaks are generally considered a very bad thing!
And what about the indexed property names
For the indexed ones, the security argument doesn't apply because you can enumerate them in the obvious way: start at 0 and keep doing [[Get]] until undefined comes back, right? So I would have no problem with just making them enumerable in both cases.
OK. It sounds like we have an idea of where we'd like the spec to end up. I will put together a PR, but I would appreciate if someone could work on web platform tests. Doing the test work should also help us understand where browsers are and thus the potential web compatibility.
@annevk do you have any opinions on this? For the TL;DR version of where we ended up, see #2777.
I'm also still hoping someone will write tests for me, but I haven't seen any signs of movement there yet...
In general this all seemed reasonable. I can probably write tests next week. Might need another ping.
Tests are at https://github.com/w3c/web-platform-tests/pull/6538. It turns out that no browser makes these properties enumerable currently and named properties were already hidden (as per PR). I guess we still want to try to change all browsers then to make them enumerable (which also makes Object.keys() work)? I'm happy to file all the relevant browser bugs provided everyone is on board.
@annevk I'm not sure what "these properties" refers to in your last comment. Is it talking about what this issue is filed on (indexed properties on same-origin WindowProxy), or something else?
Making all properties (indexed, named, and the safelisted bunch), same-origin and cross-origin, enumerable. (Although same-origin there's no safelist and named properties live on a prototype object.) Coupled with making [[OwnPropertyKeys]]() not return any named properties in the cross-origin case.
That's what #2777 proposes.
That seems pretty reasonable to me.
Thanks for helping finish this up, @annevk!
Are the tests correct?
https://github.com/w3c/web-platform-tests/pull/6538#issuecomment-315853042
Noticed this when trying to align WebKit.
Created https://github.com/w3c/web-platform-tests/pull/6571 as a follow-up for the tests. Thanks!
Most helpful comment
In general this all seemed reasonable. I can probably write tests next week. Might need another ping.