-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
hostinfo should expose NOTSAMESITE_HOST #15819
Comments
FWIW, you should feel free to modify |
Since the value is derived from the "alternate_host" configuration, I think it would be more clear to expose it as |
The important part from my point of view is that it is always "not same site" with the main host, in the sense of having a different TLD+1. I wanted to make that somewhat clear in the naming so someone doesn't accidentally change it to no longer have that property. I don't particularly care what the name is, as long as we can't accidentally break the above invariant (e.g. if we have a test to ensure that it holds). |
We should not expose it under a different name as there's already tests all over using HTTP_NOTSAMESITE_ORIGIN and HTTPS_NOTSAMESITE_ORIGIN. Furthermore, "same site" is the established terminology, see https://url.spec.whatwg.org/#host-same-site for instance. An alternative name that would be acceptable if someone wanted to rename things would be "CROSSSITE_HOST", but that doesn't seem worth the effort. (Personally I always use those the origin constants by the way, but I haven't really played with things where I need a "raw" host recently I suppose.) |
@bzbarsky btw, I'd be happy to fix this if this is blocking something for you, but I also don't really see why you couldn't just add a test where you need it and expose it at the same time. |
This is not blocking me per se; it's needed to address my review comments at #13823 (comment) |
As it happens, we just yesterday started talking about testing the files in However, the test for this file could only really assert that the JavaScript value matches the server configuration value. In order to provide the kind of assurance you'd like, we'll need to extend the server. It should verifies that the operator has not misconfigured the server by selecting an "alternate" host which is identical or otherwise related to the "primary" host. That's an important invariant regardless of if/how we expose the value in
Thanks for that context, @annevk. The conventions on the web and in the tests trump the convention in the WPT infrastructure. There might still be an opportunity to improve consistency, though. Maybe the |
Right now it doesn't seem to.
@jgraham
The text was updated successfully, but these errors were encountered: