Skip to main content
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [jaxrs-dev] Clarification regarding ReaderInterceptor
Hi all,

thanks a lot for your comments. I just created #659 to track this request for clarification.

@Santiago:
The original use case is the CSRF protection in the in JSR 371 (MVC 1.0) reference implementation. The implementation is based on the idea to provide a ReaderInterceptor which checks for the existence of a valid CSRF token as a http request header or form parameter. And we discovered that the interceptor isn't invoked in CXF. But it works if we add a javax.ws.rs.core.Form parameter to the resource method signature. By the way: You wrote this class in the first place. :-)

@Andy:
I see your point that there are two distinct APIs for manipulating/converting: ParamConverter for @*Param and ReaderInterceptor for entities. However, I don't think that they are independent of each other. Imagine a ReaderInterceptor which modifies the request body of a HTML form post. What would be the expected output for this resource method:

  @POST
  public Response processForm( @FormParam("value") String value, Form form ) {
    System.out.println( "@FormParam:  " + value );
    System.out.println( "Form entity: " + form.asMap().getFirst( "value" ) );
  }

Should the values be different? I don't think so. IMO executing the MBR including all interceptors should be done first and the values for @FormParam binding should be derived from that result. This would produce consistent results. Therefore, I also think that the interceptors should always be applied if there is an HTTP request body. 

Does this make sense? I would be happy to hear more thoughts about that.

Thanks everyone for contributing to the discussion.

Christian



Am Do., 13. Sep. 2018 um 15:16 Uhr schrieb Santiago Pericas-Geertsen <santiago.pericasgeertsen@xxxxxxxxxx>:
Christian,

 1) Could you file an issue to track this? I think we are in agreement that some clarification is needed.

 2) I’m also interested in the original use case that led to finding the incompatibility. Was there an expectation that interceptors would run with @FormParam? This information could be a good addition to the issue’s text and may help guide the discussion.

— Santiago

On Sep 12, 2018, at 10:50 AM, Andy McCright <j.andrew.mccright@xxxxxxxxx> wrote:

Christian, thanks for testing.

Santiago, all, I agree that this should be clarified.  As it is today, some apps could suffer when porting from one vendor to another.  I would like to see it clarified in the spec document, but maybe there is a good place to clarify this in the javadoc too? 

Personally, I like the CXF behavior because it allows the implementation to optimize the pipeline flow when the request includes form parameters but no explicit entity in the request body.  And since there are other mechanisms to convert/manipulate the form parameters, users don't lose any functionality in order to gain the performance improvement.  I think it also is consistent in the sense that users should use ParamConverters for *Param annotated parameters, and MBRs/ReaderInterceptors for unannotated entity parameters.  

Thanks,

Andy

On Wed, Sep 12, 2018 at 12:01 AM Christian Kaltepoth <christian@xxxxxxxxxxxx> wrote:
Hi all,

thanks a lot for your comments.

@Santiago: 
I can confirm that CXF applies the interceptor chain if the request entity is bound using "javax.ws.rs.core.Form". But it does not execute it if the resource uses @FormParam to access the parameters and no entity parameter is present.

@all:
It looks like there are different opinions about whether or not the interceptor chain should be executed in the code I posted above. I see good arguments for both ways to interpret the spec. 
  • On the one hand there is no MBR required in my example, because there is no entity parameter. So executing the interceptor chain even if there is no MBR seems weird.
  • On the other hand there is a request body posted to the server, so executing the interceptor chains seems reasonable because JAX-RS MUST parse the posted data to get the form parameter values.
I've mixed feelings about this topic. But I think that the expected behavior should be clarified.

Christian


Am Di., 11. Sep. 2018 um 23:55 Uhr schrieb Santiago Pericas-Geertsen <santiago.pericasgeertsen@xxxxxxxxxx>:
Hi Andy,

 I understand how this can be interpreted in different ways. In general, I think running interceptors is useful, but we may need to clarify this either way. 

 Do you handle the type Form in a special manner too? E.g., if a client does a post using Form, is the interceptor chain called?

