WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
184760
NetworkProcess should use CSP/content blockers for sync XHR
https://bugs.webkit.org/show_bug.cgi?id=184760
Summary
NetworkProcess should use CSP/content blockers for sync XHR
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.
Top of Page
Format For Printing
XML
Clone This Bug