You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by GitBox <gi...@apache.org> on 2021/10/12 13:56:37 UTC

[GitHub] [cxf] vladimirfx opened a new pull request #862: Fix not unique JAXRS method candidates WARN caused by CrossOriginResourceSharingFilter

vladimirfx opened a new pull request #862:
URL: https://github.com/apache/cxf/pull/862


   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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] reta commented on a change in pull request #862: Fix not unique JAXRS method candidates WARN caused by CrossOriginResourceSharingFilter

Posted by GitBox <gi...@apache.org>.
reta commented on a change in pull request #862:
URL: https://github.com/apache/cxf/pull/862#discussion_r727966559



##########
File path: rt/rs/security/cors/src/main/java/org/apache/cxf/rs/security/cors/CrossOriginResourceSharingFilter.java
##########
@@ -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));

Review comment:
       👍 , also works, we already have `JAXRSUtils::setMessageContentType`, introducing `get` counterpart would make sense




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] vladimirfx commented on a change in pull request #862: Fix not unique JAXRS method candidates WARN caused by CrossOriginResourceSharingFilter

Posted by GitBox <gi...@apache.org>.
vladimirfx commented on a change in pull request #862:
URL: https://github.com/apache/cxf/pull/862#discussion_r727712791



##########
File path: rt/rs/security/cors/src/main/java/org/apache/cxf/rs/security/cors/CrossOriginResourceSharingFilter.java
##########
@@ -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));

Review comment:
       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?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] reta commented on pull request #862: Fix not unique JAXRS method candidates WARN caused by CrossOriginResourceSharingFilter

Posted by GitBox <gi...@apache.org>.
reta commented on pull request #862:
URL: https://github.com/apache/cxf/pull/862#issuecomment-941853762


   Thanks for the pull request, @vladimirfx , could you please open an issue in the JIRA tracker (https://issues.apache.org/jira/projects/CXF)?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] reta commented on pull request #862: Fix not unique JAXRS method candidates WARN caused by CrossOriginResourceSharingFilter

Posted by GitBox <gi...@apache.org>.
reta commented on pull request #862:
URL: https://github.com/apache/cxf/pull/862#issuecomment-941853762






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] reta commented on a change in pull request #862: Fix not unique JAXRS method candidates WARN caused by CrossOriginResourceSharingFilter

Posted by GitBox <gi...@apache.org>.
reta commented on a change in pull request #862:
URL: https://github.com/apache/cxf/pull/862#discussion_r727647974



##########
File path: rt/rs/security/cors/src/main/java/org/apache/cxf/rs/security/cors/CrossOriginResourceSharingFilter.java
##########
@@ -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));

Review comment:
       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`)




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] reta commented on pull request #862: Fix not unique JAXRS method candidates WARN caused by CrossOriginResourceSharingFilter

Posted by GitBox <gi...@apache.org>.
reta commented on pull request #862:
URL: https://github.com/apache/cxf/pull/862#issuecomment-942211042


   > I want but do not know how to do that. Specifically, how can I check logs for specific WARN messages?
   > 
   
   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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] vladimirfx commented on pull request #862: Fix not unique JAXRS method candidates WARN caused by CrossOriginResourceSharingFilter

Posted by GitBox <gi...@apache.org>.
vladimirfx commented on pull request #862:
URL: https://github.com/apache/cxf/pull/862#issuecomment-941935955


   > 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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] vladimirfx commented on pull request #862: Fix not unique JAXRS method candidates WARN caused by CrossOriginResourceSharingFilter

Posted by GitBox <gi...@apache.org>.
vladimirfx commented on pull request #862:
URL: https://github.com/apache/cxf/pull/862#issuecomment-941935955


   > 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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] reta commented on a change in pull request #862: Fix not unique JAXRS method candidates WARN caused by CrossOriginResourceSharingFilter

Posted by GitBox <gi...@apache.org>.
reta commented on a change in pull request #862:
URL: https://github.com/apache/cxf/pull/862#discussion_r727647974



##########
File path: rt/rs/security/cors/src/main/java/org/apache/cxf/rs/security/cors/CrossOriginResourceSharingFilter.java
##########
@@ -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));

Review comment:
       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`)

##########
File path: rt/rs/security/cors/src/main/java/org/apache/cxf/rs/security/cors/CrossOriginResourceSharingFilter.java
##########
@@ -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));

Review comment:
       👍 , also works, we already have `JAXRSUtils::setMessageContentType`, introducing `get` counterpart would make sense




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] vladimirfx commented on a change in pull request #862: Fix not unique JAXRS method candidates WARN caused by CrossOriginResourceSharingFilter

Posted by GitBox <gi...@apache.org>.
vladimirfx commented on a change in pull request #862:
URL: https://github.com/apache/cxf/pull/862#discussion_r727712791



##########
File path: rt/rs/security/cors/src/main/java/org/apache/cxf/rs/security/cors/CrossOriginResourceSharingFilter.java
##########
@@ -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));

Review comment:
       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?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] reta commented on pull request #862: Fix not unique JAXRS method candidates WARN caused by CrossOriginResourceSharingFilter

Posted by GitBox <gi...@apache.org>.
reta commented on pull request #862:
URL: https://github.com/apache/cxf/pull/862#issuecomment-941855544


   Could you please add a test case which at least reproduces the problem? Thank you.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org