I think null should indicate there was no trailer available and empty map
should indicate a trailer that was empty.
On 14 April 2017 at 10:34, Shing Wai Chan <shing.wai.chan_at_oracle.com> wrote:
>
> On Apr 12, 2017, at 6:38 PM, Greg Wilkins <gregw_at_webtide.com> wrote:
>
>
> On 13 April 2017 at 10:47, Shing Wai Chan <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@webtide.com> CTO http://webtide.com
>
>
>
--
Greg Wilkins <gregw@webtide.com> CTO http://webtide.com