On 05/05/2017 01:54, Shing Wai Chan wrote:
Thanks for pulling all of these threads together and keeping the
discussion on track.
> 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.
> 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.
>
> 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