Closed
Bug 862380
Opened 11 years ago
Closed 11 years ago
Enumerating cross-origin objects shouldn't throw
Categories
(Core :: XPConnect, defect)
Tracking
()
RESOLVED
FIXED
mozilla24
People
(Reporter: bholley, Assigned: bholley)
Details
Attachments
(4 files, 1 obsolete file)
4.40 KB,
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
1.37 KB,
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
1.09 KB,
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
2.31 KB,
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
By my reading of the spec, enumerating cross-origin objects shouldn't throw, and we should instead return the appropriate properties, with the non-cross-origin-accessible properties filtered out. We have the capability to do that in FilteringWrapper, but currently just throw in any situations where we try to invoke traps like |enumerate| or |keys|. If this is the spec behavior, I'm happy to try to align with it. Hixie, can you confirm that this is the case?
Comment 1•11 years ago
|
||
For Window, Document, and Location (I think those are the only objects you can access cross-origin without having once been same-origin), that sounds right. I don't think the spec actually documents this right now, though. What do browsers do?
Assignee | ||
Comment 2•11 years ago
|
||
(In reply to Hixie (not reading bugmail) from comment #1) > For Window, Document, and Location (I think those are the only objects you > can access cross-origin without having once been same-origin), that sounds > right. I don't think the spec actually documents this right now, though. > What do browsers do? It appears that Safari and Chrome don't throw, whereas IE9 and Gecko do. I haven't tested IE10.
Comment 3•11 years ago
|
||
Do they enumerate anything? I really don't mind what we spec here, so long as we're consistent. Throwing seems unhelpful, but an argument could be made that enumerating frame names is a security risk, so we probably shouldn't do that even if we allow access to the names if you need them? I don't know.
Assignee | ||
Comment 4•11 years ago
|
||
(In reply to Hixie (not reading bugmail) from comment #3) > Do they enumerate anything? It looks like they don't, and Object.getOwnPropertyNames and such provide arrays of length zero. > I really don't mind what we spec here, so long as we're consistent. Throwing > seems unhelpful, but an argument could be made that enumerating frame names > is a security risk, so we probably shouldn't do that even if we allow access > to the names if you need them? Yeah, I think it's safest not to enumerate them. I'm going to upload some patches to align with Chrome/Safari here, and file a spec bug.
Assignee | ||
Comment 5•11 years ago
|
||
Attachment #751995 -
Flags: review?(mrbkap)
Assignee | ||
Comment 6•11 years ago
|
||
We already do this, but it's helpful to be clear about it.
Attachment #751996 -
Flags: review?(mrbkap)
Assignee | ||
Comment 7•11 years ago
|
||
Attachment #751997 -
Flags: review?(mrbkap)
Assignee | ||
Comment 8•11 years ago
|
||
Attachment #751998 -
Flags: review?(mrbkap)
Assignee | ||
Comment 9•11 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=4f832df35e6e Spec bug: https://www.w3.org/Bugs/Public/show_bug.cgi?id=22102
Assignee | ||
Comment 10•11 years ago
|
||
Looks green.
Updated•11 years ago
|
Attachment #751995 -
Flags: review?(mrbkap) → review+
Comment 11•11 years ago
|
||
Comment on attachment 751996 [details] [diff] [review] Part 2 - Be more explicit about rejecting JSID_VOID for XOWs. v1 Ironically, I had to look up the meaning of EIBTI.
Attachment #751996 -
Flags: review?(mrbkap) → review+
Updated•11 years ago
|
Attachment #751997 -
Flags: review?(mrbkap) → review+
Comment 12•11 years ago
|
||
Comment on attachment 751998 [details] [diff] [review] Part 4 - Tests. v1 Review of attachment 751998 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/xpconnect/tests/mochitest/test_bug862380.html @@ +23,5 @@ > + is(Object.getOwnPropertyNames(obj).length, 0, "Object.getOwnPropertyNames gives empty array"); > + is(Object.keys(obj).length, 0, "Object.keys gives empty array"); > + for (var i in obj) > + ok(false, "Enumerated something: " + i); > + ok(!((obj instanceof Window) || (obj instanceof Location)), "Instanceof doesn't do anything"); I was with you until here. Why should instanceof lie? It's pretty easy to figure out what kind of cross-origin object you have your hands on anyway.
Attachment #751998 -
Flags: review?(mrbkap)
Assignee | ||
Comment 13•11 years ago
|
||
(In reply to Blake Kaplan (:mrbkap) from comment #12) > I was with you until here. Why should instanceof lie? It's pretty easy to > figure out what kind of cross-origin object you have your hands on anyway. Ok, sure. I was just treating this as documenting our existing behavior, but you're right that said behavior is wrong. I'll remove that check and fix this in bug 870423.
Assignee | ||
Comment 14•11 years ago
|
||
Attachment #751998 -
Attachment is obsolete: true
Attachment #753095 -
Flags: review?(mrbkap)
Updated•11 years ago
|
Attachment #753095 -
Flags: review?(mrbkap) → review+
Assignee | ||
Comment 15•11 years ago
|
||
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/690f5224fb99 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/83ba168c4d7e remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/2bc3c9b632aa remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/464c47889efe
Comment 16•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/690f5224fb99 https://hg.mozilla.org/mozilla-central/rev/83ba168c4d7e https://hg.mozilla.org/mozilla-central/rev/2bc3c9b632aa https://hg.mozilla.org/mozilla-central/rev/464c47889efe
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in
before you can comment on or make changes to this bug.
Description
•