users@jax-rs-spec.java.net

[jax-rs-spec users] Re: [jsr339-experts] Re: HEADS-UP, IMPORTANT: Problems with JAX-RS interceptors

From: Sergey Beryozkin <sberyozkin_at_talend.com>
Date: Mon, 30 Apr 2012 16:14:02 +0100

On 30/04/12 16:05, Martin Matula wrote:
> Hi Bill,
>
> On 4/30/12 4:30 PM, Bill Burke wrote:
>>> 2) readEntity() should make an implicit call to bufferEntity() before it
>>> does anything. This is just for filters! Not for reading in the resource
>>> method or the client.
>>
>> Just a thought..Maybe it shouldn't be implicit? Maybe the filter is
>> reading the entity, then replacing it with writeEntity(), so a buffer
>> is overkill? Or maybe its just better to protect the filter developer
>> from himself? I don't know...
>
> OK, not implicitly buffering may provide more flexibility. On the other
> hand one poorly written filter can break everybody else. Not sure what's
> better. I don't have a strong opinion on this one.

IMHO it has to be optional. Wouldn't the buffering affect the
well-written filters which replace the entity stream after doing
readEntity ?

I believe bufferEntity() needs to go from the api completely, but I
guess it's off-topic for this thread

Cheers, Sergey
>
>>
>>> 3) maybe - remove get/setEntityStream() from filters (could not find a
>>> use-case that could not be covered by interceptors or a combination of
>>> an interceptor and a filter that triggers the interception by calling
>>> bufferEntity()). Or maybe we want to keep it to be safe. Which leaves us
>>> with just the two things above.
>>>
>>> Here is the rationale:
>>>
>>> - on the receiving end, you want "decoding" interceptors to work based
>>> on the headers, not based on the annotation - i.e. if the entity in the
>>> incoming message is zipped, it is not the resource method that tells me
>>> it is, it is the sender who should tell me - by the way of the headers.
>>> So, typically no annotations processing should be needed to make the
>>> stream usable for other consumers. At least I think I would never add a
>>> feature that would not have this kind of characteristics.
>>>
>>
>> hashes or signatures?
>
> Shouldn't the hashes/signatures be verified based on headers then? I.e.
> if the sender sends a hash/signature, don't you always want to verify?
> And only use the annotation for enforcement? That way your verification
> interceptor can be global and the enforcement would be name-bound. And
> things would work. Or do you need to read something from the annotations
> which is essential for the signature/hash computation?
>
> Otherwise I don't know how to implement it cleanly - I guess you would
> have to buffer headers (if you want to buffer the original stream) as
> well, since after my gzip interceptor removes the encoding header and
> somebody later reads the original stream from the buffer, my gzip
> interceptor will no longer know it has to decode - unless it gets the
> buffered original headers as well, no? The "mark" in the property bag
> would not be enough in this case, as the interceptor does not know when
> it works with the original stream as opposed to a new stream created as
> a result of writeEntity(). Seems to make things too complicated, no?
>
>>
>>
>>> - given that, with the current API, I can implement gzip/decryption
>>> either as a prematching filter (by wrapping the stream using
>>> get/setEntityStream() on the filter context) - this is not reusable by
>>> the client; or even better (and reusable by the client) as an untyped
>>> global (unbound) interceptor. The interceptor can wrap the entity stream
>>> and remove the encoding info from the header, to make sure it does not
>>> try to unzip an already unzipped stream later in the processing. It may
>>> be a good practice to add a "mark" to the request property bag
>>> (indicating the message was zipped or encrypted - which can be useful if
>>> you want to have encryption enforcement filter somewhere in the chain
>>> that ensures that only encrypted messages are allowed).
>>>
>>
>> FYI, resteasy implements gzip as an interceptor. The code is reusable
>> on client and server.
>
> Makes sense.
>
> [skipping part of the message]
>>
>>
>> One last issue:
>> * If you call writeEntity() what should happen to headers? For
>> example, if the original wire bytes are gzip encoded and you call
>> writeEntity() what happens to the Content-Encoding header? In this
>> case, maybe we writeEntity() should have an additional headers parameter?
>>
>> public <T> void writeEntity(
>> final MultivalueMap<String, Object> newHeaders,
>> final GenericType<T> genericType,
>> final Annotation annotations[],
>> final MediaType mediaType,
>> final T entity);
>
> What would you pass to the header parameter?
> writeEntity triggers interceptors, based on the current headers, the
> gzip interceptor would either kick in or not, no?
>
>>
>> * BTW I think writeEntity() is the most dangerous method of all.
>> readEntity() we can get around potential problems with the workaround
>> of buffering the entity. writeEntity() has no such guarantees.
>
> Another thing which would be less error prone if the gzip, encrypt, hash
> and signature interceptors are global and only the enforcement
> interceptors/filters are name-bound. You would verify everything when
> buffering/reading for the first time, add marks to properties and the
> enforcement filter/interceptor would later just check that signature was
> present and valid in the original stream by the way of checking the mark.
> This way even writeEntity should work in most cases (as it would kick in
> after these interceptors have done their work).
>
> One more thing - should annotations[] be added to the client API
> somewhere (e.g. to the methods that work with entity or to the Entity
> class) to make name-bound interceptors reusable on the client side?
> Martin