1365157 - Add tests to make sure data: URIs are cross origin by iframe.contentDocument

Status: RESOLVED FIXED

Description

[Yoshi Huang [:allstars.chh], Inactive]

Comment 1

[Henry Chang [:hchang]]

Let assume this test is for iframe.contentDocument cross-origin assurance for data:uri.

Comment 2

[Henry Chang [:hchang]]

Instead of throwing an exception, our implementation just return null
while accessing cross-domain iframe.contentDocument:

http://searchfox.org/mozilla-central/rev/e5b13e6224dbe3182050cf442608c4cb6a8c5c55/dom/html/nsGenericHTMLFrameElement.cpp#99

Comment 3

[Henry Chang [:hchang]]

Created attachment 8897271 [details]
Bug 1365157 - wpt test cases to ensure 'data:' iframe is forbidden to access its contentDocument.

Review commit: https://reviewboard.mozilla.org/r/168566/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/168566/

Comment 4

[Henry Chang [:hchang]]

https://elefant.github.io/data-uri/iframe.html

Chrome will throw a DOMException but Firefox won't. Safari will NOT
throw exception but will print error message like

Blocked a frame with origin "https://elefant.github.io" from accessing a frame with origin "null". The frame requesting access has a protocol of "https", the frame being accessed has a protocol of "data". Protocols must match.

Comment 5

[Henry Chang [:hchang]]

Comment on attachment 8897271 [details]
Bug 1365157 - wpt test cases to ensure 'data:' iframe is forbidden to access its contentDocument.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/168566/diff/1-2/

Comment 6

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8897271 [details]
Bug 1365157 - wpt test cases to ensure 'data:' iframe is forbidden to access its contentDocument.

https://reviewboard.mozilla.org/r/168566/#review174114

Hey Henry, test itself looks good, but I think we only have to update /origin-of-data-document.html inside the json, right? I know you probably ran some 'mach' command to update that json, but I am not entirely sure what implications that might have to update some other tests within that jons. Probably worth checking with jgraham.

Comment 7

[Christoph Kerschbaumer [:ckerschb]]

James, just to make sure, do those updates within the json file look OK to you?

Comment 8

[James Graham [:jgraham]]

Yeah, it's just updating file hashes; the value is just used to avoid some needless work updating the actual data, so it doesn't matter too much if it's wrong and we don't lint for correctness because it creates too many makework oranges.

Comment 9

[Henry Chang [:hchang]]

(In reply to James Graham [:jgraham] from comment #8)
> Yeah, it's just updating file hashes; the value is just used to avoid some
> needless work updating the actual data, so it doesn't matter too much if
> it's wrong and we don't lint for correctness because it creates too many
> makework oranges.

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #6)
> Comment on attachment 8897271 [details]
> Bug 1365157 - wpt test cases to ensure 'data:' iframe is forbidden to access
> its contentDocument.
> 
> https://reviewboard.mozilla.org/r/168566/#review174114
> 
> Hey Henry, test itself looks good, but I think we only have to update
> /origin-of-data-document.html inside the json, right? I know you probably
> ran some 'mach' command to update that json, but I am not entirely sure what
> implications that might have to update some other tests within that jons.
> Probably worth checking with jgraham.

Thanks for the review and yes I was told to run |./mach wpt-manifest-update|
even I didn't create a new file. I'll try to run command again to see if
I was doing anything wrong to result in those json changes other than
origin-of-data-document.html.

Comment 10

[Henry Chang [:hchang]]

(In reply to Henry Chang [:hchang] from comment #9)
> (In reply to James Graham [:jgraham] from comment #8)
> > Yeah, it's just updating file hashes; the value is just used to avoid some
> > needless work updating the actual data, so it doesn't matter too much if
> > it's wrong and we don't lint for correctness because it creates too many
> > makework oranges.
> 
> (In reply to Christoph Kerschbaumer [:ckerschb] from comment #6)
> > Comment on attachment 8897271 [details]
> > Bug 1365157 - wpt test cases to ensure 'data:' iframe is forbidden to access
> > its contentDocument.
> > 
> > https://reviewboard.mozilla.org/r/168566/#review174114
> > 
> > Hey Henry, test itself looks good, but I think we only have to update
> > /origin-of-data-document.html inside the json, right? I know you probably
> > ran some 'mach' command to update that json, but I am not entirely sure what
> > implications that might have to update some other tests within that jons.
> > Probably worth checking with jgraham.
> 
> Thanks for the review and yes I was told to run |./mach wpt-manifest-update|
> even I didn't create a new file. I'll try to run command again to see if
> I was doing anything wrong to result in those json changes other than
> origin-of-data-document.html.

I still got a tons of hash changes in the json files. Probably people don't
usually run the update command so that so many hash values have been out
of sync. So, should I leave those hash value changes (even some of them
have nothing to do with my patch)?

Comment 11

[Henry Chang [:hchang]]

Comment on attachment 8897271 [details]
Bug 1365157 - wpt test cases to ensure 'data:' iframe is forbidden to access its contentDocument.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/168566/diff/2-3/

Comment 12

[Mozilla Autoland]

hg error in cmd: hg push -r . -f try: pushing to ssh://hg.mozilla.org/try
searching for changes
remote: abort: abandoned transaction found!
remote: (run 'hg recover' to clean up transaction)
abort: stream ended unexpectedly (got 0 bytes, expected 4)

Comment 13

[Mozilla Autoland]

hg error in cmd: hg push -r . -f try: pushing to ssh://hg.mozilla.org/try
searching for changes
remote: abort: abandoned transaction found!
remote: (run 'hg recover' to clean up transaction)
abort: stream ended unexpectedly (got 0 bytes, expected 4)

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/67ce16cac163
wpt test cases to ensure 'data:' iframe is forbidden to access its contentDocument. r=ckerschb

Comment 15

[Wes Kocher (:KWierso)]

https://hg.mozilla.org/mozilla-central/rev/67ce16cac163