users@servlet-spec.java.net

[servlet-spec users] [jsr369-experts] Re: Re: Re: Trailer header implementation

From: Mark Thomas <markt_at_apache.org>
Date: Thu, 4 May 2017 11:28:47 +0100

On 03/05/17 22:17, Martin Mulholland wrote:
> I should also have mentioned this from the RFC section 4.1.2:
>
> Unless the request includes a TE header field indicating "trailers"
> is acceptable, as described in Section 4.3, a server SHOULD NOT
> generate trailer fields that it believes are necessary for the user
> agent to receive. Without a TE containing "trailers", the server
> ought to assume that the trailer fields might be silently discarded
> along the path to the user agent. This requirement allows
> intermediaries to forward a de-chunked message to an HTTP/1.0
> recipient without buffering the entire response.
>
> Should we be looking for this header and throw an IllegalStateException
> from setTrailers() if the TE header is not present or does not indicate
> that
> trailers are acceptable?

I think this has to be an application decision. Only the application can
define the "trailer fields that it believes are necessary".

Therefore, I do not think the container should look for this header and
throw an ISE if it is not present.

Mark

>
>
> Thank you,
> Martin Mulholland.
> WebSphere Application Server Web Tier Architect
> email: mmulholl_at_us.ibm.com
>
> IBM RTP, PO BOX 12195, 503/C227,
> 3039 Cornwallis Rd, RTP, NC 27709-2195
> t/l 444-4319, external (919)-254-4319
>
>
> Martin Mulholland/Raleigh/IBM_at_IBMUS wrote on 05/03/2017 05:04:04 PM:
>
>> From: Martin Mulholland/Raleigh/IBM_at_IBMUS
>> To: jsr369-experts_at_servlet-spec.java.net
>> Date: 05/03/2017 05:05 PM
>> Subject: [servlet-spec users] [jsr369-experts] Re: Re: Trailer
>> header implementation
>>
>>
>> Thank you,
>> Martin Mulholland.
>> WebSphere Application Server Web Tier Architect
>> email: mmulholl_at_us.ibm.com
>>
>> IBM RTP, PO BOX 12195, 503/C227,
>> 3039 Cornwallis Rd, RTP, NC 27709-2195
>> t/l 444-4319, external (919)-254-4319
>>
>>
>> Greg Wilkins <gregw_at_webtide.com> wrote on 05/03/2017 08:54:07 AM:
>>
>> > From: Greg Wilkins <gregw_at_webtide.com>
>> > To: jsr369-experts_at_servlet-spec.java.net
>> > Date: 05/03/2017 08:54 AM
>> > Subject: [servlet-spec users] [jsr369-experts] Re: Trailer header
>> > implementation
>> >
>> >> On 3 May 2017 at 13:02, Mark Thomas <markt_at_apache.org> wrote:
>> >> Hi,
>> >>
>> >> I've been working on trailer header implementation. No particular
> issues
>> >> with requests but the response implementation is identifying a lot of
>> >> questions.
>> >>
>> >> The first one is triggered by Section 4.4 for RFC 7230:
>> >>
>> >> <quote
>> >> When a message includes a message body encoded with the chunked
>> >> transfer coding and the sender desires to send metadata in the form
>> >> of trailer fields at the end of the message, the sender SHOULD
>> >> generate a Trailer header field before the message body to indicate
>> >> which fields will be present in the trailers. This allows the
>> >> recipient to prepare for receipt of that metadata before it starts
>> >> processing the body, which is useful if the message is being
> streamed
>> >> and the recipient wishes to confirm an integrity check on the fly.
>> >> </quote>
>> >>
>> >> 1. How is the container meant to construct the Trailer header
> field? The
>> >> point of using a supplier was so the header names and values did not
>> >> have to be known when the response was committed. However, section 4.4
>> >> requires that the names are known.
>>
>> >
>> > Firstly, it's only a SHOULD, so I'm guessing it wont be well
>> > respected. For example are we going to reject trailers not listed
>> > in a prior Trailers header?
>> >
>> > But then I guess at the time the supplier is set, the app knows it
>> > is adding a supplier with certain capabilities, so it should know
>> > what trailers may be set.
>> >
>>
>> I think we should enforce the "SHOULD" at the servlet level so consider it
>> a must.
>>
>> Also what if the application sets the trailer header using the existing
>> setHeader() api and then subsequently uses the new api to set the promised
>> trailer headers. Should we detect such a setting of the trailer header
> and
>> then ignore headers set using setTrailers if they were not indicated by
>> the trailer header.
>>
>> >
>> >> The remaining questions are a little simpler:
>> >>
>> >> 2. What happens if setTrailerFields() is called after the response is
>> >> committed?
>> >
>> > Good question. We can go either: If when it is set, the response
>> > is committed with a transport mode that cannot support trailers then
>> > an illegalStateException can be thrown; alternately we can just
>> > never call the supplier to get the trailers.
>> >
>> > Jetty currently does the later, but I can see reasons for the former.
>> >
>>
>> It is just the trailer header which must be sent before the response is
>> committed so in theory we could accept this after commit if the
>> trailer header was providedand sent before commit. This complicates
> things
>> but does not seem a unreasonable expectation from an app.
>>
>> >
>> >> 3. What happens if setTrailerFields() is called and trailer fields are
>> >> not supported (HTTP 0.9, HTTP 1.0, some proxy protocols, etc.)?
>> >
>> > Same as above.
>>
>> Difficulty is if the trailer fields include some valid headers and some
>> invalid headers so I vote for just ignoring the invalid ones.
>>
>> >
>> >> 4. What happens if setTrailerFields() is called multiple times?
>> >
>> > In Jetty, each call replaces the previous. So you can null out the
>> > supplier or give a new one.
>> >
>> > However, I can see that there might be competing concerns that want
>> > to add multiple trailers. Perhaps we need addTrailerFields and
>> > support multiple suppliers (a bit over the top), or perhaps just ISE
>> > if it has already been set?
>> >
>>
>> I would go with replacing the previous, also if we replace a previous this
>> could definitely be done after commit, just to replace previous values.
>>
>> >
>> >> 5. What happens if setTrailerFields() is called with a null Supplier?
>>
>> > Jetty currently nulls out the supplier.
>> >
>>
>> +1
>>
>> >
>> >> My current thinking:
>> >>
>> >> 1) Do we ignore the 'SHOULD' or do we change the API? I'm leaning
>> >> towards changing the API on the basis that if we are going to add
>> >> trailer support we should do it as correctly as possible.
>> >
>> > +0 as it feels a bit over complicated. i guess we could do
>> >
>> > setTrailer(String name, Supplier<String>)
>> >
>> > To set a supplier for each field and then we'd know how to set the
>> > Trailers header.
>> >
>> > But to be symmetric, would we'd need to police the Trailers header
>> > on incoming requests? symmetry also pushes us towards
>> >
>> > String getTrailer(String name)
>> >
>> > rather than the map.
>>
>> As I said before I think we enforce the SHOULD. However I don't see
>> a need to be symmetric, we have no control over what clients send in
>> and we do the best we can with what we get. If the app really wants
>> the header field it can ignore any trailer headers but we should still
>> be willing to provide them.
>>
>> >
>> >> 2) Throw an IllegalStateException
>> >> But. It also depends on 1). If we don't have to set the Trailers
>> >> header it may still be possible to send the trailer fields
>> >
>> > +1
>> >
>> >
>> >> 3) Throw an IllegalStateException
>> >
>> > +1
>> >
>> >> 4) Use the most recent Supplier
>> >
>> > +1
>> >
>> >> 5) Allow it and protect against any possible NPE in the container
>> >
>> > +1
>> >
>> >> Of these, 1) is the most serious to address. After that, 2) & 3) since
>> >> the application has no way to tell if this is going to happen. Do we
>> >> need a boolean areTrailerFieldsSupported() method?
>> >>
>> >> Mark
>> >
>>
>> >
>> > --
>> > Greg Wilkins <gregw@webtide.com> CTO http://webtide.com
> <http://webtide.com/>