Upgrade: Send a 'Prefer' header for feature detection.

Upgrade: Send a 'Prefer' header for feature detection.

As defined in https://w3c.github.io/webappsec/specs/upgrade/#feature-detect,
this patch adds a 'return=secure-representation' preference to outgoing
insecure and navigational requests.

BUG=455674
R=yoav@yoav.ws

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=190341

[Mike West, 2015-02-17 16:06:05]

Yoav, mind taking a look at this, since you've reviewed the rest of the feature?

[Yoav Weiss, 2015-02-17 16:23:31]

LGTM. A couple of questions tho

https://codereview.chromium.org/930323002/diff/1/Source/core/fetch/ResourceFe...
File Source/core/fetch/ResourceFetcher.cpp (left):

https://codereview.chromium.org/930323002/diff/1/Source/core/fetch/ResourceFe...
Source/core/fetch/ResourceFetcher.cpp:904: void
ResourceFetcher::maybeUpgradeInsecureRequestURL(FetchRequest& fetchRequest)
Should we change the method name to reflect that it also includes header
changes?

https://codereview.chromium.org/930323002/diff/1/Source/core/fetch/ResourceFe...
File Source/core/fetch/ResourceFetcher.cpp (right):

https://codereview.chromium.org/930323002/diff/1/Source/core/fetch/ResourceFe...
Source/core/fetch/ResourceFetcher.cpp:913: if
(fetchRequest.resourceRequest().frameType() != WebURLRequest::FrameTypeNone &&
!SecurityOrigin::isSecure(url))
So for insecure origins, we're sending Prefer headers on everything other than
sub-resource requests, right?

[Mike West, 2015-02-17 16:30:55]

https://codereview.chromium.org/930323002/diff/1/Source/core/fetch/ResourceFe...
File Source/core/fetch/ResourceFetcher.cpp (left):

https://codereview.chromium.org/930323002/diff/1/Source/core/fetch/ResourceFe...
Source/core/fetch/ResourceFetcher.cpp:904: void
ResourceFetcher::maybeUpgradeInsecureRequestURL(FetchRequest& fetchRequest)
On 2015/02/17 at 16:23:31, Yoav Weiss wrote:
> Should we change the method name to reflect that it also includes header
changes?

We should. Suggestions? Maybe just 'upgradeInsecureRequest'?

https://codereview.chromium.org/930323002/diff/1/Source/core/fetch/ResourceFe...
File Source/core/fetch/ResourceFetcher.cpp (right):

https://codereview.chromium.org/930323002/diff/1/Source/core/fetch/ResourceFe...
Source/core/fetch/ResourceFetcher.cpp:913: if
(fetchRequest.resourceRequest().frameType() != WebURLRequest::FrameTypeNone &&
!SecurityOrigin::isSecure(url))
On 2015/02/17 at 16:23:31, Yoav Weiss wrote:
> So for insecure origins, we're sending Prefer headers on everything other than
sub-resource requests, right?

The goal is to send the header along with navigational requests to insecure
origins, yes. These origins could then choose to redirect users to a secure
variant based on the presence of the header. We don't do the inverse, as there's
a danger that the header would otherwise become a dangling bit of the platform
in ~X years when we're all using HTTPS for everything. :)

[Mike West, 2015-02-17 16:35:53]

New patchsets have been uploaded after l-g-t-m from yoav@yoav.ws

[Yoav Weiss, 2015-02-17 16:45:06]

On 2015/02/17 16:30:55, Mike West wrote:
>
https://codereview.chromium.org/930323002/diff/1/Source/core/fetch/ResourceFe...
> File Source/core/fetch/ResourceFetcher.cpp (left):
> 
>
https://codereview.chromium.org/930323002/diff/1/Source/core/fetch/ResourceFe...
> Source/core/fetch/ResourceFetcher.cpp:904: void
> ResourceFetcher::maybeUpgradeInsecureRequestURL(FetchRequest& fetchRequest)
> On 2015/02/17 at 16:23:31, Yoav Weiss wrote:
> > Should we change the method name to reflect that it also includes header
> changes?
> 
> We should. Suggestions? Maybe just 'upgradeInsecureRequest'?

For the lack of a better name, why not? :)

> 
>
https://codereview.chromium.org/930323002/diff/1/Source/core/fetch/ResourceFe...
> File Source/core/fetch/ResourceFetcher.cpp (right):
> 
>
https://codereview.chromium.org/930323002/diff/1/Source/core/fetch/ResourceFe...
> Source/core/fetch/ResourceFetcher.cpp:913: if
> (fetchRequest.resourceRequest().frameType() != WebURLRequest::FrameTypeNone &&
> !SecurityOrigin::isSecure(url))
> On 2015/02/17 at 16:23:31, Yoav Weiss wrote:
> > So for insecure origins, we're sending Prefer headers on everything other
than
> sub-resource requests, right?
> 
> The goal is to send the header along with navigational requests to insecure
> origins, yes. These origins could then choose to redirect users to a secure
> variant based on the presence of the header. We don't do the inverse, as
there's
> a danger that the header would otherwise become a dangling bit of the platform
> in ~X years when we're all using HTTPS for everything. :)

I totally agree that this is the right behavior. Just wanted to make sure that
this is what FrameTypeNone meant.

[Mike West, 2015-02-17 16:52:33]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2015-02-17 16:54:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/930323002/20001

[commit-bot: I haz the power, 2015-02-17 18:15:45]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=190341