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

Fix missing CORS headers if response has 404 status #3206

Merged
merged 3 commits into from
Jul 21, 2021
Merged

Fix missing CORS headers if response has 404 status #3206

merged 3 commits into from
Jul 21, 2021

Conversation

tjquinno
Copy link
Member

@tjquinno tjquinno commented Jul 20, 2021

Resolves #3188

Overview

Some code had been added a while ago to the CORS logic to explicitly avoid setting CORS headers for 404 responses. This PR removes that code.

An earlier unrelated change in Helidon's Jersey support was suppressing headers added to a response if the status was 404. This PR removes that header suppression.

Two very slight behavioral changes

These are both truly bug fixes.

  1. Previously, if the Helidon Jersey ResponseWriter detected a response with status 404 and no content, it would ignore any headers already set and immediately delegate to the next handler in the chain (if any). With these changes, any headers previously set on the ContainerResponse are always copied to the ServerResponse.
  2. Previously, in MP, even if the cars.enabled config key was set to false, CORS processing would still occur for JAX-RS endpoints (as indicated by JAX-RS annotations). This PR includes changes so that config setting controls JAX-RS CORS behavior as well.

Details of changes

  • webserver/jersey
    • ResponseWriter#writeReponseStatusAndHeaders - An earlier change deferred the transmission of the response to a later handler if the response status is 404 and the content length is 0. Change to return response (not defer) if there are any headers. This handles the case where CORS is in play and has added headers to the response even for the 404 case.
  • webserver/cors
    • CorsSupportHelper#prepareResponse - If response is 404, return valid CORS headers tailored to the (failed) request instead of suppressing CORS headers.
  • CrossOriginConfig - add constant used for catch-all headers in 404 responses
  • Added and modified some tests.

Signed-off-by: tim.quinn@oracle.com tim.quinn@oracle.com

Signed-off-by: tim.quinn@oracle.com <tim.quinn@oracle.com>
@tjquinno tjquinno added this to the 2.4.0 milestone Jul 20, 2021
@tjquinno tjquinno requested a review from spericas July 20, 2021 14:19
@tjquinno tjquinno self-assigned this Jul 20, 2021
…o govern all of CORS; it used to not control CORS behavior for JAX-RS annotated endpoints
…ased on the actual request rather than wildcards
Copy link
Member

@spericas spericas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tjquinno tjquinno merged commit a9fed44 into helidon-io:master Jul 21, 2021
@tjquinno tjquinno deleted the cors-on-error branch July 21, 2021 13:30
@barchetta barchetta added this to To do in Helidon-2.3.3 Aug 9, 2021
@barchetta barchetta removed this from To do in Helidon-2.3.3 Aug 9, 2021
tjquinno added a commit that referenced this pull request Aug 17, 2021
* Fix missing CORS headers if response has 404 status

Signed-off-by: tim.quinn@oracle.com <tim.quinn@oracle.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate and fix if needed the absence of CORS headers on unsuccessful (4xx or 5xx) replies
2 participants