Bug 185443 - REGRESSION (r231479): http/tests/appcache/x-frame-options-prevents-framing.php is timing out
Summary: REGRESSION (r231479): http/tests/appcache/x-frame-options-prevents-framing.ph...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P1 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar
Depends on: 185410
Blocks:
  Show dependency treegraph
 
Reported: 2018-05-08 13:21 PDT by Daniel Bates
Modified: 2018-05-09 15:03 PDT (History)
7 users (show)

See Also:


Attachments
Patch (2.30 KB, patch)
2018-05-09 14:43 PDT, Daniel Bates
aestes: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2018-05-08 13:21:43 PDT
Following <https://trac.webkit.org/changeset/231479> (bug #185410) the test http/tests/appcache/x-frame-options-prevents-framing.php is timing out. Here is the diff output:

[[
--- /Volumes/.../OpenSource/WebKitBuild/Debug/layout-test-results/http/tests/appcache/x-frame-options-prevents-framing-expected.txt
+++ /Volumes/.../OpenSource/WebKitBuild/Debug/layout-test-results/http/tests/appcache/x-frame-options-prevents-framing-actual.txt
@@ -1,10 +1,5 @@
 CONSOLE MESSAGE: line 1: ApplicationCache is deprecated. Please use ServiceWorkers instead.
-CONSOLE MESSAGE: Refused to display 'http://127.0.0.1:8000/appcache/x-frame-options-prevents-framing.php' in a frame because it set 'X-Frame-Options' to 'deny'.
-The following iframe is a document that was cached in the application cache.
-It also had "x-frame-options: deny" set, so it should not actually show up in the iframe
+CONSOLE MESSAGE: line 1: ApplicationCache is deprecated. Please use ServiceWorkers instead.
+#PID UNRESPONSIVE - com.apple.WebKit.WebContent.Development (pid 75567)
+FAIL: Timed out waiting for notifyDone to be called
 
-
---------
-Frame: '<!--frame1-->'
---------
-
]]
Comment 1 youenn fablet 2018-05-08 13:50:14 PDT
FWIW, DTL is only skipping its security checks if the response source is network process. See isResponseComingFromNetworkProcess in DocumentThreadableLoader.cpp
Comment 2 Radar WebKit Bug Importer 2018-05-09 11:54:32 PDT
<rdar://problem/40100660>
Comment 3 Daniel Bates 2018-05-09 14:18:13 PDT
 The issue is that loads for ApplicationCache go through DocumentLoader::responseReceived(). So, we need to process CSP frame-ancestors and X-Frame-Options regardless of whether we are using WebKit2 and experimental feature Restricted HTTP Response Access is enabled. Although the fix for this issue would likely fallout naturally from fixing bug #185412. I do not see the need to gate fixing this bug on fixing bug #185412.
Comment 4 Daniel Bates 2018-05-09 14:43:02 PDT
Created attachment 340032 [details]
Patch
Comment 5 Daniel Bates 2018-05-09 14:48:52 PDT
Committed r231597: <https://trac.webkit.org/changeset/231597>
Comment 6 youenn fablet 2018-05-09 15:03:57 PDT
Comment on attachment 340032 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=340032&action=review

> Source/WebCore/loader/DocumentLoader.cpp:771
> +    if (m_substituteData.isValid() || !m_frame->settings().networkProcessCSPFrameAncestorsCheckingEnabled() || !RuntimeEnabledFeatures::sharedFeatures().restrictedHTTPResponseAccess()) {

I believe that we are currently skipping CSP checks if the response is coming from Memory Cache or from Service Worker.
We should probably fix that.

As I said previously, DocumentThreadableLoader is disabling CSP checks only if the response is coming from NetworkProcess and if platformStrategies()->loaderStrategy()->isDoingLoadingSecurityChecks() returns true.
Please look at DocumentThreadableLoader::redirectReceived and isResponseComingFromNetworkProcess.

We should also probably unskip http/tests/appcache/x-frame-options-prevents-framing.php after this patch.