Bug 184760

Summary:

NetworkProcess should use CSP/content blockers for sync XHR

Product:

WebKit

Reporter:

youenn fablet <youennf>

Component:

WebKit Misc.

Assignee:

youenn fablet <youennf>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, cdumez, commit-queue, dbates, ews-watchlist, mkwst, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch for landing	none

youenn fablet

Reported 2018-04-18 16:07:59 PDT

NetworkProcess should use CSP/content blockers for sync XHR

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
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

youenn fablet

Comment 1 2018-04-18 16:09:08 PDT

Created attachment 338271 [details] Patch

Daniel Bates

Comment 2 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.

youenn fablet

Comment 3 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.

Chris Dumez

Comment 4 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.

youenn fablet

Comment 5 2018-04-19 09:31:00 PDT

Created attachment 338330 [details] Patch for landing

WebKit Commit Bot

Comment 6 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>

WebKit Commit Bot

Comment 7 2018-04-19 10:54:42 PDT

All reviewed patches have been landed. Closing bug.

Radar WebKit Bug Importer

Comment 8 2018-04-19 10:55:19 PDT

<rdar://problem/39569304>

Note You need to log in before you can comment on or make changes to this bug.