feat: support route-level CORS configuration#384
Conversation
Signed-off-by: Vantroy <vimutti@outlook.com>
Signed-off-by: Vantroy <vimutti@outlook.com>
Signed-off-by: Vantroy <vimutti@outlook.com>
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for route-level CORS configuration overrides, allowing developers to customize or disable CORS on a per-route basis using the config.cors option.
- Implemented route-level CORS configuration merging with global settings
- Added comprehensive test coverage for route-level overrides
- Updated documentation with clear examples of the new feature
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| index.js | Modified addCorsHeadersHandler to merge global and route-level CORS options, enabling per-route customization |
| test/cors.test.js | Added test case validating route-level CORS configuration including default inheritance, custom overrides, and disabling |
| README.md | Updated documentation with examples demonstrating route-level CORS configuration patterns |
Comments suppressed due to low confidence (1)
index.js:183
- The check for
cors: falsehappens too late in the execution flow. Whenreq.routeOptions.config?.cors === false, the code should skip CORS processing entirely, but currently:
- Line 160 attempts to merge options (spreading
falseis a no-op, so this doesn't work correctly) - Line 168 evaluates and potentially wraps the origin function
- Line 170 calls the origin resolver (which could be an expensive function call or async operation)
- Only at line 181 does it check if CORS should be disabled
This means for routes with cors: false, unnecessary work is done. The check should be moved before line 160 to avoid this:
// Check if CORS is disabled for this route
if (req.routeOptions.config?.cors === false) {
return next()
}
const options = { ...globalOptions, ...(typeof req.routeOptions.config?.cors === 'object' ? req.routeOptions.config.cors : {}) }function addCorsHeadersHandler (fastify, globalOptions, req, reply, next) {
const options = { ...globalOptions, ...req.routeOptions.config?.cors }
if ((typeof options.origin !== 'string' && options.origin !== false) || options.dynamic) {
// Always set Vary header for non-static origin option
// https://fetch.spec.whatwg.org/#cors-protocol-and-http-caches
addOriginToVaryHeader(reply)
}
const resolveOriginOption = typeof options.origin === 'function' ? resolveOriginWrapper(fastify, options.origin) : (_, cb) => cb(null, options.origin)
resolveOriginOption(req, (error, resolvedOriginOption) => {
if (error !== null) {
return next(error)
}
// Disable CORS and preflight if false
if (resolvedOriginOption === false) {
return next()
}
// Allow routes to disable CORS individually
if (req.routeOptions.config?.cors === false) {
return next()
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Vantroy <vimutti@outlook.com>
Signed-off-by: Manuel Spigolon <behemoth89@gmail.com>
|
I would love to see this feature! Is there anything I can do to help and make this available? |
Signed-off-by: Vantroy <vimutti@outlook.com>
|
Always a bit lost on these changes, isn't this what child contexts and encapsulation is for? |
I am using this feature as part of a nestjs application that uses fastify. I want to add a route specific CORS config and this is how I am able to achieve it via the RouteConfig. @RouteConfig({
cors: { origin: ['https://example.com'] },
})
async post(@Res({ passthrough: true }) reply: FastifyReply) {
///
} |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Checklist
npm run test && npm run benchmark --if-presentand the Code of conduct
This PR adds support for configuring CORS on a per-route basis via config.cors. This allows developers to override or disable CORS behavior for specific routes, while still using the global plugin configuration as a default.