Bug 184760 - NetworkProcess should use CSP/content blockers for sync XHR
Summary: NetworkProcess should use CSP/content blockers for sync XHR
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-04-18 16:07 PDT by youenn fablet
Modified: 2018-04-19 10:55 PDT (History)
7 users (show)

See Also:


Attachments
Patch (10.51 KB, patch)
2018-04-18 16:09 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (10.46 KB, patch)
2018-04-19 09:31 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2018-04-18 16:07:59 PDT
NetworkProcess should use CSP/content blockers for sync XHR
Comment 1 youenn fablet 2018-04-18 16:09:08 PDT
Created attachment 338271 [details]
Patch
Comment 2 Daniel Bates 2018-04-18 21:40:38 PDT
Comment on attachment 338271 [details]
Patch

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

> LayoutTests/http/tests/contentextensions/sync-xhr-redirection-blocked.html:1
> +<head>

Pleae add a <!DOCTYPE html>.

> LayoutTests/http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/resources/insecure-sync-xhr-in-main-frame-window.html:1
> +<html>

Please add a <!DOCTYPE html>. Also, is this markup well formed?

> LayoutTests/platform/mac-wk1/TestExpectations:100
> +# WK1 sync XHR unsupported features.

I am unclear what this comment means. WebKit1 supports sync XHR last I recall. Or did we remove such support? WebKit1 does not support context extensions.

> LayoutTests/platform/win/TestExpectations:2214
> +# WK1 sync XHR unsupported features.

Ditto.
Comment 3 youenn fablet 2018-04-18 22:05:13 PDT
> I am unclear what this comment means. WebKit1 supports sync XHR last I
> recall. Or did we remove such support? WebKit1 does not support context
> extensions.

Sync XHR in WK2 now supports handling of CORS/CSP/ContentBlockers for redirections.
This is not the case in WK1.
Comment 4 Chris Dumez 2018-04-19 08:55:52 PDT
Comment on attachment 338271 [details]
Patch

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

r=me with comments.

>> LayoutTests/http/tests/contentextensions/sync-xhr-redirection-blocked.html:1
>> +<head>
> 
> Pleae add a <!DOCTYPE html>.

+1

>> LayoutTests/http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/resources/insecure-sync-xhr-in-main-frame-window.html:1
>> +<html>
> 
> Please add a <!DOCTYPE html>. Also, is this markup well formed?

+1

> LayoutTests/http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/resources/insecure-sync-xhr-in-main-frame-window.html:2
> +<meta http-equiv="Content-Security-Policy" content="upgrade-insecure-requests">

I think it'd be nicer to put the meta tag inside a <head>

> LayoutTests/http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/resources/insecure-sync-xhr-in-main-frame-window.html:7
> +    xhr = new XMLHttpRequest();

const xhr =

I do not think it needs to be global.

> LayoutTests/platform/mac-wk1/TestExpectations:101
> +http/tests/contentextensions/sync-xhr-redirection-blocked.html [ Skip ]

This folder is skipped globally and only enabled on mac-wk2 so this entry should not be needed.

> LayoutTests/platform/mac-wk1/TestExpectations:102
> +http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/upgrade-insecure-sync-xhr-in-main-frame.html [ Skip ]

I am assuming this fails on WK1 because you fixed Sync XHR + CSP for WK2 only. I think this is fine but the comment should be clearer if this is the case.

> LayoutTests/platform/win/TestExpectations:2215
> +http/tests/contentextensions/sync-xhr-redirection-blocked.html [ Skip ]

Ditto.

> LayoutTests/platform/win/TestExpectations:2216
> +http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/upgrade-insecure-sync-xhr-in-main-frame.html [ Skip ]

ditto.
Comment 5 youenn fablet 2018-04-19 09:31:00 PDT
Created attachment 338330 [details]
Patch for landing
Comment 6 WebKit Commit Bot 2018-04-19 10:54:40 PDT
Comment on attachment 338330 [details]
Patch for landing

Clearing flags on attachment: 338330

Committed r230810: <https://trac.webkit.org/changeset/230810>
Comment 7 WebKit Commit Bot 2018-04-19 10:54:42 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2018-04-19 10:55:19 PDT
<rdar://problem/39569304>