-
-
Notifications
You must be signed in to change notification settings - Fork 451
Fix: web websocket connect issue #1555
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
Reviewer's GuideExtend the WebSocket proxy configuration to support CORS preflight requests, apply CORS headers to all WebSocket responses, and enrich proxy headers with real client details. Sequence diagram for WebSocket CORS preflight handlingsequenceDiagram
actor Client
participant Nginx
participant Backend
Client->>Nginx: OPTIONS /ws (preflight)
Nginx-->>Client: 204 No Content + CORS headers
Note over Nginx,Backend: No request sent to Backend
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `nginx/nginx.conf:81` </location>
<code_context>
# WebSocket
location /ws {
+ # Handle preflight OPTIONS requests
+ if ($request_method = 'OPTIONS') {
+ add_header 'Access-Control-Allow-Origin' $http_origin always;
+ add_header 'Access-Control-Allow-Credentials' 'true' always;
+ add_header 'Access-Control-Allow-Headers' 'Authorization, Content-Type, X-Requested-With' always;
+ add_header 'Access-Control-Allow-Methods' 'GET, POST, OPTIONS' always;
+ add_header 'Access-Control-Max-Age' 1728000 always;
+ add_header 'Content-Type' 'text/plain charset=UTF-8' always;
+ add_header 'Content-Length' 0 always;
+ return 204;
+ }
+
</code_context>
<issue_to_address>
Consider using 'add_header' with 'always' only outside conditional blocks.
'add_header ... always;' is not effective inside 'if' blocks in Nginx. For reliable CORS headers on OPTIONS requests, use a dedicated location block or move header configuration outside the 'if'.
</issue_to_address>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
if ($request_method = 'OPTIONS') { | ||
add_header 'Access-Control-Allow-Origin' $http_origin always; | ||
add_header 'Access-Control-Allow-Credentials' 'true' always; | ||
add_header 'Access-Control-Allow-Headers' 'Authorization, Content-Type, X-Requested-With' always; | ||
add_header 'Access-Control-Allow-Methods' 'GET, POST, OPTIONS' always; | ||
add_header 'Access-Control-Max-Age' 1728000 always; | ||
add_header 'Content-Type' 'text/plain charset=UTF-8' always; | ||
add_header 'Content-Length' 0 always; | ||
return 204; |
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.
issue: Consider using 'add_header' with 'always' only outside conditional blocks.
'add_header ... always;' is not effective inside 'if' blocks in Nginx. For reliable CORS headers on OPTIONS requests, use a dedicated location block or move header configuration outside the 'if'.
Summary by Sourcery
Enable CORS for WebSocket connections in Nginx by handling preflight OPTIONS requests and adding requisite headers, and propagate client forwarding headers to the backend.
Enhancements:
Deployment: