Bug 224626 - [ wk2 ] http/tests/security/contentSecurityPolicy/report-only-connect-src-xmlhttprequest-redirect-to-blocked.py is a constant text failure
Summary: [ wk2 ] http/tests/security/contentSecurityPolicy/report-only-connect-src-xml...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Gambrell
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-04-15 15:14 PDT by Robert Jenner
Modified: 2021-05-24 09:53 PDT (History)
13 users (show)

See Also:


Attachments
Test list used to reproduce Text Failure. (132.13 KB, text/plain)
2021-04-15 15:57 PDT, Robert Jenner
no flags Details
Patch (1.71 KB, patch)
2021-04-15 20:49 PDT, Chris Gambrell
no flags Details | Formatted Diff | Diff
Patch (4.25 KB, patch)
2021-05-21 14:52 PDT, Kate Cheney
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Jenner 2021-04-15 15:14:25 PDT
http/tests/security/contentSecurityPolicy/report-only-connect-src-xmlhttprequest-redirect-to-blocked.py

is a constant text failure on wk2 in macOS and iOS. 

HISTORY:
https://results.webkit.org/?suite=layout-tests&test=http%2Ftests%2Fsecurity%2FcontentSecurityPolicy%2Freport-only-connect-src-xmlhttprequest-redirect-to-blocked.py

History shows this test has failed in wk2 since it was brought online. 

TEXT DIFF:
 CONSOLE MESSAGE: The Content Security Policy 'connect-src http://127.0.0.1:8000/security/contentSecurityPolicy/resources/redir.py' was delivered in report-only mode, but does not specify a 'report-uri'; the policy will have no effect. Please either add a 'report-uri' directive, or deliver the policy via the 'Content-Security-Policy' header.
+CONSOLE MESSAGE: [Report Only] Refused to connect to http://localhost:8000/security/contentSecurityPolicy/resources/echo-report.py because it does not appear in the connect-src directive of the Content Security Policy.
 CONSOLE MESSAGE: [Report Only] Refused to connect to http://localhost:8000/security/contentSecurityPolicy/resources/xhr-redirect-not-allowed.py because it does not appear in the connect-src directive of the Content Security Policy.
 PASS XMLHttpRequest.send() did follow the redirect.
 PASS successfullyParsed is true
Comment 1 Robert Jenner 2021-04-15 15:28:27 PDT
Running standalone, the failure does not reproduce. But running against a test list, it does reproduce. Was able to reproduce the failure at BigSur Release ToT using the following test, and attached test list to this bug:

run-webkit-tests --root <path to revision> --test-list <path to test list>  --child-process=50

Working on narrowing down what test causes this test to fail.
Comment 2 Robert Jenner 2021-04-15 15:57:15 PDT
Created attachment 426148 [details]
Test list used to reproduce Text Failure.

Attaching test list used to reproduce test failure.
Comment 3 Robert Jenner 2021-04-15 16:29:32 PDT
After process of elimination, I have found that when the following test is removed from the test list, the test in question passes:

http/tests/security/contentSecurityPolicy/report-only-connect-src-beacon-redirect-blocked.py
Comment 4 Radar WebKit Bug Importer 2021-04-15 16:30:40 PDT
<rdar://problem/76729427>
Comment 5 Robert Jenner 2021-04-15 16:31:27 PDT
Looks like this test was added at r275917 by Chris:

https://trac.webkit.org/changeset/275917/webkit
Comment 6 Chris Gambrell 2021-04-15 20:49:06 PDT
Created attachment 426179 [details]
Patch
Comment 7 Chris Gambrell 2021-04-15 20:51:19 PDT
(In reply to Chris Gambrell from comment #6)
> Created attachment 426179 [details]
> Patch

Changing http/tests/security/contentSecurityPolicy/report-only-connect-src-beacon-redirect-blocked.py to partially match the functionality of http/tests/security/contentSecurityPolicy/report-only-connect-src-xmlhttprequest-redirect-to-blocked.py results in the test list passing
Comment 8 Daniel Bates 2021-04-16 09:55:27 PDT
Comment on attachment 426179 [details]
Patch

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

> LayoutTests/http/tests/security/contentSecurityPolicy/report-only-connect-src-beacon-redirect-blocked.py:28
> +    xhr.open("GET", "resources/redir.py?url=http://localhost:8000/security/contentSecurityPolicy/resources/echo-report.py", true);

I don't think testing using an XHR is the same as testing with a beacon.
Comment 9 Amir Mark Jr 2021-04-19 12:44:52 PDT
Updated test expectations:

https://trac.webkit.org/changeset/276268/webkit
Comment 10 Kate Cheney 2021-05-21 14:52:33 PDT
Created attachment 429346 [details]
Patch
Comment 11 Sam Weinig 2021-05-22 15:47:22 PDT
Comment on attachment 429346 [details]
Patch

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

> LayoutTests/ChangeLog:10
> +        for the console logging. This results in flakiness of other test that
> +        occasionally pick up this console logging in their output. To fix this,

I wonder if we can somehow detect when this happens programmatically and blame the test that is causing the issue. If I had to guess, our current willAddMessageToConsole delegate functions probably don't include all the context from the ConsoleMessage object in WebCore, and I am not ever sure how well we keep track of these things in WebCore, but it would be a nice tool in the toolbox for tracking these flaky things down.
Comment 12 Alexey Proskuryakov 2021-05-23 15:11:50 PDT
I wonder if it's still true that creating a new web view for each test is prohibitively slow. We could isolate tests better by using individual views.
Comment 13 Kate Cheney 2021-05-24 08:11:55 PDT
Comment on attachment 429346 [details]
Patch

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

>> LayoutTests/ChangeLog:10
>> +        occasionally pick up this console logging in their output. To fix this,
> 
> I wonder if we can somehow detect when this happens programmatically and blame the test that is causing the issue. If I had to guess, our current willAddMessageToConsole delegate functions probably don't include all the context from the ConsoleMessage object in WebCore, and I am not ever sure how well we keep track of these things in WebCore, but it would be a nice tool in the toolbox for tracking these flaky things down.

This would be nice. AFAIK currently there's little to no separation of console logging between tests, it only relies on timing.
Comment 14 Kate Cheney 2021-05-24 08:14:22 PDT
(In reply to Alexey Proskuryakov from comment #12)
> I wonder if it's still true that creating a new web view for each test is
> prohibitively slow. We could isolate tests better by using individual views.

Do you know if each view has isolated console logging or is that shared between them? If they are isolated, it would be useful to have for CSP tests which often create a lot of console logging and/or have it in their expectations.
Comment 15 EWS 2021-05-24 08:30:54 PDT
Committed r277952 (238079@main): <https://commits.webkit.org/238079@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 429346 [details].
Comment 16 Alexey Proskuryakov 2021-05-24 09:53:49 PDT
> Do you know if each view has isolated console logging or is that shared between them?

I see that my comment may have sounded like I knew exactly what to try, but in reality, there is likely an exploration needed of possible tradeoffs. For perfect separation, we would obviously need to restart UI process, which is likely not feasible, but using a new WebContent process and a new page group with its own delegate object may be?