— Santiago

On Sep 11, 2018, at 12:40 PM, Andy McCright <j.andrew.mccright@xxxxxxxxx> wrote:

Hi Christian, Santiago,

I might be taking the spec too literally, but I would view that the @FormParam annotation indicates that foobar is a parameter, not an entity.  The JAX-RS spec shows that the MBR interceptor chain is optional in Appendix C.  In this case, I think CXF is optimizing the pipeline since it does not see an entity.  If I expand on the example code and use a ParamConverterProvider/ParamConverter, I see that CXF invokes them - allowing me to manipulate the value that is ultimately passed to the post method.   I think that this should be the proper way to "intercept" @FormParam (or any other @*Param parameters).

Similarly, CXF will invoke the MBR and ReaderInterceptor if I remove the @FormParam annotation from the parameter - IMO, that changes the foobar parameter from being a parameter to to being an entity.

Christian, would it be possible for you to try those options too? (1) Remove @FormParam and process foobar as an entity and (2) Use a ParamConverterProvider to process foobar as a parameter?

Here is the ParamConverterProvider I used:
@Provider
public class MyParamConverterProvider implements ParamConverterProvider {
    private final static Logger _log = Logger.getLogger(MyParamConverterProvider.class.getName());

    @SuppressWarnings("unchecked")
    @Override
    public <T> ParamConverter<T> getConverter(Class<T> aClass, Type type, Annotation[] annos) {
        _log.info("getConverter");
        if(String.class.isAssignableFrom(aClass)) {
            return (ParamConverter<T>) new ParamConverter<String>(){

                @Override
                public java.lang.String toString(String s) {
                    return "toString"+s;
                }

                @Override
                public String fromString(String s) {
                    _log.info("fromString " + s);
                    return "fromString" + s;
                }};
        }
        return null;   
    }
}


Thanks,

Andy

On Tue, Sep 11, 2018 at 8:13 AM Santiago Pericas-Geertsen <santiago.pericasgeertsen@xxxxxxxxxx> wrote:
Hi Christian,

 What would be the rationale for not invoking the interceptor? If there’s an entity and a MBR, there should be a (possibly empty) interceptor chain.

— Santiago

On Sep 11, 2018, at 1:22 AM, Christian Kaltepoth <christian@xxxxxxxxxxxx> wrote:

Hi all,

I've a quick question and would love to get your feedback.

Let's assume there is ReaderInterceptor like this:

  @Provider
  public class MyReaderInterceptor implements ReaderInterceptor {

    public Object aroundReadFrom(ReaderInterceptorContext context) {
      // do stuff
    }

  }

And the following JAX-RS resource class:

  @Path("some-path")
  public class SomeResource {

    @POST
    public Response post(@FormParam("foobar") String foobar) {
      // do stuff
    }

  }

If a client sends a POST request to the resource and submits the form parameter in the body, would you expect the ReaderInterceptor to get invoked or not?

I'm asking because JAX-RS implementations handle this case differently. Jersey and RESTEasy seem to invoke the interceptor, but CXF does not.

Any thoughts?

Christian


_______________________________________________
jaxrs-dev mailing list
jaxrs-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jaxrs-dev

_______________________________________________
jaxrs-dev mailing list
jaxrs-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jaxrs-dev
_______________________________________________
jaxrs-dev mailing list
jaxrs-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jaxrs-dev

_______________________________________________
jaxrs-dev mailing list
jaxrs-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jaxrs-dev


--
Christian Kaltepoth

_______________________________________________
jaxrs-dev mailing list
jaxrs-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jaxrs-dev
_______________________________________________
jaxrs-dev mailing list
jaxrs-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jaxrs-dev

_______________________________________________
jaxrs-dev mailing list
jaxrs-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jaxrs-dev


--
Christian Kaltepoth
Blog: http://blog.kaltepoth.de/
Twitter: http://twitter.com/chkal
GitHub: https://github.com/chkal

Back to the top