On Oct 21, 2012, at 4:47 PM, Jan Algermissen <jan.algermissen_at_nordsc.com> wrote:
> [Top posting because it is really just supportive oif the overall issue]
>
> Having gone through the whole thing today, I agree with Markus that there is a severe problem with the overall 2.0 spec.
>
> Kudos for pointing at that - its a tricky thing, easily overlooked when looking at the individual features.
>
>
> The problem is the original design of the MessageBodyWriter API (Bill hinted at that already) not the new filter API:
>
> MBWs are invoked *after* filters.
>
> Filters can (and we need that use case) hook transformers into the output stream.
>
> Filters do not see any Content-Length because that is taken from the getSize() of MBWs
>
> Once MBWs are reached, they do not know (and should not care) whether the stream they are going to produce will be changed.
>
> Hence any result of getSize() that is not -1 is completely useless and error prone because it can lead to wrong Content-Length header value any time.
>
> Producing an impossible to find bug from the POV of the API user, BTW.
>
> Solutions:
>
> 1) describe exactly what JAX-RS implementations have to do. E.g. Ignore getSize() once setOutputStream( ... ) is called.
This option did not occur to me. I am wondering if it would solve all the issues though? E.g. what if I really only want to log or audit the streamed data? Does it mean that I have to still ignore the MBW.getSize()?
Marek
>
> or
>
> 2) depricate getSize()
>
> I guess 1) needs to be added and implemented by every container in any case given that we cannot deprecate getSize immediately.
>
>
> It never struck me as an issue, because I never use built-in MBWs and mine allways return -1 from getSize() but looking at this, the original design of MBWs deciding upon Content-Length vs. Chunked-Encoding as simply wrong. This is an HTTP-connector issue and should be left to that layer.
>
> Bottom line: +1 to Markus saying that 2.0 cannot be shipped without that being fixed.
>
> Jan
>
>
>
> On Oct 20, 2012, at 6:09 PM, Markus KARG wrote:
>
>> Marek,
>>
>>>> Sorry for repeating myself, but, whether to use chunked encoding or
>>> set a content-length header should be left up to the underlying HTTP
>>> engine (i.e. the servlet container). I know for a fact that Jetty and
>>> Tomcat use a buffered output stream. If you finish a request within
>>> the limits of the buffer (without calling flush), then Content-length
>>> is set, otherwise chunk encoding is used.
>>
>> ...not just "should" but MUST! And that underlying "engine" for a JAX-RS
>> filter (hence the layer that the filter relies on to manage this) is the
>> JAX-RS implementation (independend of whether it does it on its own or
>> instead relies on a deeper layer to do it), as that "engine" is defined
>> solely by means of an API -- JAX RS 2.0! (I think we all agree that a filter
>> has to work in Java SE environments, too, as in the opposite case we do not
>> need to define JAX-RS filter API at all: If we enforce Servlets, we can just
>> use Servlet Filters!).
>>
>>> Ok, sorry for confusion. What I meant to say is that if you remove
>>> manually set Content-Lenght header as well as return -1 from
>>> MBW.getSize(), the IO container will take care of it. Since the
>>> solution is IO container dependent it would be hard to enforce a single
>>> standard behavior in JAX-RS IMO.
>>
>> This does not work as the Content-Length header is not manually set, but is
>> set by Jersey. See below.
>>
>>>> getSize() should also be deprecated and ignored. With the ability to
>>> wrap streams to do things like GZip encoding, getSize() is often wrong
>>> and should never be relied upon.
>>
>> Exactly!
>>
>> BTW, agreed, even for a simple log recorder *both*, getEntityStream *and*
>> setEntityStream are needed, otherwise the recorded content will never be
>> transferred. You are right; I missed this side effect.
>>
>> But, frankly spoken, I still think (basing on what I experienced in the past
>> with Servlet Filters I came across, and basing on the filters I contributed
>> to) many of the future JAX-RS filters will be representational
>> *transformers* (hence: change Content-Length) in some way (not Java entity
>> modifiers or MBR/MBW caches, which surely are good for *different*
>> purposes), hence will enforce a "correctly working" underlying *automatic*
>> processing of Content-Length / Chunking in the JAX-RS provider (which may
>> itself offload this burden to the Servlet Container or implement it directly
>> in a Java SE case, in its sole discretion), adjusting the Content-Length
>> information according to the transformation *result* (not *source*) in its
>> representational form (hence: modify JSON or XML without knowledge or care
>> of neither the used MBR / MBW, nor Java Entity Class, nor JAX-RS Resource).
>>
>> In the case of ISVs -like me- the problem is getting rather worse: The
>> filters we provide to the open market not only are purely representational
>> transformers, but are provided in binary form to anybody that has the need
>> to enable the particular functionality (hence "feature") of that filter to
>> any representational form produced by any completely unknown combination of
>> MBR / MBW, Java Entity Class, JAX-RS Resource *and JAX-RS implementation*.
>> Due to that, any proposed solution to the mentioned problem will "not exist
>> by definition" (i. e. are to be ignored by the designers), as that solution
>> is *Jersey* and / or *Servlet* specific! Neither does an ISV know whether or
>> not the filter will be used in a Servlet container or on a Java SE
>> environment, nor would it be wise to rely on one particular JAX-RS product.
>> Hence, ISVs can and will *only* rely on solutions guaranteed to work by pure
>> JAX-RS 2.0 API. So a Jersey-only solution is "not existing by definition" in
>> the context of a *spec* discussion.
>>
>> Moreover I think you maybe missed the fact the a filter *cannot* remove the
>> false Content-Length header, as at time of filtering that header is actually
>> *not set* --- because it is *not* the JAX-RS Resource nor an earlier filter
>> that has set that header, but it is the JAX-RS 2.0 implementation (here:
>> Jersey 2.0 M08-1) itself which had set that header *automatically* basing on
>> the *original* representation (maybe it is a MBW inside of Jersey, I don't
>> know, it just is not there at .filter() time, but it is there in the
>> representation received by the client)! THAT DOES SIMPLY NOT WORK AND LEADS
>> TO SEVERE ERROR IN PRODUCTION USE!
>>
>> As a result, there is *no* solution to the problem I have so far, which
>> unfortunately *enforces* a vote for "NO GO!" for the current draft of the
>> spec!
>>
>> Potentially (and even worse: such easily) opening the doors to
>> *unexpectedly* induce such severe problems is a definitive no-go situation
>> for the complete specification, unfortunately! And the sole existing
>> workaround (using a filter to inspect request headers to enable / disable
>> the transformer, store them as a context property, check this in an entity
>> listener and then apply the transformation *there* as this runs *before* the
>> JAX-RS implementation auto-adds the Content-Length header), is such overly
>> complicated and error-prone that it simply looks as a design fault of the
>> specification to the average reader (as one sees the getEntityStream /
>> setEntityStream combination and will think that he can simply inline a
>> FilterStream, which, again, will result in the wrong Content-Length). Also
>> it will induce the next problem: What if that transformer must be the *last*
>> step in the filter chain (to ensure that another filter does not ruin the
>> transformed result again) - which currently simply is impossible as by
>> definition entity listeners MUST run *before* the very first filter will
>> run? As you see, all currently proposed "solutions" are just NOT COMPLETE OR
>> WORKING STABLE in any way!
>>
>> Maybe you see a working solution with the current draft. I do not. The only
>> solution I see is that the spec has to be changed.
>>
>> My solution proposal is: The spec must mandate that if a compliant JAX-RS
>> 2.0 implementation is setting the Content-Length header, then it MUST
>> calcuate its value from the *transformed* representation, but not from the
>> *original* representation provided by the MBW. Moreover, I would even
>> mandate that a compliant implementation MUST OVERWRITE the Content-Length
>> (to the correct value, certainly) always, as neither the JAX-RS resource,
>> nor a MBW, nor another filter, will know whether the deployer might add a
>> filter that changes the length of the representation! At last, JAX-RS must
>> not allow that a JAX-RS 2.0 implementation is providing a false value to
>> Content-Length just like Jersey does it at the moment (better to have NO
>> Content-Length set by Jersey than to have it set a wrong one)!
>>
>> This is a very severe issue, and we MUST have a solution for that before
>> final release. I see absolutely no chance for a "GO!" to a draft that
>> provides the possibility to modify the representation using an inlined
>> FilterStream while the JAX-RS implementation is allowed to add a *false*
>> Content-Length header at the same time.
>>
>> Regards
>> Markus
>>
>>
>