Skip to content
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

4.x CORS does not account for custom headers and other conditions in deciding if a request is CORS or normal #8167

Closed
tjquinno opened this issue Dec 19, 2023 · 2 comments
Assignees
Labels
4.x Version 4.x bug Something isn't working cors Related to CORS support P2
Milestone

Comments

@tjquinno
Copy link
Member

tjquinno commented Dec 19, 2023

Environment Details

  • Helidon Version: 4.0.1
  • Helidon SE or Helidon MP
  • JDK version:
  • OS:
  • Docker version (if applicable):

Problem Description

Generally, an incoming request with a matching origin and host (scheme, node, and port) is treated as a normal (non-CORS) "same-origin" request. But there are some cases in which the CORS protocol should subject a same-origin request to CORS processing in some situations, including the following: (see this page)

  • The length of any header's value exceeds 128.
  • The request asks to allow headers outside the set of CORS-allowed headers, for example: Accept, Accept-Language, Content-Language, Content-Type, and Range).
  • The value of an Accept header contains an unsafe byte.
  • The value of Content-Language contains a byte outside certain ranges.
  • The value of Content-Type contains an unsafe byte or is not one of application/x-www-form-urlencoded, multipart/form-data, or text/plain.
  • The Range value does not parse as a valid range or if the first range value is 0.

Users have noticed that, in some such cases, a browser triggers the CORS protocol for a same-origin request.

This issue is to make sure that the Helidon server-side CORS implementation is behaving correctly.

@tjquinno tjquinno added bug Something isn't working cors Related to CORS support 4.x Version 4.x labels Dec 19, 2023
@m0mus m0mus added the P2 label Dec 21, 2023
@m0mus m0mus added this to the 4.0.3 milestone Dec 21, 2023
@barchetta barchetta modified the milestones: 4.0.3, 4.0.4 Jan 8, 2024
@barchetta barchetta modified the milestones: 4.0.4, 4.1.0 Jan 25, 2024
@tjquinno
Copy link
Member Author

tjquinno commented Jul 30, 2024

Here is some clarification now that I've looked into this a bit more.

(BTW, this issue was triggered by a user who reported seeing Chrome send a preflight request for a same-host request and was wondering why.)

There is a possible Helidon bug here: the way Helidon identifies an incoming request as a preflight request might not be quite correct; I'm still digging into that more fully.

The real responsibility is on the client to detect if the CORS/fetch protocol indicates that a preflight request is needed because of any of the reasons mentioned in the issue description and then to send the preflight request, receive the CORS response, and then prepare and send the CORS request.

In addition to the bug fix we might be able to enhance the Helidon CORS logic so that if CORS is enabled and an incoming request matches any of the "unsafe" checks but is not a CORS request (as the protocol indicates it should be) then we would reject the request in some way. Have to think about that a bit more.

@barchetta barchetta modified the milestones: 4.1.0, 4.1.1 Aug 1, 2024
@tjquinno
Copy link
Member Author

tjquinno commented Aug 7, 2024

There are indeed some cases in which the user agent should treat a request as a CORS request even if the origin and host match--unsafe header names or header values that are too long or contain unsafe characters. In these cases the user agent sends a CORS preflight request which in turn triggers CORS processing on the server side.

According to the part of the fetch spec that addresses CORS for the server https://fetch.spec.whatwg.org/#http-cors-protocol, the server is not responsible for checking headers for these conditions.

As a result, I'll close this issue: the server is doing what it should.

@tjquinno tjquinno closed this as completed Aug 7, 2024
@m0mus m0mus added this to Backlog Aug 12, 2024
@m0mus m0mus moved this to Closed in Backlog Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.x Version 4.x bug Something isn't working cors Related to CORS support P2
Projects
Archived in project
Development

No branches or pull requests

3 participants