Upgrade insecure request: Drop the reporting functionality.

Upgrade insecure request: Drop the reporting functionality.

Reporting will be handled by report-only directives, not via a special
upgrade-based-reporting system, as described in [1].

[1]: https://w3c.github.io/webappsec/specs/upgrade/#reporting-upgrades

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

[Mike West, 2015-03-05 10:34:51]

mkwst@chromium.org changed reviewers:
+ yoav@yoav.ws

[Mike West, 2015-03-05 10:34:51]

Mind taking a look at this when you wake up, Yoav?

[Yoav Weiss, 2015-03-05 12:06:14]

Sure. Woke up way earlier than I thought I would...

OTOH, I'm not sure I understand where we want to go with the upgrade reporting.
Can you add an example of the header syntax? (either here or in the spec)

https://codereview.chromium.org/983533005/diff/1/Source/core/frame/csp/CSPDir...
File Source/core/frame/csp/CSPDirectiveList.cpp (right):

https://codereview.chromium.org/983533005/diff/1/Source/core/frame/csp/CSPDir...
Source/core/frame/csp/CSPDirectiveList.cpp:587: }
Why is report-only invalid here?

[Mike West, 2015-03-05 12:22:12]

On 2015/03/05 at 12:06:14, yoav wrote:
> Sure. Woke up way earlier than I thought I would...

Welcome to jetlag!

> OTOH, I'm not sure I understand where we want to go with the upgrade
reporting. Can you add an example of the header syntax? (either here or in the
spec)

This is explained a bit in
https://w3c.github.io/webappsec/specs/upgrade/#reporting-upgrades. I'll add that
link to the CL description.


>
https://codereview.chromium.org/983533005/diff/1/Source/core/frame/csp/CSPDir...
> File Source/core/frame/csp/CSPDirectiveList.cpp (right):
> 
>
https://codereview.chromium.org/983533005/diff/1/Source/core/frame/csp/CSPDir...
> Source/core/frame/csp/CSPDirectiveList.cpp:587: }
> Why is report-only invalid here?

Because the directive now has no effect in `csp-report-only` mode.

[Yoav Weiss, 2015-03-05 12:31:34]

OK, LGTM % question to make sure I get it.

https://codereview.chromium.org/983533005/diff/1/Source/core/frame/csp/Conten...
File Source/core/frame/csp/ContentSecurityPolicyTest.cpp (right):

https://codereview.chromium.org/983533005/diff/1/Source/core/frame/csp/Conten...
Source/core/frame/csp/ContentSecurityPolicyTest.cpp:75:
EXPECT_EQ(SecurityContext::InsecureContentDoNotUpgrade,
csp->insecureContentPolicy());
So in this case, we would also have an invalid policy?

[Mike West, 2015-03-05 12:37:06]

On 2015/03/05 at 12:31:34, yoav wrote:
> OK, LGTM % question to make sure I get it.
> 
>
https://codereview.chromium.org/983533005/diff/1/Source/core/frame/csp/Conten...
> File Source/core/frame/csp/ContentSecurityPolicyTest.cpp (right):
> 
>
https://codereview.chromium.org/983533005/diff/1/Source/core/frame/csp/Conten...
> Source/core/frame/csp/ContentSecurityPolicyTest.cpp:75:
EXPECT_EQ(SecurityContext::InsecureContentDoNotUpgrade,
csp->insecureContentPolicy());
> So in this case, we would also have an invalid policy?

Well, depends on what you mean by "invalid". :) We'd have an empty policy, as
we'd just ignore the directive.

[Yoav Weiss, 2015-03-05 12:39:57]

LGTM

Off to the airport! :)

On Thu, Mar 5, 2015 at 1:37 PM, <mkwst@chromium.org> wrote:

> On 2015/03/05 at 12:31:34, yoav wrote:
>
>> OK, LGTM % question to make sure I get it.
>>
>
>
> https://codereview.chromium.org/983533005/diff/1/Source/core/frame/csp/
> ContentSecurityPolicyTest.cpp
>
>> File Source/core/frame/csp/ContentSecurityPolicyTest.cpp (right):
>>
>
>
> https://codereview.chromium.org/983533005/diff/1/Source/core/frame/csp/
> ContentSecurityPolicyTest.cpp#newcode75
>
>> Source/core/frame/csp/ContentSecurityPolicyTest.cpp:75:
>>
> EXPECT_EQ(SecurityContext::InsecureContentDoNotUpgrade,
> csp->insecureContentPolicy());
>
>> So in this case, we would also have an invalid policy?
>>
>
> Well, depends on what you mean by "invalid". :) We'd have an empty policy,
> as
> we'd just ignore the directive.
>
> https://codereview.chromium.org/983533005/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[Mike West, 2015-03-05 12:54:34]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2015-03-05 12:54:47]

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

[commit-bot: I haz the power, 2015-03-05 14:18:10]

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