Bug 163544

Summary: Redirections should be upgraded if CSP policy says so
Product: WebKit Reporter: youenn fablet <youennf>
Component: WebCore Misc.Assignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, buildbot, cdumez, commit-queue, dbates, japhet, mkwst, rniwa
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews102 for mac-yosemite
none
Archive of layout-test-results from ews105 for mac-yosemite-wk2
none
Patch
none
Patch for landing none

Description youenn fablet 2016-10-17 08:10:05 PDT
We only upgrade the initial requests but not the redirections, which goes against fetch spec and Gecko behavior.
Comment 1 youenn fablet 2016-10-17 08:14:59 PDT
Created attachment 291819 [details]
Patch
Comment 2 youenn fablet 2016-10-17 08:16:06 PDT
New test is passing in Firefox, but not in Chrome.
Comment 3 Build Bot 2016-10-17 09:14:54 PDT
Comment on attachment 291819 [details]
Patch

Attachment 291819 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/2304869

New failing tests:
http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/upgrade-redirect-https-to-http-script-in-iframe.html
Comment 4 Build Bot 2016-10-17 09:14:58 PDT
Created attachment 291822 [details]
Archive of layout-test-results from ews102 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 5 Build Bot 2016-10-17 10:00:18 PDT
Comment on attachment 291819 [details]
Patch

Attachment 291819 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2305065

New failing tests:
http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/upgrade-redirect-https-to-http-script-in-iframe.html
Comment 6 Build Bot 2016-10-17 10:00:22 PDT
Created attachment 291827 [details]
Archive of layout-test-results from ews105 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 7 youenn fablet 2016-10-18 05:27:57 PDT
(In reply to comment #5)
> Comment on attachment 291819 [details]
> Patch
> 
> Attachment 291819 [details] did not pass mac-wk2-ews (mac-wk2):
> Output: http://webkit-queues.webkit.org/results/2305065
> 
> New failing tests:
> http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/upgrade-
> redirect-https-to-http-script-in-iframe.html

This test is timing out because the new behavior upgrades the HTTPS request and we end up with a "CFNetwork SSLHandshake failed (-9847)" error.
Comment 8 youenn fablet 2016-10-18 05:55:41 PDT
Created attachment 291944 [details]
Patch
Comment 9 Darin Adler 2016-10-21 23:23:42 PDT
Comment on attachment 291944 [details]
Patch

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

> Source/WebCore/loader/cache/CachedResourceLoader.cpp:503
> +    if (Document* document = m_documentLoader->cachedResourceLoader().document())

I like auto* for cases like this.
Comment 10 youenn fablet 2016-10-24 00:17:24 PDT
Created attachment 292590 [details]
Patch for landing
Comment 11 youenn fablet 2016-10-24 00:17:50 PDT
Thanks for the review.

(In reply to comment #9)
> Comment on attachment 291944 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=291944&action=review
> 
> > Source/WebCore/loader/cache/CachedResourceLoader.cpp:503
> > +    if (Document* document = m_documentLoader->cachedResourceLoader().document())
> 
> I like auto* for cases like this.

Done
Comment 12 WebKit Commit Bot 2016-10-24 00:52:02 PDT
Comment on attachment 292590 [details]
Patch for landing

Clearing flags on attachment: 292590

Committed r207752: <http://trac.webkit.org/changeset/207752>
Comment 13 WebKit Commit Bot 2016-10-24 00:52:08 PDT
All reviewed patches have been landed.  Closing bug.