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()
regards
--
Greg Wilkins <gregw@webtide.com> CTO http://webtide.com