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

netlify: Change 301 redirects to 302 #345

Merged
merged 1 commit into from Mar 16, 2020
Merged

Conversation

adisbladis
Copy link
Member

@adisbladis adisbladis commented Mar 15, 2020

302s are potentially cached indefinitely and it makes it very hard to change anything in the future.

This also changes /nixops/manual to a 200 (netlify proxy).

@samueldr
Copy link
Member

This may conflict with #344. Just saying for when this will be merged.

@adisbladis
Copy link
Member Author

@samueldr Should I cherry-pick your changes to this PR then?

@samueldr
Copy link
Member

As you see fit. Another option would be to remove the Nixops manual redirect from your PR I guess.

A last option is not to do anything and leave that to when it's being merged.

netlify.toml Outdated Show resolved Hide resolved
@garbas
Copy link
Member

garbas commented Mar 16, 2020

Would you like to publish manual in the same manner as nixos homepage (to netlify using github actions). And maybe even get a subdomain for it, eg. nixops.nixos.org or ops.nixos.org.

netlify.toml Outdated Show resolved Hide resolved
netlify.toml Outdated Show resolved Hide resolved
@garbas
Copy link
Member

garbas commented Mar 16, 2020

I don't have strong opinion on this, but semantics say that when you move (permanently) you should use 301 and when this would be a temporary thing then use 302. I think we should continue using 301 when possible.

@zimbatm
Copy link
Member

zimbatm commented Mar 16, 2020

Every second that we wait is another client caching the redirect permanently. It's one of the rare thing that has no TTL associated with it. It's better to use 302 by default and only use 301 when you are really really really sure that it's never going to move.

@edolstra
Copy link
Member

My understanding of the HTTP standard is that 301s are not in fact cacheable permanently, though they can be cached by default using the standard heuristics.

@zimbatm
Copy link
Member

zimbatm commented Mar 16, 2020

I guarantee you that Chrome and Firefox will cache a 301 redirect with no expiry date.
Do what you want but I recommend merging this PR and have the long conversation later.

@adisbladis
Copy link
Member Author

Indeed browsers tend to treat 301 as a permanent redirect.
Can we merge this PR before we have even more clients ending up with permanent caches?

302s are potentially cached indefinitely and it makes it very hard to
change anything in the future.
@garbas
Copy link
Member

garbas commented Mar 16, 2020

relevant why this needs to be merged. browsers cache 301 "forever" (until they ran out of cache or until cache is cleared)... https://stackoverflow.com/questions/9130422/how-long-do-browsers-cache-http-301s

@edolstra
Copy link
Member

Well, the whole point it to redirect permanently. Also, if clients have already cached it forever, it's too late to change it back.

@garbas garbas merged commit f793293 into NixOS:master Mar 16, 2020
@samueldr
Copy link
Member

samueldr commented Mar 16, 2020

Just a note, "permanent" here is not "this is permanently a redirect", but "we promise this new url is its new home forever". This annoying distinction is why 302 is more apt than 301. We can't control some of those URLs if, for any reason, the endpoint needs to be changed. Good luck getting GitHub to honor a redirect outside GitHub, or even inside GitHub depending.

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.

None yet

5 participants