Bug 185807 – REGRESSION (r231107): Test http/tests/quicklook/same-origin-xmlhttprequest-allowed.html logs CSP failure

• Component: Tools / Tests
• Status: REOPENED

[Daniel Bates]

Before changeset <https://trac.webkit.org/changeset/231107/> (bug #184741) the expected result for test http/tests/quicklook/same-origin-xmlhttprequest-allowed.html was:

[[
CONSOLE MESSAGE: line 1: PASS: XMLHttpRequest allowed


--------
Frame: '<!--framePath //<!--frame0-->-->'
--------
Run test
]]
<https://trac.webkit.org/browser/webkit/trunk/LayoutTests/http/tests/quicklook/same-origin-xmlhttprequest-allowed-expected.txt?rev=214189&format=txt>

After changeset r231107 the result of this test changed (and unfortunately these new results were committed):

[[
CONSOLE MESSAGE: Blocked by Content Security Policy
CONSOLE MESSAGE: XMLHttpRequest cannot load about: due to access control checks.
CONSOLE MESSAGE: line 1: PASS: XMLHttpRequest allowed


--------
Frame: '<!--framePath //<!--frame0-->-->'
--------
Run test
]]
<https://trac.webkit.org/browser/webkit/trunk/LayoutTests/http/tests/quicklook/same-origin-xmlhttprequest-allowed-expected.txt?rev=231107&format=txt>

[Radar WebKit Bug Importer]

<rdar://problem/40402483>

[youenn fablet]

The test is trying to load a quick look URL using XHR.
In that particular case, ResourceLoader::willSendRequestInternal will call previewConverter->safeRequest. The request URL is then updated and becomes "about:".

Before moving the checks to NetworkProcess, DocumentThreadableLoader was not doing any further check since the request URL was the quick look URL and not "about:", thus the request is considered same-origin.

Now that checks are done in NetworkProcess, "about:" is used for the checks.
For this particular quick look + "about:" URL, it is easy to fix it in WebLoaderStrategy by disabling network checks in that particular case.

The same issue might arise if an injected bundle decides to change the request URL on the fly.

[youenn fablet]

Created attachment 341580 [details]
Special casing quick look

[youenn fablet]

Uploaded a patch that is fixing iOS-sim to the previous behavior for the given test.
Except if we end up in big compatibility issues, I am not sure we should actually try to solve the issue where some layer (quick look, injected bundles) below our previous security checks are changing the request (URL or headers).

[youenn fablet]

I talked with andy about it.
The URL is converted from a quicklook URL to about: because it is not pointing to a valid resource. It is fine to keep the current behavior for now and make the load fails.

That said, we are no longer checking that CORS checks are passing in Network Process in case of a valid quick look URL.
We should add a test for that case.
I will file a separate bug entry for this particular issue and mark this one as WontFix.

[Daniel Bates]

Re-opening this bug to update the test to actually check if the load was allowed or blocked and add a FIXME comment to determine a valid QuickLook attachment URL to use when making the XHR request. Currently the test considers XHR completion regardless of error as success.

[Daniel Bates]

The test <https://trac.webkit.org/browser/webkit/trunk/LayoutTests/http/tests/quicklook/same-origin-xmlhttprequest-allowed.html> does not seem materially consistent with example given in <rdar://problem/29898214>. We need a document Q_i that when QuickLook converted has a non-empty sub-QuickLook attachment Q_i_0. Then we should modify same-origin-xmlhttprequest-allowed.html to fetch Q_i_o via XHR and ensure it loaded the contents of Q_i_0.