users@jax-rs-spec.java.net

[jax-rs-spec users] [jsr339-experts] Re: Re: Re: Re: Re: Heads Up: Severe problem when rewriting responses! Is our Filter API suitable?

From: Markus KARG <markus_at_headcrashing.eu>
Date: Mon, 22 Oct 2012 18:53:00 +0200

+1

Good to see that you understood that the problem is neither a bug in Jersey
(but in the spec 2.0 draft), nor can be solved by the programmer as it is
Jersey's default MBW that produces the problem, which cannot be solved by an
app assembler just picking binaries.

> > When you say the overall design is broken, do you specifically mean
> > the MBW.getSize() not fitting well into the picture
>
> Yes, that is what I meant. Sorry.
>
>
> > or is there anything else?
> > Also, when you say that the problem cannot be fixed by the
> developers, do you think that (given most MBWs already return -1
> anyway) a workaround where a user would need to override a particular
> MBW with it's own (perhaps wrapper or extension) implementation that
> returns -1 from getSize() is not possible? Again, most MBWs already are
> "compatible", so we're talking about marginal use cases here...
>
> Agreed (as I said, I never use getSize() anyhow, hence never occurred
> to me). However, in Markus' case the non -1 return value seems to come
> from a Jersey build-in MBW. That is what I mean when I say that it is a
> very hard to find 'bug'.
>
> >
> > FWIW, I agree that we need to fix the getSize() issue in the spec. It
> just seems to me we need a longer discussion about this and I'm not
> sure if we can agree on a solution in time for 2.0.
>
> I am +1 on Bill's suggestion to deprecate it and require ignoring it
> from 2.0 on. This will not break existing apps as they would silently
> default to chinked transfer encoding.
>
> JAX-RS implementations could be required to issue a warning if they
> find an MBW that overwrites getSize() *and* filters or interceptors.
>
> Jan
>
> >
> > Marek
> >
> > On Oct 22, 2012, at 12:45 PM, Jan Algermissen
> <jan.algermissen_at_nordsc.com> wrote:
> >
> >>
> >> On Oct 22, 2012, at 12:43 PM, Jan Algermissen wrote:
> >>>>
> >>>
> >>> I think the overall design is now broken because the original MBW
> API does not fit with the filter concept.
> >>>
> >>> The problem must be taken care of by the runtime because a
> developer alone cannot ensure correct Content-Length settings.
> >>
> >> Sorry, should have added '..taken care of until getSize() is removed
> from the API ...'
> >>
> >> Jan
> >>
> >>
> >>>
> >>> I am not sure though the runtime can do that either.
> >>>
> >>> Jan
> >>>
> >>>>
> >>>> 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
> >>>>>>
> >>>>>>
> >>>>>
> >>>>
> >>>
> >>
> >