Skip to content
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

Chromium's implementation of use-URL-credentials probably does not match the spec #1496

Open
domenic opened this issue Sep 30, 2022 · 1 comment
Labels
interop Implementations are not interoperable with each other needs tests Moving the issue forward requires someone to write tests

Comments

@domenic
Copy link
Member

domenic commented Sep 30, 2022

Spinning off from #465 (comment) .

We suspect that Chromium implements this flag by stripping the username and password from the URL before doing the fetch, which will cause the server worker, or any redirect destinations, to observe the modified URL. Whereas in the Fetch spec, there's a separate boolean which causes the URL credentials to be not-used.

This might be just a Chromium bug, but it's worth checking at least WebKit given the shared lineage. It's possible we might want to update the spec to match Chromium's behavior instead, as arguably reducing the number of URLs with usernames/passwords in them throughout the ecosystem is nice.

First step is to write some proper web platform tests, I guess.

@domenic domenic added the interop Implementations are not interoperable with each other label Sep 30, 2022
@annevk annevk added the needs tests Moving the issue forward requires someone to write tests label Sep 30, 2022
@domenic
Copy link
Member Author

domenic commented Oct 5, 2022

Stripping the username and password also doesn't match the algorithm in HTTP-network-or-cache fetch, which roughly uses the flag only when both URL credentials and authentication side table credentials are present. In other words, if only URL credentials are present, the spec still uses them, whereas Chromium does not (since they are stripped from the URL). See #1498 (comment) for a more detailed analysis.

So yeah, tests needed in general...

domenic added a commit that referenced this issue Oct 23, 2022
Although, see #1496 for potential followup.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interop Implementations are not interoperable with each other needs tests Moving the issue forward requires someone to write tests
Development

No branches or pull requests

2 participants