jsr339-experts@jax-rs-spec.java.net

[jsr339-experts] Re: [jax-rs-spec users] Re: 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 19:05:14 +0200

Hooray! :-)

 

From: Marek Potociar [mailto:marek.potociar_at_oracle.com]
Sent: Montag, 22. Oktober 2012 18:37
To: jsr339-experts_at_jax-rs-spec.java.net
Subject: [jsr339-experts] Re: [jax-rs-spec users] Re: Re: Re: Re: Re: Re:
Heads Up: Severe problem when rewriting responses! Is our Filter API
suitable?

 

FYI - Seems the JAX-RS 1.x TCK tests do not cover this area, so we should be
able to make the suggested change.

 

Marek

 

On Oct 22, 2012, at 5:44 PM, Marek Potociar <marek.potociar_at_oracle.com>
wrote:





I filed a new Jira issue to track this:
http://java.net/jira/browse/JAX_RS_SPEC-279

 

Please note that I first need to check if the suggested change does not
break any existing TCK tests. If it does, the change would not be that
straightforward, I'm afraid.

 

Marek

 

On Oct 22, 2012, at 4:18 PM, Jan Algermissen <jan.algermissen_at_nordsc.com>
wrote:






On Oct 22, 2012, at 2:29 PM, Marek Potociar wrote:




Hi Jan,

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