Bug 227564

Summary: WebSocket traffic should be associated with content in Safari rather than the app itself
Product: WebKit Reporter: Richard Houle <rhoule>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Critical CC: ews-watchlist, katherine_cheney, toyoshim, webkit-bug-importer, yutak
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: iPhone / iPad   
OS: Unspecified   
Attachments:
Description Flags
Proposed fix
none
test case app initiated
none
test case user initiated
none
V2
none
V3
ews-feeder: commit-queue-
V4
katherine_cheney: review+, ews-feeder: commit-queue-
V4 with Changeling none

Description Richard Houle 2021-06-30 20:36:22 PDT
Created attachment 432654 [details]
Proposed fix

<rdar://79307301>

WebSocket traffic is currently assigned to Safari. Someone browsing on a website using web sockets as messaging will be reported as Safari contacting a website rather than the content contacting a website.
Comment 1 Kate Cheney 2021-07-01 11:43:46 PDT
Looks good. I think we should write a test before landing.
Comment 2 Radar WebKit Bug Importer 2021-07-07 20:37:16 PDT
<rdar://problem/80303362>
Comment 3 Kate Cheney 2021-07-08 20:52:29 PDT
Created attachment 433197 [details]
test case app initiated
Comment 4 Kate Cheney 2021-07-08 20:52:50 PDT
Created attachment 433198 [details]
test case user initiated
Comment 5 Kate Cheney 2021-07-08 20:54:57 PDT
Comment on attachment 432654 [details]
Proposed fix

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

> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1696
> +    if (!request.isAppBound()) {

I did a rename so you will likely have to rebase this patch.

> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1701
> +#endif

I think you'll also need something like this:

appPrivacyReportTestingData().didLoadAppInitiatedRequest(nsRequest.get().attribution == NSURLRequestAttributionDeveloper);

here to send the attribution data to the test runner.
Comment 6 Richard Houle 2021-07-22 12:12:54 PDT
Created attachment 434027 [details]
V2
Comment 7 Richard Houle 2021-07-22 13:44:07 PDT
Created attachment 434032 [details]
V3
Comment 8 Kate Cheney 2021-07-22 15:29:11 PDT
(In reply to Richard Houle from comment #7)
> Created attachment 434032 [details]
> V3

You will need to update test expectations to skip the new tests in open source because our EWS bots are iOS 14 only. You can either move them to the LayoutTests/http/tests/app-privacy-report/ directory which is already skipped or add the test path you made to LayoutTests/TestExpectations (just make sure they pass locally).
Comment 9 Richard Houle 2021-07-22 17:51:49 PDT
Created attachment 434049 [details]
V4
Comment 10 Richard Houle 2021-07-22 17:54:25 PDT
I got the following output when I ran them locally:

Test configuration: <, x86_64, release>
Placing test results in /Volumes/Work/Safari/OpenSource/WebKitBuild/Release-iphonesimulator/layout-test-results
Using Release build
Pixel tests disabled
Regular timeout: 30000, slow test timeout: 150000
Command line: /Volumes/Work/Safari/OpenSource/WebKitBuild/Release-iphonesimulator/WebKitTestRunnerApp.app -

--lint-test-files warnings:                                           
LayoutTests/platform/ios/TestExpectations:2652 Path does not exist. imported/w3c/web-platform-tests/html/webappapis/timers/type-long-setinterval.html
LayoutTests/platform/ios-wk2/TestExpectations:1204 Path does not exist. imported/w3c/web-platform-tests/html/webappapis/timers/negative-settimeout.html
LayoutTests/platform/ios-wk2/TestExpectations:1278 Path does not exist. imported/w3c/web-platform-tests/websockets/Create-Secure-verify-url-set-non-default-port.any.html

Found 2 tests; running 2, skipping 0.
                        
Verbose baseline search path: platform/iphone-se (1st generation)-simulator-15-wk2 -> platform/iphone-se (1st generation)-simulator-15 -> platform/iphone-se (1st generation)-simulator-wk2 -> platform/iphone-se (1st generation)-simulator -> platform/iphone-simulator-15-wk2 -> platform/iphone-simulator-15 -> platform/iphone-simulator-wk2 -> platform/iphone-simulator -> platform/ios-simulator-15-wk2 -> platform/ios-simulator-15 -> platform/ios-simulator-wk2 -> platform/ios-simulator -> platform/iphone-se (1st generation)-15-wk2 -> platform/iphone-se (1st generation)-15 -> platform/iphone-se (1st generation)-wk2 -> platform/iphone-se (1st generation) -> platform/iphone-15-wk2 -> platform/iphone-15 -> platform/iphone-wk2 -> platform/iphone -> platform/ios-15-wk2 -> platform/ios-15 -> platform/ios-wk2 -> platform/ios -> platform/wk2 -> generic

Baseline search path: platform/ios-simulator-wk2 -> platform/ios-simulator -> platform/ios-wk2 -> platform/ios -> platform/wk2 -> generic

Running 2 tests for iPhone SE (1st generation) running iOS 15

Running 1 WebKitTestRunnerApp.app.

[1/2] http/tests/app-privacy-report/websocket-isappinitiated.html passed
[2/2] http/tests/app-privacy-report/websocket-isnotappinitiated.html passed
                        
All 2 tests ran as expected.
Comment 11 Kate Cheney 2021-07-23 10:09:36 PDT
Comment on attachment 434049 [details]
V4

Looks good! r=me
Comment 12 EWS 2021-07-23 10:10:42 PDT
Unable to find any modified ChangeLog in Attachment 434049 [details]
Comment 13 Kate Cheney 2021-07-23 10:11:39 PDT
Ah, missing a ChangeLog, can you upload a new patch with one?
Comment 14 Richard Houle 2021-07-23 10:24:43 PDT
Created attachment 434097 [details]
V4 with Changeling
Comment 15 EWS 2021-07-23 11:23:22 PDT
Committed r280250 (239917@main): <https://commits.webkit.org/239917@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 434097 [details].