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

🚀 [Feature]: allow users to disable caching of CORS-preflight responses #2609

Closed
3 tasks done
jub0bs opened this issue Aug 29, 2023 · 3 comments · Fixed by #2649
Closed
3 tasks done

🚀 [Feature]: allow users to disable caching of CORS-preflight responses #2609

jub0bs opened this issue Aug 29, 2023 · 3 comments · Fixed by #2649

Comments

@jub0bs
Copy link

jub0bs commented Aug 29, 2023

Feature Description

Omitting the Access-Control-Max-Age header from a preflight response leads browsers to cache that response for 5 seconds, whereas including

Access-Control-Max-Age: 0

in a preflight response instructs browsers not to cache that preflight response. However, Fiber CORS middleware ignores that distinction and takes a MaxAge value of 0 as a cue to omit the Access-Control-Max-Age header. Therefore, Fiber prevents its users from disabling caching of preflight responses.

Checklist:

  • I agree to follow Fiber's Code of Conduct.
  • I have checked for existing issues that describe my suggestion prior to opening this one.
  • I understand that improperly formatted feature requests may be closed without explanation.
@ReneWerner87
Copy link
Member

ok good point, then we would have to create the possibility to distinguish between the not set (no header) and the 0 value

@jub0bs
Copy link
Author

jub0bs commented Aug 30, 2023

@ReneWerner87 Indeed. If that's any comfort, CORS middleware other than Fiber's are similarly limited:

FWIW, retrofitting Fiber to understand this distinction (0 value set/unset) may be difficult with the current config-struct approach; I discussed this topic in a recent talk. I can think of four possibilities:

  1. Adding a CustomMaxAge bool field to the Config struct type, which users would have to set to true in addition to setting MaxAge; not very ergonomic.
  2. Adding a non-exported customMaxAge bool field and some SetMaxAge(seconds int) method on the Config struct; but that would be weird, because that would force users to configure CORS via both struct fields and methods on that struct.
  3. Change the type of MaxAge from int to *int, where a nil MaxAge would indicate that users haven't set a max age; but that would be a backwards-incompatible API change.
  4. Overhaul the CORS middleware and adopt the functional-options pattern instead of using a config struct; but that would mean an even bigger backwards-incompatible API change.

@jub0bs
Copy link
Author

jub0bs commented Sep 8, 2023

rs/cors recently fixed this problem by instructing users to set the MaxAge field to a negative integer in order to disable preflight caching. I don't think this solution is ideal, but it works well enough. Perhaps something to consider for Fiber too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants