jsr369-experts@servlet-spec.java.net

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

From: Martin Mulholland <mmulholl_at_us.ibm.com>
Date: Fri, 5 May 2017 10:26:04 -0400

Mark Thomas <markt_at_apache.org> wrote on 05/05/2017 04:46:59 AM:

> From: Mark Thomas <markt_at_apache.org>
> To: jsr369-experts_at_servlet-spec.java.net
> Date: 05/05/2017 04:48 AM
> Subject: [servlet-spec users] [jsr369-experts] Re: Trailer header
> implementation
>
> On 05/05/2017 01:54, Shing Wai Chan wrote:
>
> Thanks for pulling all of these threads together and keeping the
> discussion on track.

+1

>
> > There are three main questions on trailer.
> > Let me try to summarize it from my point of view.
> >
> > Section 1: is trailer supported?
> > RFC 7230, Section 4.3 TE
> > The "TE" header field in a request indicates what transfer codings,
> > besides chunked, the client is willing to accept in response, and
> > whether or not the client is willing to accept trailer fields in a
> > chunked transfer coding.
> >
> > The presence of the keyword "trailers" indicates that the client is
> > willing to accept trailer fields in a chunked transfer coding, as
> > defined in Section 4.1.2, on behalf of itself and any downstream
> > clients.
> >
> > MT has proposed an API to query whether trailer is supported.
> > And we will need to add a API in HttpServletRequest. The following are
> > options:
> > a) public boolean areTrailerFieldSupported(); // from MT
> > b) public false isTrailerFieldSupportSpecified(); // from MM
> > c) public boolean isTrailerSupported();
> > d) no new API users can just query TE header from the request
> >
> > In (a)-(c), we will add an default method returning false.
>
> It looks like I didn't make my point very well. Let me try again.
>
> This is not about whether the TE header from the client contains
> 'Trailers'. The application can check that for themselves and I don't
> see a need to add a convenience method to do so.
>
> Note (see RFC 7230, section 4.1.2, final paragraph) the application may
> opt to still send trailer fields even if the TE field from the client
> does not contain 'Trailers'. The 'necessary' in that paragraph is
> entirely subjective and something only the application can determine.
>
> My motivation for suggesting this method comes from a different
perspective.
>
> In the following circumstances, if the application tries to set trailer
> fields on the response, they will not be sent because it is impossible
> to do so:
> - the response has been committed and chunked encoding is not being used
> - the underlying protocol (HTTP 0.9, HTTP 1.0, some proxy protocols)
> does not support trailer fields
>
> It has been proposed that an IllegalStateException is thrown if the
> application tries to set trailer fields in any of the above cases.
>
> (If we do set the Trailer header on the response then we may also want
> to throw an ISE if the app tries to add a new trailer field after that
> header has been committed)
>
> The issue I see is that an application should not have to try setting a
> trailer field and catch an exception to see if the response is in a
> valid state to set trailer fields. There should be a method provided on
> the response to enable the application to determine if trailer headers
> may be set at that point.
>
> The name is of less concern to me than than the semantics. If we agree
> this method is required, the 'right' name will depend on the API we
> select in point 2 below.
>

Agree such a method would be useful. a) looks good except "field" should
be plural so:
public boolean areTrailerFieldsSupported();

My method was a convenience method for getting info from the TE header. I
am
also fine with just making this a app responsibility in which cas we
should
mention this header in the java doc.

>
> > Section 2: set trailer
> > RFC 7230, Section 4.4 Trailer
> > 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.
> >
> > MT, GW and MM has a discussion on this.
> > The main issues are
> > 1) generation of Http header with name "Trailer"
> > 2) what happens when supplier generate headers with names other than
> > those in (1)
> > 3) when can we call #setTrailerFields
> >
> > The following are options (adapted from MT with additional comments):
> > Aa) Keep the existing API:
> > void setTrailerFields(Supplier<Map<String, String>>)
> > For (1), it is developer's responsibility to generate the http
header
> > "Trailer".
> > For (2), servlet container can
> > a) only send headers specified in (1), ignore those headers
not
> > specified in (1).
> > For (3), the API can be called before response commit
> >
> > Ab) Keep the existing API:
> > void setTrailerFields(Supplier<Map<String, String>>)
> > For (1), it is developer's responsibility to generate the http
header
> > "Trailer".
> > For (2), servlet container can
> > b) send all the headers in supplier
> > For (3), the API can be called before response commit
> >
> > Ba) New API for each trailer field:
> > void setTrailer(String name, Supplier<String>)
> > For (1), it is the container's responsibility to write the http
> > header "Trailer".
> > Servlet container will:
> > a) ignore the http header "Trailer" set by developer.
> > For (2), it is not an issue.
> > For (3), #setTrailerFields needs to be called before the any body
> > message committed.
> >
> > Bb) New API for each trailer field:
> > void setTrailer(String name, Supplier<String>)
> > For (1), it is the container's responsibility to write the http
> > header "Trailer".
> > Servlet container will:
> > b) clear the http header "Trailer" set by developer.
> > For (2), it is not an issue.
> > For (3), #setTrailerFields needs to be called before the any body
> > message committed.
> >
> > C) New API for the whole trailer:
> > void setTrailer(Function<String, String>)
> > For (1), it is the developer's responsibility to generate the http
> > header "Trailer".
> > For (2), this will not happen for this API as container read
trailer
> > field
> > names from "Trailer" http header.
> > For (3), the API can be called before response commit
> >
> > With the above setting, I believe Mark's preferences are
> > (Ba), (Bb), (Ab), (Aa), (C)
>
> Correct.

I agree that it should be the containers responsibility to set the trailer

header. Given this we need to be ensure that the api is good so that:

1. It can be called before the response is committed so that we can set
   and send the trailer field.
2. It can be called after the response is committed so that the app can
   provide the trailer field values.

Section 4.1.2 of the RFC makes it clear that the trailer is for headers
for
which the values are not known when the response is committed:

<quote>
    A trailer allows the sender to include additional fields at the end
   of a chunked message in order to supply metadata that might be
   dynamically generated while the message body is sent, such as a
   message integrity check, digital signature, or post-processing
   status.
</quote>

If we want to use one api that the application calls twice for different
reasons I will accept that but I don't like it. my preference is for a
different api for each purpose.

>
> >
> > In B's, we need to restrict when we can call the API.
> > Is it simplier to hava A's and C?
> > Any other opinion?
> >
> >
> > Section 3: get trailer
> > MM> For an inbound trailer Section 4.1.2 of RFC2370 says:
> > MM> <quote>
> > MM> When a chunked message containing a non-empty trailer is
received,
> > MM> the recipient MAY process the fields (aside from those
forbidden
> > MM> above) as if they were appended to the message's header
section.
> > MM> </quote>
> > MM> So in addition to getTrailers() we should make the trailer headers
> > available
> > MM> through the getHeader() api's and automatically add them when they
> > become
> > MM> available (all inbound data is read).
> > MM> But this leads me to a concern that the RFC is a specification of
how
> > MM> the transport layer should behave and it s not clear to me how
much
> > of this transport
> > MM> function should be exposed in the Servlet Specification. In the
> > Servlet Spec we
> > MM> should not be making assumptions about how the transport layer
works
> > in which case
> > MM> we should not have a getTrailers() API but instead we just expose
> > the headers using
> > MM> existing api's once all the data is read. That way we work whether
> > or not the transport
> > MM> layer exposes the trailer headers or not.
> >
> > On April 6, 2017, I proposed (in (a) StandardHeader option [1])
> > to use the standard header APIs as one of the option to retrieve
trailer
> > fields.
> > This option was rejected in [2] and [3] as follows:
> >
> > GW> with regards to request trailers, the key decision is when they
are
> > GW> available, specially with async. Does the application have to read
> > content
> > GW> to -1 or onAllDataRead before the trailers are available? I think
yes.
> > GW> Then it does not make much sense to make them available via the
normal
> > GW> header API and I think they need their own API.
> >
> > SD> I don't think this is a good approach. The trailers will not be
> > SD> available immediately, so this will basically result in the
headers
> > SD> changing after the request has been read. I think it could be very
> > SD> confusing to the developer.
> >
> > Since the trailer fields are processed "as if they were appended to
the
> > message's
> > header section."
> > Should trailer fields be available to get header API's, too?
>
> I think not. I agree that having the return values for the standard
> header API change depending on whether the request is fully read or not
> is very likely to cause confusion. So +1 to GW and SD comments above.
>
> Mark
>

I disagree, if the app reads the trailer header it knows which headers to
re-read after the inbound data is fully read. Adding getTrailers()
is imposing a level of support from the transport layer which to me is
not appropriate.

Martin.