Bug 224626 – [ wk2 ] http/tests/security/contentSecurityPolicy/report-only-connect-src-xmlhttprequest-redirect-to-blocked.py is a constant text failure

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Robert Jenner]

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

[Robert Jenner]

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.

[Robert Jenner]

Created attachment 426148 [details]
Test list used to reproduce Text Failure.

Attaching test list used to reproduce test failure.

[Robert Jenner]

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

[Radar WebKit Bug Importer]

<rdar://problem/76729427>

[Robert Jenner]

Looks like this test was added at r275917 by Chris:

https://trac.webkit.org/changeset/275917/webkit

[Chris Gambrell]

Created attachment 426179 [details]
Patch

[Chris Gambrell]

(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

[Daniel Bates]

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.

[Amir Mark Jr]

Updated test expectations:

https://trac.webkit.org/changeset/276268/webkit

[Kate Cheney]

Created attachment 429346 [details]
Patch

[Sam Weinig]

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.

[Alexey Proskuryakov]

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.

[Kate Cheney]

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.

[Kate Cheney]

(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.

[EWS]

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].

[Alexey Proskuryakov]

> 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?