Closed Bug 1397656 Opened 7 years ago Closed 7 years ago

Update tests within editor/ to comply with new toplevel data: URI navigation policy

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: ckerschb, Assigned: ckerschb)

References

Details

(Whiteboard: [domsecurity-active])

Attachments

(1 file)

      No description provided.
Blocks: 1380959
Priority: -- → P3
Whiteboard: [domsecurity-backlog1]
Masayuki, we are about to block toplevel data: URI navigations within Firefox (because those navigations are mostly only used for phishing attempts). Flipping the pref |security.data_uri.block_toplevel_data_uri_navigations| causes the following tests within editor/ to fail:

* editor/libeditor/tests/browser_bug527935.js
* editor/libeditor/tests/test_bug635636.html
* editor/libeditor/tests/test_bug966155.html
* editor/libeditor/tests/test_bug966552.html

Please note that the only change I performed for those tests is to load them from an external file instead of using a data: URI.
Attachment #8905455 - Flags: review?(masayuki)
Assignee: nobody → ckerschb
Status: NEW → ASSIGNED
Priority: P3 → P1
Whiteboard: [domsecurity-backlog1] → [domsecurity-active]
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #1)
> Masayuki, we are about to block toplevel data: URI navigations within
> Firefox (because those navigations are mostly only used for phishing
> attempts).

Oh, then, cannot load simple testcase as data URI even if inputting it in the URL bar even on Nightly??
Comment on attachment 8905455 [details] [diff] [review]
bug_1397656_editor_tests.patch

># HG changeset patch
># User Christoph Kerschbaumer <ckerschb@christophkerschbaumer.com>
># Date 1504781468 -7200
>#      Thu Sep 07 12:51:08 2017 +0200
># Node ID 5e11f845f965d8515e6a6b959f1b300ca14a989b
># Parent  575a7492d497a02c32f270a80a9356d6ab78e94b
>Bug 1397656 - Update tests within editor/ to comply with new toplevel data: URI navigation policy. r=masayuki
>
>diff --git a/editor/libeditor/tests/browser.ini b/editor/libeditor/tests/browser.ini
>--- a/editor/libeditor/tests/browser.ini
>+++ b/editor/libeditor/tests/browser.ini
>@@ -1,6 +1,8 @@
> [browser_bug527935.js]
> skip-if = toolkit == 'android'
>-support-files = bug527935.html
>+support-files =
>+  bug527935.html
>+  bug527935_2.html

I like "file_" or "window_" or "frame_" prefixed file name better. However, following test's support file doesn't have any prefix. I guess, browser chrome tests don't use such prefix... Okay, keep using it.
Attachment #8905455 - Flags: review?(masayuki) → review+
(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #2)
> Oh, then, cannot load simple testcase as data URI even if inputting it in
> the URL bar even on Nightly??

We only block navigations to data: URIs, e.g. through click, window.open, window.location. When inputting a data: URI directly in the URL bar it will continue to work.
Pushed by mozilla@christophkerschbaumer.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f42e4a61d6a5
Update tests within editor/ to comply with new toplevel data: URI navigation policy. r=masayuki
Backed out for failing modified mochitest test_bug966552.html:

https://hg.mozilla.org/integration/mozilla-inbound/rev/78ab7b444a9c125ead34fae6777021d8103571d7

First push with ran failing tests: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=6e4988e39120b0a1f7a7eeeb872c4781f9d5d9ee&filter-resultStatus=exception&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=retry&filter-resultStatus=testfailed&filter-resultStatus=busted
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=129301471&repo=mozilla-inbound

17:55:09     INFO -  1392 INFO TEST-START | editor/libeditor/tests/test_bug966552.html
17:55:10     INFO -  TEST-INFO | started process screenshot
17:55:10     INFO -  TEST-INFO | screenshot: exit 0
17:55:10    ERROR -  1393 INFO TEST-UNEXPECTED-FAIL | editor/libeditor/tests/test_bug966552.html | undefined assertion name - got "st\n", expected "st"
17:55:10     INFO -      SimpleTest.is@SimpleTest/SimpleTest.js:312:5
17:55:10     INFO -      runTest/<@editor/libeditor/tests/test_bug966552.html:40:5
17:55:10     INFO -      focusedOrLoaded/<@SimpleTest/SimpleTest.js:795:59
Flags: needinfo?(ckerschb)
(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #6)
> Backed out for failing modified mochitest test_bug966552.html:

Thanks, I'll take a look.
Flags: needinfo?(ckerschb)
Since we are loading from an external file it seems that body.textContent includes the \n, hence calling .trim() before doing the comparison. Also, since we are loading the data: URI from an external file now, there is no need for setting
> ["security.data_uri.unique_opaque_origin", false]]}, nextTest);

I also removed that.
TRY looks good:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9fe2deb311fe28e7e43e0616ddf9368a1ce555d7&selectedJob=129589335
Pushed by mozilla@christophkerschbaumer.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a85670ee5e95
Update tests within editor/ to comply with new toplevel data: URI navigation policy. r=masayuki
https://hg.mozilla.org/mozilla-central/rev/a85670ee5e95
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.