-
Notifications
You must be signed in to change notification settings - Fork 38.3k
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
SPR-15708 - Add debug logging for CORS rejections #1466
Conversation
Issue: SPR-15708
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.
Thanks, could you have a look to my remarks and force push an updated commit.
@@ -126,7 +126,23 @@ protected boolean handleInternal(ServerHttpRequest request, ServerHttpResponse r | |||
List<String> requestHeaders = getHeadersToUse(request, preFlightRequest); | |||
List<String> allowHeaders = checkHeaders(config, requestHeaders); | |||
|
|||
if (allowOrigin == null || allowMethods == null || (preFlightRequest && allowHeaders == null)) { | |||
if (allowOrigin == null) { |
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.
To be moved just bellow checkOrigin
invocation ?
return false; | ||
} | ||
|
||
if (allowMethods == null) { |
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.
To be moved just bellow checkMethods
invocation ?
return false; | ||
} | ||
|
||
if ((preFlightRequest && allowHeaders == null)) { |
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.
To be moved just bellow checkHeaders
invocation ?
@@ -126,7 +126,23 @@ protected boolean handleInternal(ServerHttpRequest request, ServerHttpResponse r | |||
List<String> requestHeaders = getHeadersToUse(request, preFlightRequest); | |||
List<String> allowHeaders = checkHeaders(config, requestHeaders); | |||
|
|||
if (allowOrigin == null || allowMethods == null || (preFlightRequest && allowHeaders == null)) { | |||
if (allowOrigin == null) { | |||
logger.debug("rejecting request because CORS processor cannot determine " + |
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.
What about a message like "Rejecting CORS request because '" + requestOrigin + "' origin is not allowed"
} | ||
|
||
if (allowMethods == null) { | ||
logger.debug("rejecting request because CORS processor cannot determine " + |
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.
Similar remark than for origin for the message
} | ||
|
||
if ((preFlightRequest && allowHeaders == null)) { | ||
logger.debug("rejecting request because CORS processor cannot determine " + |
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.
Similar remark than for origin for the message
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.
I'm not going to get to this any time soon - feel free to reject or do own fix.
Fixed via 9901c38. |
Issue: SPR-15708
I have submitted the ICLA.