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
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.
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