> 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 <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@webtide.com <mailto:gregw@webtide.com>> CTO http://webtide.com <http://webtide.com/>