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

Specify the default age value used when Access-Control-Max-Age isn't present #990

Closed
ehsan opened this issue Jan 7, 2020 · 8 comments · Fixed by #992
Closed

Specify the default age value used when Access-Control-Max-Age isn't present #990

ehsan opened this issue Jan 7, 2020 · 8 comments · Fixed by #992

Comments

@ehsan
Copy link

ehsan commented Jan 7, 2020

Chromium uses 5 seconds as a default. I'm modifying Gecko accordingly in https://bugzilla.mozilla.org/show_bug.cgi?id=1607615. There is a WPT test which fails only in Firefox because we currently don't consider a default for when this header is missing.

@annevk
Copy link
Member

annevk commented Jan 8, 2020

From what I recall the default was supposed to be 24h, but maybe that's only what Jonas intended to ship in Firefox.

@yutakahirano do you know why Chrome has such a low default?

@yutakahirano
Copy link
Member

I don't know. It seems the constant was introduced in the WebKit era (search for "= 5;" in the diff).

https://chromium.googlesource.com/chromium/src/+/ac64933b073b0e2280fb19a4d2f9feb561e3403f%5E%21/#F19

@annevk
Copy link
Member

annevk commented Jan 9, 2020

Alright, I guess I'll add 5 to the specification and then if we get complaints about that we can all come together and figure out something better. 😊

@annevk
Copy link
Member

annevk commented Jan 9, 2020

Oh wait, the default today is 0, and the imposed limit was supposed to be 24h, I remembered that wrong. Okay, will change the default to 5.

annevk added a commit that referenced this issue Jan 9, 2020
Already covered by web-platform-tests.

Fixes #990.
@ehsan
Copy link
Author

ehsan commented Jan 14, 2020

One question that I have is what to do when the response contains Access-Control-Max-Age: \r\n, should we use the default value in that case as well?

@annevk
Copy link
Member

annevk commented Jan 14, 2020

That's not really well defined (see #814). Ideally it's consistent with how we parse delta-seconds elsewhere. If no input results in failure, it should be 5 as well.

@ehsan
Copy link
Author

ehsan commented Jan 14, 2020

Thanks!

@ehsan
Copy link
Author

ehsan commented Jan 14, 2020

That's not really well defined (see #814). Ideally it's consistent with how we parse delta-seconds elsewhere. If no input results in failure, it should be 5 as well.

web-platform-tests/wpt#21170 adds a test for this.

annevk added a commit that referenced this issue Jan 16, 2020
Already covered by web-platform-tests.

Fixes #990.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants