Closed Bug 1365157 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: DOM: Security, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: allstars.chh, Assigned: hchang)

References

Details

(Whiteboard: [domsecurity-active])

Attachments

(1 file)

No description provided.
Status: NEW → ASSIGNED
Priority: -- → P2
Assignee: allstars.chh → nobody
Status: ASSIGNED → NEW
Let assume this test is for iframe.contentDocument cross-origin assurance for data:uri.
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
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.
Assignee: nobody → hchang
Status: NEW → ASSIGNED
Attachment #8897271 - Flags: review?(ckerschb)
Summary: add tests for bug 1364369 to make sure data: URIs are cross origin → Add tests to make sure data: URIs are cross origin by iframe.contentDocument
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.
Attachment #8897271 - Flags: review?(ckerschb) → review+
James, just to make sure, do those updates within the json file look OK to you?
Flags: needinfo?(james)
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.
Flags: needinfo?(james)
(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.
(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)?
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)
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)
Pushed by hchang@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/67ce16cac163 wpt test cases to ensure 'data:' iframe is forbidden to access its contentDocument. r=ckerschb
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: