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> 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
>
> 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/>