Bug 163544 - Redirections should be upgraded if CSP policy says so
Summary: Redirections should be upgraded if CSP policy says so
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-10-17 08:10 PDT by youenn fablet
Modified: 2016-10-24 00:52 PDT (History)
8 users (show)

See Also:

Attachments
Patch (15.04 KB, patch)
2016-10-17 08:14 PDT, youenn fablet 		no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-yosemite (909.48 KB, application/zip)
2016-10-17 09:14 PDT, Build Bot 		no flags Details
Archive of layout-test-results from ews105 for mac-yosemite-wk2 (1.28 MB, application/zip)
2016-10-17 10:00 PDT, Build Bot 		no flags Details
Patch (16.08 KB, patch)
2016-10-18 05:55 PDT, youenn fablet 		no flags Details | Formatted Diff | Diff
Patch for landing (15.95 KB, patch)
2016-10-24 00:17 PDT, youenn fablet 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.