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 header
trailers.
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. 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_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
>