Skip to content

Conversation

sdangol
Copy link
Contributor

@sdangol sdangol commented Sep 19, 2025

Summary

This PR fixes some of the bugs that were introduced in the CORS middleware

Changes

Please provide a summary of what's being changed

  • Fixed issue with wildcard origins when included in array
  • Removed resolveOrigin function and made it inline
  • Added logic to check for requested method to match with allowed method in preflight request
  • Added logic to check for requested header to match with allowed headers in preflight request
  • Refactored some of the tests

Please add the issue number below, if no issue is present the PR might get blocked and not be reviewed

Issue number: closes #4509


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@pull-request-size pull-request-size bot added the size/L PRs between 100-499 LOC label Sep 19, 2025
@sdangol sdangol self-assigned this Sep 19, 2025
@boring-cyborg boring-cyborg bot added event-handler This item relates to the Event Handler Utility tests PRs that add or change tests labels Sep 19, 2025
@dreamorosi dreamorosi self-requested a review September 22, 2025 08:26
svozza
svozza previously approved these changes Sep 22, 2025
@sdangol sdangol added the do-not-merge This item should not be merged label Sep 22, 2025
@sdangol sdangol removed the do-not-merge This item should not be merged label Sep 22, 2025
@dreamorosi
Copy link
Contributor

@sdangol can you try to simplify the function a bit further? SonarCloud is reporting a higher than normal complexity.

I see that there are two instances where we check isOptions, I wonder if we can merge them into one and return the NO_CONTENT response early by rearranging or slightly repeating things. This would probably bring the cognitive complexity down.

Alternatively, even though in previous iterations we consolidated everything in its body, now that it has grown maybe we do need to split some parts.

@dreamorosi
Copy link
Contributor

Quality Gate Passed Quality Gate passed

Issues 1 New issue 0 Accepted issues

Measures 0 Security Hotspots 0.0% Coverage on New Code 0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Almost there :D

Copy link

@sdangol sdangol merged commit dd368fa into main Sep 22, 2025
37 checks passed
@sdangol sdangol deleted the bug/cors-behaviour branch September 22, 2025 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

event-handler This item relates to the Event Handler Utility size/L PRs between 100-499 LOC tests PRs that add or change tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: CORS middleware not working as expected in some conditions

3 participants