Add tests to verify origin headers sent by ping

[rwlbuis]

No description provided.

[wpt-pr-bot]

There are no reviewers for this pull request. Please reach out on W3C's irc server (irc.w3.org, port 6665) on channel #testing (web client) to get help with this. Thank you!

[rwlbuis]

@annevk any idea for the testRunner references? This only needed for chromium. I noticed webnfc uses testRunner as well, not sure how acceptable that is?

[annevk]

Sorry, I don't really know what that is. @jgraham might know?

[rwlbuis]

Sorry, I don't really know what that is. @jgraham might know?

Thnx Anne. @jgraham do you know how to deal with testRunner invocations in WPT tests? I believe tesTrunner is needed to test this functionality (hyperlink auditing) and make the tests pass for Chromium only.

[domfarolino]

(Copying what I wrote in the chromium codereview).

The testRunner preference override is required by [1] at least for Chromium (and maybe WebKit?). So we have to keep the window.testRunner reference. I think we can do this by adding a script to resources/chromium called enable-hyperlink-auditing.js or something that simply calls the overridePreference method. Then, we can follow what [2] did, and modify the lint whitelist to allow this, and simply import the script to all of the tests that need it.

1. https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/html/html_anchor_element.cc?l=341
2. https://chromium-review.googlesource.com/c/chromium/src/+/1895345

[rwlbuis]

(Copying what I wrote in the chromium codereview).

The testRunner preference override is required by [1] at least for Chromium (and maybe WebKit?). So we have to keep the window.testRunner reference. I think we can do this by adding a script to resources/chromium called enable-hyperlink-auditing.js or something that simply calls the overridePreference method. Then, we can follow what [2] did, and modify the lint whitelist to allow this, and simply import the script to all of the tests that need it.

1. https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/html/html_anchor_element.cc?l=341
2. https://chromium-review.googlesource.com/c/chromium/src/+/1895345

Thanks Dominic, that makes sense, I'll try to implement this today.

[rwlbuis]

I think this is ready for review after Dominic's suggestions. I think the test expectations are correct since nothing is set for referrer-policy, so the default empty string is used, and according to
https://fetch.spec.whatwg.org/#append-a-request-origin-header this will not result in null.
This feels a bit unintuitive given the referrer=no-referrer, but I guess it is intentional...

[rwlbuis]

@annevk care for another review, now the testRunner issue is addressed? :)

[annevk]

Wouldn't https://fetch.spec.whatwg.org/#append-a-request-origin-header cause the origin to be null here?

[rwlbuis]

I think you are right. So setting referrer to no-referrer only has an effect on the request's referrer, not its origin header, correct? I updated header-origin-no-referrer.html.

[rwlbuis]

Sorry for the confusion, I meant "setting referrer to no-referrer" in this context )not the meta tag):
https://html.spec.whatwg.org/multipage/links.html#hyperlink-auditing

[annevk]

It has an effect on the request's Origin header as well as the request's Referer header. (It does not always affect the Origin header, that algorithm defines when it does.)

Does that help?

[rwlbuis]

You mean https://fetch.spec.whatwg.org/#append-a-request-origin-header right, in case of not GET/HEAD? Right in my examples only no-referrer case is triggered, as the tests are same-origin and same protocol.

[domfarolino]

Just a general note on the duplication: I notice each test is basically the same with the exception of a couple lines. I wonder if it would be feasible to make one test file that opens a bunch of windows for each test, with parameters that describe each test case, like https://github.com/web-platform-tests/wpt/blob/master/workers/modules/dedicated-worker-import-referrer.html#L61-L79. I'd prefer that approach if it is reasonable to implement, but am not against the one here

[annevk]

https://web-platform-tests.org/writing-tests/testharness.html#variants would be the way to avoid duplication I think in combination with the sub pipe https://web-platform-tests.org/writing-tests/server-pipes.html (though that doesn't seem to document how to insert a variant). @zcorpan?

I'm also gonna approve this though as I don't think we've made a decision as a project to block on such things... Up to Rob.

[rwlbuis]

https://web-platform-tests.org/writing-tests/testharness.html#variants would be the way to avoid duplication I think in combination with the sub pipe https://web-platform-tests.org/writing-tests/server-pipes.html (though that doesn't seem to document how to insert a variant). @zcorpan?

I'm also gonna approve this though as I don't think we've made a decision as a project to block on such things... Up to Rob.

Thanks, those are great ideas. I'll land this now and try to clean up later, as the similar tests for referer in the same directory will need the same treatment.

[zcorpan]

@annevk do you mean using something like

<meta name="variant" content="?variant='null'">
...
testOriginHeader({{GET[variant]}});

?

[annevk]

@zcorpan yeah, something like that (I was thinking of the referrer policy value and use some logic for the expected origin).

[annevk]

I had also forgotten that a variant basically has to be a query string so it would naturally be {{GET}}.