-
Notifications
You must be signed in to change notification settings - Fork 13k
fix: CORS headers incorrectly set for GET #36085
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
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: b87939a The changes in this PR will be included in the next version bump. This PR includes changesets to release 36 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## release-7.7.0 #36085 +/- ##
=================================================
+ Coverage 64.20% 64.75% +0.55%
=================================================
Files 3018 3110 +92
Lines 91839 93196 +1357
Branches 17433 17741 +308
=================================================
+ Hits 58967 60351 +1384
+ Misses 30185 30057 -128
- Partials 2687 2788 +101
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
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.
Pull Request Overview
This PR fixes the issue with CORS headers not being correctly sent for GET requests in the Rocket.Chat API. It ensures that the Access-Control-Allow-Origin header is always set to "*" for GET requests and updates the test cases to validate this behavior.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/meteor/app/api/server/middlewares/cors.ts | Removes the conditional check and now always sets the allow-origin header for GET requests. |
| apps/meteor/app/api/server/middlewares/cors.spec.ts | Updates test cases to expect the Access-Control-Allow-Origin header and remove expectations for other CORS headers. |
| .changeset/fuzzy-bottles-unite.md | Updates changeset documentation to reflect the fix for GET CORS headers. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
Pull Request Overview
This PR fixes an issue with CORS headers for GET requests in the Rocket.Chat API by always setting the Access-Control-Allow-Origin header to "*" regardless of the CORS settings.
- Updated the CORS middleware to always send the allow-origin header for GET requests.
- Modified test cases to validate that the allow-origin header is always present for GET requests, and removed assertions for other CORS headers.
- Added a changeset file for the patch release.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/meteor/app/api/server/middlewares/cors.ts | Updated middleware to always set the Access-Control-Allow-Origin header for GET requests. |
| apps/meteor/app/api/server/middlewares/cors.spec.ts | Revised tests reflecting the new CORS header behavior for GET requests. |
| .changeset/fuzzy-bottles-unite.md | Included changeset documentation for the patch update. |
| res.headers.set('Access-Control-Allow-Methods', defaultHeaders['Access-Control-Allow-Methods']); | ||
| res.headers.set('Access-Control-Allow-Headers', defaultHeaders['Access-Control-Allow-Headers']); | ||
| } | ||
| res.headers.set('Access-Control-Allow-Origin', '*'); |
Copilot
AI
May 27, 2025
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.
Consider adding an inline comment explaining that the Access-Control-Allow-Origin header is always set for GET requests, clarifying that this behavior is intentional as part of the fix.
|
/backport 7.6.3 |
Co-authored-by: Guilherme Gazzo <guilherme@gazzo.xyz> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Pull request #36139 added to Project: "Patch 7.6.3" |
This pull request addresses an issue with CORS (Cross-Origin Resource Sharing) headers not being properly sent for GET requests in the Rocket.Chat API. It includes changes to the middleware logic and updates to the associated test cases to reflect the new behavior.
Fixes and Enhancements to CORS Handling:
apps/meteor/app/api/server/middlewares/cors.ts: Updated the middleware to always set theAccess-Control-Allow-Originheader to*for GET requests, regardless of whether CORS is enabled in the settings.Test Updates for CORS Behavior:
apps/meteor/app/api/server/middlewares/cors.spec.ts:Access-Control-Allow-Originheader is included for GET requests, both when CORS is enabled and disabled. [1] [2] [3]Proposed changes (including videos or screenshots)
Issue(s)
Fix #36054
Steps to test or reproduce
Further comments