On 14 April 2017 at 18:04, Mark Thomas <markt_at_apache.org> wrote:
> On 14/04/2017 04:21, Greg Wilkins wrote:
> > I think null should indicate there was no trailer available and empty
> > map should indicate a trailer that was empty.
>
> I'm not clear how we differentiate between 'not available' and 'empty'.
>
> Do you mean:
> - chunk encoding not used -> null
- chunked encoding used but no (non-ignored) trailer parts - > empty
>
>
Chunked encoding or HTTP/2 would give an empty map if there are no
trailers, otherwise null to indicate no trailers could have been
transferred.
> How should trailer headers be treated if the contain a header that is
> not permitted as a trailer? Ignore or error (400)? Personally, I'd lean
> towards ignore but only because Tomcat currently ignores all trailer
> headers unless a header is explicitly configured as allowed. I'd be
> happy with either option.
>
>
+1 for ignore. Jetty currently ignores all trailers and will not act on any
of them. They are simply made available to the application as extra meta
data.
If we want containers to act on trailers, then we'd need clear RFC guidance
as to which ones we should look at.
cheers
> Mark
>
>
> >
> >
> > On 14 April 2017 at 10:34, Shing Wai Chan <shing.wai.chan_at_oracle.com
> > <mailto:shing.wai.chan_at_oracle.com>> wrote:
> >
> >
> >> On Apr 12, 2017, at 6:38 PM, Greg Wilkins <gregw_at_webtide.com
> >> <mailto:gregw_at_webtide.com>> wrote:
> >>
> >>
> >> On 13 April 2017 at 10:47, Shing Wai Chan
> >> <shing.wai.chan_at_oracle.com <mailto:shing.wai.chan_at_oracle.com>>
> wrote:
> >>
> >> Look like we have an agreement on B.c (SuppierMap write).
> >>
> >>
> >> \o/
> >>
> >>
> >> Now, let us discuss more on A.c (RequestTrailerMap read) and
> >> A.e (ConsumerCallback read).
> >>
> >>
> >>
> >> I too am attracted to the symmetry of the ConsumerCallback API,
> >> but I just think that it requires very careful specification about
> >> who/when the consumer is called.
> >>
> >> Consider a very simple request:
> >>
> >> GET / HTTP/1.1\r\n
> >>
> >> Transfer-Encoding: chunked\r\n
> >>
> >> \r\n
> >>
> >> 0\r\n
> >>
> >> Some-Trailer: value\r\n
> >>
> >> \r\n
> >>
> >> There is no content, so the application may never call
> >> request.getInputStream().read(), so there is no opportunity to use
> >> the caller of the read method to call the consumer. More over, it
> >> may be that the trailer is actually parsed before the servlet is
> >> even invoked.
> >>
> >> So if the trailer is already parsed before
> >> setTrailerCallback(Consumer<Map<String, String>> callback) is
> >> called and/or read() is never called after it is called, then we
> >> do not have a thread with which we could call the callback. This
> >> problem is solvable, but only by an API contract that says the
> >> application must read the input to -1 after the callback is set,
> >> perhaps we should even through a ISE if the consumer is set after
> >> -1 is read? Perhaps something like:
> >>
> >> /**
> >>
> >> * Set a Request Trailer Consumer
> >> *
> >>
> >> * This method must be called before the application reads the
> >>
> >> * request content. After setting the consumer, the
> >> application must
> >>
> >> * read the content until EOF indication.
> >>
> >> *
> >>
> >> * @param callback Consumer of request headers which will be
> >> called within
> >>
> >> * the scope of the call that reads the EOF indication of the
> >> request content.
> >> *
> >>
> >> * @exception IllegalStateException if the either of {_at_link
> >> #getReader} or
> >> * {_at_link #getInputStream} methods have already been called
> >> for this request
> >>
> >> */
> >> setTrailerCallback(Consumer<Map<String, String>> callback)
> >>
> >>
> >> Now there are still some concerns with this. Let's say a client
> >> sends the request but stalls after an incomplete header. This
> >> means that a blocking read cannot progress and must block until
> >> the complete trailer is received, so it can call the consumer
> >> within the correct scope.
> >>
> >> Similarly with async, we cannot call onAllDataRead until after the
> >> entire trailer is received. Which may lead to some funky logic
> >> around what isReady() should return if we have seen the final
> >> chunk, but not the full trailer. We have to distinguish between
> >> parsing the end of the content and the end of the framing, which
> >> we had to do internally already, but now we are making that
> >> difference visible to the application.
> >>
> >> Again, there are all solvable, but subtle and if different
> >> containers do slightly different timings of how they consume
> >> trailers, then we may have portability problems. So it may be
> >> better to go with the simpler to specify (but essentially
> equivalent):
> >>
> >> /**
> >>
> >> * Get a Request Trailer
> >> *
> >>
> >> * This method can only be called after the application reads
> all
> >>
> >> * request content.
> >>
> >> *
> >>
> >> * @returns A map of trailers or null if the request did not
> >> contain any.
> >>
> >> * @exception IllegalStateException if the neither
> >> {_at_ReadListener#onAllDataRead}
> >> * has been called nor an EOF indication has been
> >>
> >> * returned from the request {_at_link #getReader} or {_at_link
> >> #getInputStream}
> >>
> >> */
> >> Map<String,String> getTrailers()
> >>
> >
> > Should we allow null here? Or just an empty map when there is no
> > trailer?
> >
> > Shing Wai Chan
> >
> >>
> >> regards
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >> --
> >> 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
>
>
--
Greg Wilkins <gregw@webtide.com> CTO http://webtide.com