jsr369-experts@servlet-spec.java.net

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

From: Martin Mulholland <mmulholl_at_us.ibm.com>
Date: Thu, 4 May 2017 10:48:55 -0400

A couple of other things:

We should at least specify when the trailer headers are sent. I assume
this would be when
the underlying printWriter or ServletOutputStream is closed. Should we
also provide a
convenience mathod on HttpServletResponse such as sendTrailers()? This
would have
the effect of also closing the writer or outputStream.

In the description of ServletResponse.reset() which should mention it also
clears any trailer
headers which are set.

Thanks,
Martin.

Martin Mulholland/Raleigh/IBM_at_IBMUS wrote on 05/04/2017 10:17:04 AM:

> From: Martin Mulholland/Raleigh/IBM_at_IBMUS
> To: jsr369-experts_at_servlet-spec.java.net
> Date: 05/04/2017 10:18 AM
> Subject: [servlet-spec users] [jsr369-experts] Re: Re: Re: Re: Re:
> Trailer header implementation
>
> Mark Thomas <markt_at_apache.org> wrote on 05/04/2017 06:53:19 AM:
>
> > From: Mark Thomas <markt_at_apache.org>
> > To: jsr369-experts_at_servlet-spec.java.net
> > Date: 05/04/2017 06:54 AM
> > Subject: [servlet-spec users] [jsr369-experts] Re: Re: Re: Re:
> > Trailer header implementation
> >
> > On 04/05/17 06:55, Greg Wilkins wrote:
> > >
> > > If we provide an API like setTrailer(String name, Supplier<String>),
> > > then the container can generate a trailers header that is sufficient
for
> > > any trailers that are eventually supplied, as there is no other
> > > mechanism available to set trailers. If the app has also set a
> > > Trailers header, then it can either be ignored or replaced. I
prefer
> > > ignored as there are many bad combinations of headers that an app
can
> > > already generate and we don't enforce RFC compliance for other
headers.
> > >
> > > If we provide and API like setTrailer(Supplier<Map<String,String>),
then
> > > we could look at any Trailers header set and only query the Map for
> > > listed trailers. In that case we would probably be better off with
> > > setTrailer(Function<String,String>) and avoid the map.
> > >
> > > The requirement regarding the TE header is very subjective and hard
for
> > > us to enforce. Is a checksum "necessary for the user agent to
receive"
> > > ? Depends on the user-agent.
> > >
> > > So options are:
> > > a. setTrailer(Supplier<Map<String,String>) iterate over app set
Trailer
> > > headers
> > > b. setTrailer(Function<String,String>), iterate over app set
> Trailers header
> > > c. setTrailer(String name, Supplier<String>), ignore app set
> Trailers header
> > > d. setTrailer(String name, Supplier<String>), clear app set
> Trailers header
> > > e. setTrailer(Supplier<Map<String,String>) ignore app set Trailers
> > > header (status quo)
> > >
> > > My preferences are b,c,d,e,a
> >
> > I don't see a benefit in requiring the app to set a valid Trailers
> > header and then use that header to filter the data the same app passes
> > to setTrailer(TBD). It seems like we are asking the user to specify
the
> > valid headers twice.
> >
> > My preferences are:
> >
> > c,d,e,a,b
> >
> > Mark
> >
>
> The app may not know the value it needs to set for a trailer header
> until after
> the response is committed so I think we need a two step process:
>
> 1. before commit : set the trailers headers which will be set.
> 2. after commit : set the values for the previously specified
headertrailers.
>
> Overloading one api for both functions is a not great so I would suggest

