[html] Add tests for parsing COOP values by jugglinmike · Pull Request #20873 · web-platform-tests/wpt

jugglinmike · 2019-12-19T19:18:38Z

No description provided.

domenic · 2019-12-19T19:22:48Z

I think we need to clarify whether the plan is for COOP to be a structured header or not, since that impacts various results...

jugglinmike · 2019-12-19T19:58:26Z

Has that change been discussed online?

domenic · 2019-12-19T20:00:19Z

Not to my knowledge; I just kind of thought we were not creating any new non-structured headers. (And, it'd be pretty weird if COOP was one thing and COEP another.)

jugglinmike · 2019-12-19T20:12:11Z

Got it. I filed whatwg/html#5172 to discuss this

annevk

As per whatwg/html#5172 (comment) I think this is the way to go, perhaps with a comment indicating we want to change this once SH is an RFC.

…ML standard

zcorpan · 2020-08-18T13:06:25Z

I've now updated this to match the current specs.

HTML now says COOP is a Structured Header and doesn't disallow parameters.
Structured Field Values changed details around whitespace (see Inconsistent rules around whitespace httpwg/http-extensions#1251 ), byte sequence syntax, allows * in tokens.

zcorpan · 2020-08-18T13:19:35Z

I also removed the same-origin\0 test because it now resulted in a network error in chromium (intentional per #25181 (comment) ) ~~for wptserve. (cc @ziransun @Hexcles , maybe related to python3 changes?)~~

A few tests time out for me in Chrome. Not sure why.

zcorpan · 2020-08-19T12:51:44Z

I had misunderstood the layering between HTTP and SFV. HTTP still trims OWS around the field-value: httpwg/http-extensions#1251 (comment)

Now fixed.

annevk

I have not done an extremely detailed take, but I'm assuming that the set of cases that now cause a policy to be enforced is strictly additive to what we had originally. If that's not the case let's revisit.

zcorpan · 2020-08-20T15:24:46Z

Thanks @annevk. Some tests here are still getting TIMEOUTs in both chrome and firefox. I tried to figure out what is causing it, without luck so far. For example, the "same-origin\t" test times out, while "same-origin " passes.

https://staging.wpt.fyi/results/html/cross-origin-opener-policy/header-parsing.https.html?label=pr_head&max-count=1&pr=20873

Am I missing something obvious?

annevk · 2020-08-20T15:32:52Z

It might be the Python header API not dealing with invalid values the way you would want for testing purposes. If that's the case you could write out the entire message instead in raw...

jugglinmike · 2020-08-21T00:19:08Z

The procedural test generation logic derives a name for the BroadcastChannel from the COOP value. Just like the COOP itself, the channel name is passed around via query string parameters, so we have to take care to URL-encode it as well.

The latest commit fixes the timeouts. Chromium passes all 24 subtests, and Firefox fails 2:

FAIL unspecified document opening popup to https://web-platform.test:8443 with COOP: "same-origin;same-origin" - assert_equals: name expected "" but got "unspecified_to_SAME_ORIGIN_same-origin;same-origin"
FAIL unspecified document opening popup to https://web-platform.test:8443 with COOP: "same-origin; foo=bar" - assert_equals: name expected "" but got "unspecified_to_SAME_ORIGIN_same-origin;-foo=bar"

Hexcles · 2020-08-21T00:50:46Z

I also removed the same-origin\0 test because it now resulted in a server error for wptserve. (cc @ziransun @Hexcles , maybe related to python3 changes?)

So that's a null byte at the end? Could you provide some more details (stacktrace maybe?)?

zcorpan · 2020-08-21T12:17:11Z

Thank you @jugglinmike!

The stability checks fail because they time out. I think since changing common.js causes all tests using it to run in the stability check (10 times each), and many of the tests take a long time to run, the whole thing takes too long time to run. So I think we'll need to admin-merge this (after review).

@Hexcles I can open a new PR to reintroduce the null byte test, to decouple from the rest here. Thanks!

zcorpan · 2020-08-24T12:53:40Z

@annevk you have reviewed this but there were 2 commits since.

annevk · 2020-08-24T13:08:24Z

html/cross-origin-opener-policy/header-parsing.https.html

+
+  // None of the following should be recognized as "same-origin" (hence the
+  // "expected opener" value of `true`).
+  [SAME_ORIGIN, "same\u2014origin", true], // non-ASCII character (em dash)


Having seen the COEP comments I guess we should address here too what bytes are going to be emitted by the server. And what shortcomings this approach has (in that 0xFF isn't feasible). Perhaps we should change approach so we can effectively specify bytes?

Yeah, good point. I've changed it now for this PR to specify bytes.

Did you check that other users of common.js not rely on encodeURIComponent? Especially reporting tests might be impacted, perhaps?

Hmm yes, though it's not obvious from test results (chrome, firefox).

But instead of sprinkling encodeURIComponent everywhere, I'm thinking about doing it differently, like using an object for the 0xFF test case:

[SAME_ORIGIN, { percentEscaped: "same%FForigin" }, true]

That seems reasonable, though I'd name it percentEncoded.

This reverts commit 31c5e51.

zcorpan · 2020-08-25T14:55:54Z

Thanks! I'll admin-merge, per #20873 (comment)

wpt-pr-bot added the html label Dec 19, 2019

wpt-pr-bot assigned foolip Dec 19, 2019

wpt-pr-bot requested review from annevk, domenic, foolip, jdm, jgraham, zcorpan and zqzhang December 19, 2019 19:18

jugglinmike mentioned this pull request Dec 19, 2019

Html coop parsing bocoup/wpt#12

Closed

jugglinmike mentioned this pull request Dec 19, 2019

[COOP] Header syntax whatwg/html#5172

Closed

jugglinmike mentioned this pull request Dec 20, 2019

Naming for new items in mode. w3c/webappsec-fetch-metadata#45

Closed

annevk approved these changes Jan 2, 2020

View reviewed changes

jugglinmike and others added 2 commits August 18, 2020 10:41

[html] Add tests for parsing COOP values

1de7e8d

Update tests to draft-ietf-httpbis-header-structure-19 and current HT…

2843a50

…ML standard

zcorpan force-pushed the html-coop-parsing-2 branch from 42287c9 to 2843a50 Compare August 18, 2020 12:56

wpt-pr-bot requested review from ParisMeuleman, arturjanc, hemeryar, mikewest and valenting August 18, 2020 12:56

Fix tests - leading and trailing OWS are removed by HTTP

05c9fe2

annevk approved these changes Aug 20, 2020

View reviewed changes

Fix typo

44ce78b

fixup! [html] Add tests for parsing COOP values

9839b15

This was referenced Aug 21, 2020

COOP: test COOP popup from a CSP-sandboxed popup #21111

Merged

Coop header parsing null byte #25181

Closed

zcorpan added the status:needs-review label Aug 24, 2020

annevk reviewed Aug 24, 2020

View reviewed changes

zcorpan added 4 commits August 24, 2020 20:00

Test bytes by using URL percent encoding

31c5e51

Revert "Test bytes by using URL percent encoding"

a7de38c

This reverts commit 31c5e51.

Use an object to represent the 0xFF byte test

84cfdf8

escaped -> encoded

76c0730

annevk approved these changes Aug 25, 2020

View reviewed changes

zcorpan merged commit f32f380 into web-platform-tests:master Aug 25, 2020

moz-wptsync-bot added mozilla:gecko-blocked and removed mozilla:gecko-blocked labels Aug 25, 2020

Conversation

jugglinmike commented Dec 19, 2019

Uh oh!

domenic commented Dec 19, 2019

Uh oh!

jugglinmike commented Dec 19, 2019

Uh oh!

domenic commented Dec 19, 2019

Uh oh!

jugglinmike commented Dec 19, 2019

Uh oh!

annevk left a comment

Choose a reason for hiding this comment

Uh oh!

zcorpan commented Aug 18, 2020

Uh oh!

zcorpan commented Aug 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zcorpan commented Aug 19, 2020

Uh oh!

annevk left a comment

Choose a reason for hiding this comment

Uh oh!

zcorpan commented Aug 20, 2020

Uh oh!

annevk commented Aug 20, 2020

Uh oh!

jugglinmike commented Aug 21, 2020

Uh oh!

Hexcles commented Aug 21, 2020

Uh oh!

zcorpan commented Aug 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zcorpan commented Aug 24, 2020

Uh oh!

annevk Aug 24, 2020

Choose a reason for hiding this comment

Uh oh!

zcorpan Aug 24, 2020

Choose a reason for hiding this comment

Uh oh!

annevk Aug 25, 2020

Choose a reason for hiding this comment

Uh oh!

zcorpan Aug 25, 2020

Choose a reason for hiding this comment

Uh oh!

annevk Aug 25, 2020

Choose a reason for hiding this comment

Uh oh!

zcorpan Aug 25, 2020

Choose a reason for hiding this comment

Uh oh!

zcorpan commented Aug 25, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

zcorpan commented Aug 18, 2020 •

edited

Loading

zcorpan commented Aug 21, 2020 •

edited

Loading