-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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 not unique JAXRS method candidates WARN caused by CrossOriginResourceSharingFilter #862
base: main
Are you sure you want to change the base?
Conversation
… candidates for handling the current request which can lead to unpredictable results' warn
Thanks for the pull request, @vladimirfx , could you please open an issue in the JIRA tracker (https://issues.apache.org/jira/projects/CXF)? |
@@ -290,8 +291,21 @@ private OperationResourceInfo findPreflightMethod( | |||
String httpMethod, | |||
MultivaluedMap<String, String> values, | |||
Message m) { | |||
final String contentType = MediaType.WILDCARD; | |||
final MediaType acceptType = MediaType.WILDCARD_TYPE; | |||
Map<String, List<String>> protocolHeaders = CastUtils.cast((Map<?, ?>)m.get(Message.PROTOCOL_HEADERS)); |
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 would suggest to use HttpHeadersImpl
:
final HttpHeaders headers = new HttpHeadersImpl(m);
MediaType contentType = headers.getMediaType();
if (contentType == null) {
contentType = MediaType.WILDCARD_TYPE;
}
final List<MediaType> acceptTypes = headers.getAcceptableMediaTypes();
(which would need to introduce JAXRSUtils::findTargetMethod
that accepts content type as MediaType
)
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.
This implementation is based on the main JAXRS routing code. It is really a good idea to differ that logic?
I think it is best to extract some common code for extraction of content-type and accept headers to JAXRSUtils. What do you think?
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.
👍 , also works, we already have JAXRSUtils::setMessageContentType
, introducing get
counterpart would make sense
Could you please add a test case which at least reproduces the problem? Thank you. |
I want but do not know how to do that. Specifically, how can I check logs for specific WARN messages? Now there is a "quick and dirty" implementation that set MediaType.WILDCARD as content type and accept. Obviously it produces non-deterministic results when lookup the method in services with sophisticated mime type-based operation descrimination. I've just tried to mimic operation selection logic from the main JAXRS routing code. |
We do have examples, please check https://github.com/apache/cxf/blob/master/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/impl/ConfigurationImplTest.java#L142, happy to help if you run into difficulties with that. Thank you. |
Fix 'JAXRSUtils : Both Resource#method1 and Resource#method2 are equal candidates for handling the current request which can lead to unpredictable results' warning by using real 'Accept' and 'Content-Type' values for operation lookup.