> setTrailerHeaders() - must be called before commit, subsequent call
> overrides a previous call
> setTrailerHeaderValues() - can be called after commit, subsequent
> call overrides a previous call.
>
> Will leave Mark and Greg, who understand Java 8 much better than me,
> to suggest how the
> parameters should be specified. :-).
>
> Thanks,
> Martin Mulholland.
>
>
> >
> > >
> > >
> > > cheers
> > >
> > >
> > >
> > >
> > > On 4 May 2017 at 02:01, Shing Wai Chan <shing.wai.chan_at_oracle.com
> > > <mailto:shing.wai.chan_at_oracle.com>> wrote:
> > >
> > > Let us summarize what we discuss on the trailer implementation.
> > >
> > > MT> The first one is triggered by Section 4.4 for RFC 7230:
> > >
> > > MT> <quote
> > > MT> When a message includes a message body encoded with the
chunked
> > > MT> transfer coding and the sender desires to send metadata
> in the form
> > > MT> of trailer fields at the end of the message, the sender
SHOULD
> > > MT> generate a Trailer header field before the message body
> to indicate
> > > MT> which fields will be present in the trailers. This allows
the
> > > MT> recipient to prepare for receipt of that metadata
> before it starts
> > > MT> processing the body, which is useful if the message is
being
> > > streamed
> > > MT> and the recipient wishes to confirm an integrity check
> on the fly.
> > > MT> </quote>
> > >
> > > MT> 1. How is the container meant to construct the Trailer
header
> > > field? The
> > > MT> point of using a supplier was so the header names and
> values did not
> > > MT> have to be known when the response was committed. However,
> > > section 4.4
> > > MT> requires that the names are known.
> > >
> > > GW> Firstly, it's only a SHOULD, so I'm guessing it wont be well
> > > respected.
> > > GW> For example are we going to reject trailers not listed in a
> > > prior Trailers header?
> > > GW> But then I guess at the time the supplier is set, the app
knows
> > > it is adding
> > > GW> a supplier with certain capabilities, so it should know what
> > > trailers may be set.
> > >
> > > MM> I think we should enforce the "SHOULD" at the servlet level
so
> > > consider it
> > > MM> a must.
> > > MM> Also what if the application sets the trailer header using
the
> > > existing
> > > MM> setHeader() api and then subsequently uses the new api to
set
> > > the promised
> > > MM> trailer headers. Should we detect such a setting of the
trailer
> > > header and
> > > MM> then ignore headers set using setTrailers if they were not
> > > indicated by
> > > MM> the trailer header.
> > >
> > > MT> Do we ignore the 'SHOULD' or do we change the API? I'm
leaning
> > > MT> towards changing the API on the basis that if we are going
to add
> > > MT> trailer support we should do it as correctly as possible.
> > >
> > > GW> +0 as it feels a bit over complicated. i guess we could
do
> > > GW> setTrailer(String name, Supplier<String>)
> > > GW> To set a supplier for each field and then we'd know how to
set the
> > > GW> Trailers header.
> > > GW> But to be symmetric, would we'd need to police the Trailers
> > > header on incoming
> > > GW> requests? symmetry also pushes us towards
> > > GW> String getTrailer(String name)
> > > GW> rather than the map.
> > >
> > > MM> As I said before I think we enforce the SHOULD. However
> I don't see
> > > MM> a need to be symmetric, we have no control over what clients
> > > send in
> > > MM> and we do the best we can with what we get. If the app
> really wants
> > > MM> the header field it can ignore any trailer headers but we
should
> > > still
> > > MM> be willing to provide them.
> > >
> > > +1 for no need for symmetric for #getTrailerFields.
> > >
> > > In the previous API, Supplier<Map<String, String>>, it is the
> > > developer responsbility
> > > to generate a "trailer" header.
> > >
> > > In the above, setTrailerField(String name, Supplier<String>
> > > supplier), a "trailer"
> > > header will be generated by the servlet container.
> > >
> > > If we want to enforce the SHOULD at the servlet level, then we
would
> > > like to consider
> > > the issue mentioned by Martin above:
> > > req.setHeader("trailers", "foo");
> > > req.setHeader("foo", "v1")
> > > // set a supplier generated trailer header "foo" with value
"v2"
> > > The following are possible options:
> > > a) a header "foo: v1" is sent and a trailer header "foo: v2" is
sent
> > > b) only a trailer header "foo: v1" is sent, ignore the
> trailer supplier
> > > c) only a trailer header "foo: v2" is sent, ignore the header
> > > d) throw IllegalStateException
> > >
> > > MM> I should also have mentioned this from the RFC section
4.1.2:
> > > MM> Unless the request includes a TE header field
indicating"trailers"
> > > MM> is acceptable, as described in Section 4.3, a server
SHOULD NOT
> > > MM> generate trailer fields that it believes are necessary for
the
> > > user
> > > MM> agent to receive. Without a TE containing "trailers",the
server
> > > MM> ought to assume that the trailer fields might be silently
> > > discarded
> > > MM> along the path to the user agent. This requirement allows
> > > MM> intermediaries to forward a de-chunked message to an
HTTP/1.0
> > > MM> recipient without buffering the entire response.
> > > MM> Should we be looking for this header and throw an
> > > IllegalStateException
> > > MM> from setTrailers() if the TE header is not present or does
not
> > > indicate that
> > > MM> trailers are acceptable?
> > >
> > > The possible options:
> > > i) throw IllegalStateException
> > > ii) ignore
> > >
> > > Let us discuss more on (1).
> > >
> > > ---
> > > MT> 2. What happens if setTrailerFields() is called after the
> > > response is
> > > MT> committed?
> > >
> > > GW> Good question. We can go either: If when it is set, the
> > > response is committed
> > > GW> with a transport mode that cannot support trailers then an
> > > illegalStateException
> > > GW> can be thrown; alternately we can just never call the
supplier
> > > to get the trailers.
> > > GW> Jetty currently does the later, but I can see reasons for
the
> > > former.
> > >
> > > MM> It is just the trailer header which must be sent before the
> > > response is
> > > MM> committed so in theory we could accept this after commit if
the
> > > MM> trailer header was provided and sent before commit. This
> > > complicates things
> > > MM> but does not seem a unreasonable expectation from an app.
> > >
> > > MT> Throw an IllegalStateException
> > > MT> But. It also depends on 1). If we don't have to set the
Trailers
> > > MT> header it may still be possible to send the trailer fields
> > >
> > > GW> +1
> > >
> > > +1
> > >
> > > ---
> > > MT> 3. What happens if setTrailerFields() is called and trailer
> > > fields are
> > > MT> not supported (HTTP 0.9, HTTP 1.0, some proxy protocols,
etc.)?
> > >
> > > GW> Same as above.
> > >
> > > MM> Difficulty is if the trailer fields include some valid
headers
> > > and some
> > > MM> invalid headers so I vote for just ignoring the invalid
ones.
> > >
> > > MT> Throw an IllegalStateException
> > >
> > > GW> +1
> > >
> > > +1
> > >
> > > ---
> > > MT> 4. What happens if setTrailerFields() is called multiple
times?
> > >
> > > GW> In Jetty, each call replaces the previous. So you can null
out
> > > the supplier
> > > GW> or give a new one.
> > > GW> However, I can see that there might be competing concerns
that
> > > want to add
> > > GW> multiple trailers. Perhaps we need addTrailerFields and
support
> > > multiple
> > > GW> suppliers (a bit over the top), or perhaps just ISE if it
has
> > > already been set?
> > >
> > > MM> I would go with replacing the previous, also if we replace a
> > > previous this
> > > MM> could definitely be done after commit, just to replace
previous
> > > values.
> > >
> > > MT> Use the most recent Supplier
> > >
> > > GW> +1
> > >
> > > +1
> > >
> > > ---
> > > MT> 5. What happens if setTrailerFields() is called with a null
> > > Supplier?
> > >
> > > GW> Jetty currently nulls out the supplier.
> > >
> > > MM> +1
> > >
> > > MT> Allow it and protect against any possible NPE in the
container
> > >
> > > GW> +1
> > >
> > > +1
> > >
> > > ---
> > > MT> Do we need a boolean areTrailerFieldsSupported() method?
> > >
> > > Not at this point unless (1) needs this.
> > >
> > > Thanks.
> > > Shing Wai Chan
> > >
> > >
> > >
> > >> On May 3, 2017, at 2:17 PM, Martin Mulholland
<mmulholl_at_us.ibm.com
> > >> <mailto:mmulholl_at_us.ibm.com>> 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?
> > >>
> > >>
> > >> Thank you,
> > >> Martin Mulholland.
> > >> WebSphere Application Server Web Tier Architect
> > >> email: mmulholl_at_us.ibm.com <mailto: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
> > >> <mailto: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 <mailto: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 <mailto:gregw_at_webtide.com>>
> > >> wrote on 05/03/2017 08:54:07 AM:
> > >> >
> > >> > > From: Greg Wilkins <gregw_at_webtide.com <
mailto:gregw_at_webtide.com>>
> > >> > > To: jsr369-experts_at_servlet-spec.java.net
> > >> <mailto: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
> > >> <mailto: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.
Thisallows 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
Idon'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_at_webtide.com <mailto:gregw_at_webtide.com>>
> > >> CTO http://webtide.com<http://webtide.com/>
> > >
> > >
> > >
> > >
> > > --
> > > Greg Wilkins <gregw_at_webtide.com <mailto:gregw_at_webtide.com>> CTO
> > > http://webtide.com
> >