-
Notifications
You must be signed in to change notification settings - Fork 227
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
CORS issue in creating origin headers #355
Comments
Thank you for reporting the issue.
Yes, it seems to be a bug, and CORS handler should return some specific return isCorsOriginAllowed(origin, options)
? { "access-control-allow-origin": origin, vary: "origin" }
: {}; Then, although this is a design mistake
This is my design as the original author of the feature. I wanted to treat "*" and "null" as special cases and wanted developers to be aware of them apart from specific origins, and also wanted to handle the former apart from the latter in the code. But, thinking in again, it would be more convenient if developers can specify the only one allowed origin as an unwrapped string or RegExp. To avoid breaking changes, how about the following hot-fix? // As-Is
export interface H3CorsOptions {
origin?:
| "*"
| "null"
| (string | RegExp)[]
| ((origin: string) => boolean)
}
// To-Be
export interface H3CorsOptions {
origin?:
| "*"
| "null"
| string // (1)
| [RegExp, string] // (2)
| (string | RegExp | [RegExp, string])[] // (3)
| ((origin: string) => boolean | string) // (4)
}
Sorry to bother you, but do you have any thoughts on this issue? My idea above is based on my guess that you don't want to introduce breaking changes and bump up to new major version. But, if possible, there would be more sophisticated API. Any feedback from you is also welcome. |
@nozomuikuta great! That should work. I'm thinking about one more thing in the For me, it was not clear how to set the |
Environment
Node: v18.15.0
npm: v9.5.0
Nuxt: v3.2.3 + nuxt-security v0.11.0
Reproduction
https://github.com/maxdzin/nuxt-cors-issue
Describe the bug
When working on API, I used nuxt-security and I noticed that the
Access-Control-Allow-Origin
header is not set correctly.So, there could be a bug related to the just-merged
@nozomuikuta/h3-cors
into H3.There is a discussion about the CORS problems in this nuxt-security issue and it is related to
h3-cors
things.In short, there's a response 'Access-Control-Allow-Origin' header is not set correctly, while the options contain specific origin values.
So, there are these problems:
H3CorsOptions.origin
value is set as an array of strings or an array of RegExp, theaccess-control-allow-origin
header is not set in this case. In order to set that response header correctly, I believe thatcreateOriginHeaders
method must return withaccess-control-allow-origin
value set in the last return block here:So, if the
isCorsOriginAllowed
is false, it returns an empty object. But, it must be set byoriginOption
value instead.The string type is missing there.
Additional context
No response
Logs
No response
The text was updated successfully, but these errors were encountered: