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

Httpsupgrades November 2024 updates - added non-normative examples and changed Https upgrade step #1

Merged
merged 4 commits into from
Nov 26, 2024

Conversation

meacer
Copy link
Collaborator

@meacer meacer commented Nov 26, 2024

Chris, could you PTAL before I pull these into the actual PR?

I had to move the upgrade step (section 4.1.5 before, 4.1.11 now) right after the HSTS step to fix the issue at whatwg#1655 (comment)

However, the HTTPS upgrade now happens after the mixed content upgrade step (4.1.5 in the unmodified spec). So perhaps mixed content upgrade should happen after the HSTS upgrade (4.1.10 in the unmodified spec) in the first place? Do you think this is a bug in the unmodified spec or am I missing something?

@christhompson
Copy link
Collaborator

christhompson commented Nov 26, 2024

In Chrome at least, the order of precedence is:

  • Mixed content upgrade/block
  • HSTS upgrade (we check if HSTS is enforced and if so defer upgrades to HSTS)
  • HTTPS-Upgrades and fallback

HTTPS-RR happen after all of this as it requires actually going out on the network for the DNS lookup (conceptually, we can think of it as a sort of anti-downgrade protection given all the other upgrade sources). Technically HSTS happens after HTTPS-Upgrades as well and we just skip Upgrades if we know HSTS will be enforced (so that we can do the right warning precedence if HTTPS-First Mode is enabled).

fetch.bs Outdated Show resolved Hide resolved
@christhompson
Copy link
Collaborator

The update LGTM. I think the new ordering makes sense, and the HTTPS RR subtlety is already called out in the note above (that is, HTTPS RR can only happen after we already make a connection, so it is kind of in the wrong place or shouldn't even be in the Fetch spec and just be part of HTTP).

@meacer
Copy link
Collaborator Author

meacer commented Nov 26, 2024

Thanks! Will merge into the main PR with the notes we discussed.

@meacer meacer merged commit afe8ea2 into httpsupgrades Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants