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

Consider logging a warning if @CrossOrigin is on a non-OPTIONS method (or revise existing error message) #3655

Closed
tjquinno opened this issue Nov 18, 2021 · 9 comments · Fixed by #3932
Assignees
Labels
2.x Issues for 2.x version branch cors Related to CORS support enhancement New feature or request MP P3

Comments

@tjquinno
Copy link
Member

tjquinno commented Nov 18, 2021

Environment Details

  • Helidon Version: 2.x
  • Helidon SE or Helidon MP MP
  • JDK version:
  • OS:
  • Docker version (if applicable):

Problem Description

We ask Helidon MP app developers to CORS-enable their endpoints by adding @CrossOrigin annotations to @OPTIONS methods on their JAX-RS resource classes. If they incorrectly add the annotation to the non-OPTIONS endpoints, Helidon logs a mysterious error:

java.lang.IllegalArgumentException: Could not locate expected CORS information while preparing response to request RequestAdapterMp{path=/network/nodes, method=GET, headers={Origin=[http://localhost:8000], Accept=[application/json, text/plain, */*], Connection=[keep-alive], Referer=[http://localhost:8000/], User-Agent=[Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/95.0.4638.69 Safari/537.36], Sec-Fetch-Dest=[empty], Sec-Fetch-Site=[same-site], Host=[localhost:7001], Accept-Encoding=[gzip, deflate, br], Sec-Fetch-Mode=[cors], sec-ch-ua=["Google Chrome";v="95", "Chromium";v="95", ";Not A Brand";v="99"], sec-ch-ua-mobile=[?0], sec-ch-ua-platform=["macOS"], Accept-Language=[en-US,en;q=0.9]}}
        at io.helidon.webserver.cors.CorsSupportHelper.lambda$prepareResponse$3(CorsSupportHelper.java:355)
        at java.base/java.util.Optional.orElseThrow(Optional.java:408)
        at io.helidon.webserver.cors.CorsSupportHelper.prepareResponse(CorsSupportHelper.java:355)
        at io.helidon.webserver.cors.CorsSupportBase.prepareResponse(CorsSupportBase.java:90)
        at io.helidon.microprofile.cors.CorsSupportMp.prepareResponse(CorsSupportMp.java:72)
        at io.helidon.microprofile.cors.CrossOriginFilter.filter(CrossOriginFilter.java:76)
...

This error could occur in other circumstances, but developers typically see it if they have annotated the wrong method.

Although our doc is pretty clear, it might be worth adding a runtime warning if developers have misplaced the @CrossOrigin annotation.

Steps to reproduce

TBD

@tjquinno tjquinno self-assigned this Nov 18, 2021
@tjquinno tjquinno added 2.x Issues for 2.x version branch cors Related to CORS support enhancement New feature or request MP labels Nov 18, 2021
@spericas
Copy link
Member

It could even be a fatal error at startup or during annotation processing I suppose.

@barchetta barchetta added the P3 label Nov 18, 2021
@AbhideepChakravarty
Copy link

So what is the solution? I have a DELETE method and when I create a OPTIONS like

@path("/removeFavLocation/{id}")
@options
public void optionsForRemoveFromFavorite() {
}

No matter if I add CROSSORIGIN or not, I get the same error. What I am doing wrong?

@spericas
Copy link
Member

spericas commented Feb 15, 2022

@AbhideepChakravarty Are you not using CORS at all? If not, you should be able to exclude it from you classpath I suppose.

@spericas spericas self-assigned this Feb 15, 2022
@spericas
Copy link
Member

spericas commented Feb 15, 2022

@AbhideepChakravarty Some quick testing shows that in order to get that exception in the logs the following conditions likely hold:

  1. @OPTIONS without @CrossOrigin on a method
  2. Helidon CORS in the classpath
  3. Helidon determines a request is not normal, i.e. it is a CORS request

For (3), Helidon checks if ORIGIN is absent or if present that it contains the same info as the HOST header (sort of). Perhaps you can share the request that is triggering the exception for further analysis.

@spericas
Copy link
Member

@AbhideepChakravarty Any chance you can share more info about that request?

@AbhideepChakravarty
Copy link

I got the issue. I need up with http method on the client side and created mess. When I corrected things, everything worked fine. But I am not yet sure what was the root cause of the problem. As the original post, a warning it critical error would help, if given with proper reason.

@spericas
Copy link
Member

@AbhideepChakravarty As explained above, I think somehow your requests included an ORIGIN header for some reason (maybe accidentally?). I suggest that we close this issue for now then and re-open with more information at a later time.

@AbhideepChakravarty
Copy link

Sure we can do that. If we can find a way to reproduce it and document it properly, we should open it.

@tjquinno
Copy link
Member Author

The original purpose of the issue remains - warning or failing during start-up if the developer has placed a @CrossOrigin annotation on a non-OPTIONS method, so I'm re-opening the issue.

@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
2.x Issues for 2.x version branch cors Related to CORS support enhancement New feature or request MP P3
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants