Content-Security-Policy: Add tests when source scheme is empty.

Content-Security-Policy: Add tests when source scheme is empty.

The current behavior is that when a CSP source expression doesn't have a
scheme, the url scheme must be equal to the 'self' scheme. Alternatively
when the 'self' scheme is 'http', every scheme in the HTTP familly are
allowed.
This CL adds tests for documenting this behavior.

BUG=None.

Review-Url: https://codereview.chromium.org/2694233002
Cr-Commit-Position: refs/heads/master@{#450680}
Committed: https://chromium.googlesource.com/chromium/src/+/6e9b7247c80de98888c1d90ed61457538f51931a

[arthursonzogni, 2017-02-14 12:54:22]

arthursonzogni@chromium.org changed reviewers:
+ mkwst@chromium.org

[arthursonzogni, 2017-02-14 12:54:23]

Hi Mike,

I retrieved a section of a test I took from the browser-side implementation of
CSP.
https://codereview.chromium.org/2612793002/

The current behaviors that look weird are the same on the renderer-side
implementation.

https://codereview.chromium.org/2694233002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/frame/csp/CSPSourceTest.cpp (right):

https://codereview.chromium.org/2694233002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/csp/CSPSourceTest.cpp:167:
EXPECT_FALSE(source.matches(KURL(base, "https-so://a.com")));
Same unexpected behavior as in https://codereview.chromium.org/2691063003/
Do you think I should fill a bug report?

https://codereview.chromium.org/2694233002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/csp/CSPSourceTest.cpp:189:
EXPECT_FALSE(source.matches(KURL(base, "non-standard-scheme://a.com")));
Same behavior as in https://codereview.chromium.org/2691063003/
Maybe this case doesn't happens in practice though.
It is nice to keep this test to detect future changes.

[Mike West, 2017-02-15 06:46:00]

LGTM % bugs and TODOs.

https://codereview.chromium.org/2694233002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/frame/csp/CSPSourceTest.cpp (right):

https://codereview.chromium.org/2694233002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/csp/CSPSourceTest.cpp:167:
EXPECT_FALSE(source.matches(KURL(base, "https-so://a.com")));
On 2017/02/14 at 12:54:22, arthursonzogni wrote:
> Same unexpected behavior as in https://codereview.chromium.org/2691063003/
> Do you think I should fill a bug report?

Yes, please file a bug and add a TODO. I don't know if we ever see a suborigin
serialization when we're hitting CSP (I don't think we do), but we should make
sure that the behavior is sane.

I'm more worried about this in the browser process than in Blink, though. Part
of the serialization mess we've created is to bridge that gap. :/

https://codereview.chromium.org/2694233002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/csp/CSPSourceTest.cpp:189:
EXPECT_FALSE(source.matches(KURL(base, "non-standard-scheme://a.com")));
On 2017/02/14 at 12:54:23, arthursonzogni wrote:
> Same behavior as in https://codereview.chromium.org/2691063003/
> Maybe this case doesn't happens in practice though.
> It is nice to keep this test to detect future changes.

Please file a bug and add a TODO. This seems wrong.

[arthursonzogni, 2017-02-15 11:59:55]

The CQ bit was checked by arthursonzogni@chromium.org

[arthursonzogni, 2017-02-15 11:59:56]

The patchset sent to the CQ was uploaded after l-g-t-m from mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/2694233002/#ps20001
(title: "Add TODO and BUG id.")

[arthursonzogni, 2017-02-15 12:00:14]

Thanks!

https://codereview.chromium.org/2694233002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/frame/csp/CSPSourceTest.cpp (right):

https://codereview.chromium.org/2694233002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/csp/CSPSourceTest.cpp:167:
EXPECT_FALSE(source.matches(KURL(base, "https-so://a.com")));
On 2017/02/15 06:46:00, Mike West (sloooooow) wrote:
> On 2017/02/14 at 12:54:22, arthursonzogni wrote:
> > Same unexpected behavior as in https://codereview.chromium.org/2691063003/
> > Do you think I should fill a bug report?
> 
> Yes, please file a bug and add a TODO. I don't know if we ever see a suborigin
> serialization when we're hitting CSP (I don't think we do), but we should make
> sure that the behavior is sane.
> 
> I'm more worried about this in the browser process than in Blink, though. Part
> of the serialization mess we've created is to bridge that gap. :/

Done, BUG=692442

https://codereview.chromium.org/2694233002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/csp/CSPSourceTest.cpp:189:
EXPECT_FALSE(source.matches(KURL(base, "non-standard-scheme://a.com")));
On 2017/02/15 06:46:00, Mike West (sloooooow) wrote:
> On 2017/02/14 at 12:54:23, arthursonzogni wrote:
> > Same behavior as in https://codereview.chromium.org/2691063003/
> > Maybe this case doesn't happens in practice though.
> > It is nice to keep this test to detect future changes.
> 
> Please file a bug and add a TODO. This seems wrong.

Done. BUG=692449

[commit-bot: I haz the power, 2017-02-15 12:00:22]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-15 13:15:13]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1487159995719430,
"parent_rev": "c4b071b9088980c941d3dea85478b10675a479e6", "commit_rev":
"6e9b7247c80de98888c1d90ed61457538f51931a"}

[commit-bot: I haz the power, 2017-02-15 13:15:53]

Description was changed from

==========
Content-Security-Policy: Add tests when source scheme is empty.

The current behavior is that when a CSP source expression doesn't have a
scheme, the url scheme must be equal to the 'self' scheme. Alternatively
when the 'self' scheme is 'http', every scheme in the HTTP familly are
allowed.
This CL adds tests for documenting this behavior.

BUG=None.
==========

to

==========
Content-Security-Policy: Add tests when source scheme is empty.

The current behavior is that when a CSP source expression doesn't have a
scheme, the url scheme must be equal to the 'self' scheme. Alternatively
when the 'self' scheme is 'http', every scheme in the HTTP familly are
allowed.
This CL adds tests for documenting this behavior.

BUG=None.

Review-Url: https://codereview.chromium.org/2694233002
Cr-Commit-Position: refs/heads/master@{#450680}
Committed:
https://chromium.googlesource.com/chromium/src/+/6e9b7247c80de98888c1d90ed614...
==========

[commit-bot: I haz the power, 2017-02-15 13:15:55]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/6e9b7247c80de98888c1d90ed614...