Summary: | REGRESSION (r231107): Test http/tests/quicklook/same-origin-xmlhttprequest-allowed.html logs CSP failure | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Daniel Bates <dbates> | ||||
Component: | Tools / Tests | Assignee: | youenn fablet <youennf> | ||||
Status: | REOPENED --- | ||||||
Severity: | Normal | CC: | aestes, cdumez, ews-watchlist, japhet, lforschler, webkit-bug-importer, youennf | ||||
Priority: | P1 | Keywords: | InRadar, Regression | ||||
Version: | WebKit Local Build | ||||||
Hardware: | iPhone / iPad | ||||||
OS: | iOS 11 | ||||||
See Also: |
https://bugs.webkit.org/show_bug.cgi?id=186165 https://bugs.webkit.org/show_bug.cgi?id=186202 |
||||||
Bug Depends on: | 184741 | ||||||
Bug Blocks: | |||||||
Attachments: |
|
Description
Daniel Bates
2018-05-20 13:49:20 PDT
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. Created attachment 341580 [details]
Special casing quick look
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). 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. 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. 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. |