-
Notifications
You must be signed in to change notification settings - Fork 11
fix(api): set max age for cors preflight caching #1647
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
Conversation
WalkthroughThe CORS middleware in the application was updated to explicitly handle OPTIONS preflight requests by responding with a 204 status and terminating further processing. The allowed HTTP methods and headers were expanded, and a new "Access-Control-Max-Age" header was added to specify the caching duration for preflight responses. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server (CORS Middleware)
Client->>Server (CORS Middleware): Sends OPTIONS preflight request
Server (CORS Middleware)-->>Client: Responds with 204 No Content and CORS headers
Note over Server (CORS Middleware): No further middleware processing
sequenceDiagram
participant Client
participant Server (CORS Middleware)
participant Next Middleware
Client->>Server (CORS Middleware): Sends non-OPTIONS request
Server (CORS Middleware)->>Next Middleware: Passes request after setting CORS headers
Next Middleware-->>Client: Handles request as usual
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/app.ts (1)
82-82
: Consider security implications of 24-hour preflight caching.The Access-Control-Max-Age header is set to 86400 seconds (24 hours). While this improves performance by reducing preflight requests, it means browsers will cache CORS policy for 24 hours. Consider if this duration aligns with your security requirements, especially if CORS policies might need rapid updates.
For more flexibility, consider a shorter duration:
- res.header('Access-Control-Max-Age', '86400'); + res.header('Access-Control-Max-Age', '3600'); // 1 hour
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app.ts
(1 hunks)
🔇 Additional comments (3)
src/app.ts (3)
79-79
: LGTM: OPTIONS method correctly added to allowed methods.Adding "OPTIONS" to the allowed methods is necessary for proper CORS preflight request handling.
80-80
: LGTM: Authorization header added to allowed headers.Adding "Authorization" to the allowed headers is essential for APIs that use bearer tokens or other authorization mechanisms.
84-87
: LGTM: Proper OPTIONS request handling.The preflight request handling correctly responds with 204 status and prevents further middleware execution, which is the standard approach for CORS preflight requests.
🎉 This PR is included in version 3.27.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
No description provided.