On 26/05/17 18:15, Shing Wai Chan wrote:

Right now, in HttpServletRequest, we have the following:
public Map<String, String> getTrailerFields();
public boolean isTrailerFieldsReady();

And in HttpServletResponse, we have
public void setTrailerFields(Supplier<Map<String, String>>);

There are two possible method name for getter:
a) public Supplier<Map<String, String>> getTrailerFieldsSupplier();
The return type "Supplier" is explicited in the method name.
This is asymmetrical to setter.

b) public Supplier<Map<String, String>> getTrailerFields();
This is symmetrical to the setter.
But HttpServletRequest has the same method name with a different
return type.

I think (b) is better in this case.

+1

Mark

Any other comment?

Thanks.
Shing Wai Chan

On May 26, 2017, at 12:00 AM, Greg Wilkins <gregw@...
<mailto:gregw@...>> wrote:


On 25 May 2017 at 22:59, Shing Wai Chan <shing.wai.chan@...
<mailto:shing.wai.chan@...>> wrote:

If we want to support the supplier composition use case, we need
to add a getter
API in HttpServletRequest and HttpServletRequestWrapper as follows:

default public Supplier<Map<String, String>
getTrailerFieldsSupplier() {
return null;
}

Similarly, we need corresponding update in HttpServletRequestWrapper.

Do we see a need for supporting this use case?
Note that the Public Review Ballot has already started.
We need to minimize adding new API unless it is really necessary.
Please let me know your comment.



I think it is a good thing to add:

* Setters without getters are seldom justified
* While trailer usage is currently few and far between, support in
HTTP/2 and the servlet API will result in an increased usage and
thus we can expect competing concerns.
* If we have agreed on the name of the setter, then naming the
getter should be easy!

cheers



--
Greg Wilkins <gregw@... <mailto:gregw@...>> CTO
http://webtide.com <http://webtide.com/>

On 25/05/17 17:14, Martin Mulholland wrote:

servlet-spec@javaee.groups.io wrote on 05/25/2017 09:39:34 AM:

<snip/>

I don't agree, the point of trailer fields is that they are set after
the response is committed, because the values are not known
when the response is committed. If you have to set before commit you can
use normal headers.

I'm not going to repeat the long discussions we've already had on this
apart from to re-iterate that the reason a Supplier is used is that it
is expected that the requirement to use trailers is known before the
commit (and we need to signal to the container that chunking must be
used) but the values of the trailers are not.

We would need to know the headers being sent before commit if we planned
to send the trailer header. However we agreed both not to do that and also
to not limit trailers to headers specified in the trailer header. This means
from a practical standpoint we don't need to know anything about which
trailer
headers will be sent until after commit. OK, you can do that in the
current api
by calling setTrailers before commit and them modifying the supplier
after commit
but it seems unnecessary to me to reject a setTrailer calls if it is
made after commit
even if the response is chunked. Why introduce artificial restrictions?

I agree. The current Tomcat implementation only throws the ISE if the
response is committed AND chunked encoding isn't in use.

On a different tangent I think one use case would be for a filter to
add a trailer header before calling doChain() and then setting a value for
the trailer on return from doChain(). The filter could call
setTrailers() before
calling doChain() but its supplier may have been replaced by the time
doChain()
returns. Do we support multiple suppliers?

We don't. And I don't see a way for an application do anything like that
with the current API.

Mark

On 25/05/2017 14:32, Martin Mulholland wrote:

Also it is not clear to me that we would ever throw the exception
because the response is not in chunked mode. This is
because when the app calls setTrailers()
(a) we may not know if the response will be chunked or not.

If the response has been committed then we will know. If the response
hasn't been committed then we can control chunking.

(b) we can, in response to the call, enforce the response to be chunked.

If the response has been committed and chunked encoding is not being
used we cannot change this.

<snip/>

I don't agree, the point of trailer fields is that they are set after
the response is committed, because the values are not known
when the response is committed. If you have to set before commit you can
use normal headers.

I'm not going to repeat the long discussions we've already had on this
apart from to re-iterate that the reason a Supplier is used is that it
is expected that the requirement to use trailers is known before the
commit (and we need to signal to the container that chunking must be
used) but the values of the trailers are not.

Mark

Illegal state exception should be thrown if trailers are not supported
and nothing to do with commit.


Thank you,
Martin Mulholland.
WebSphere Application Server Web Tier Architect
email: mmulholl@...

IBM RTP, PO BOX 12195, 503/C227,
3039 Cornwallis Rd, RTP, NC 27709-2195
t/l 444-4319, external (919)-254-4319


servlet-spec@javaee.groups.io wrote on 05/24/2017 03:23:27 PM:

From: "Shing Wai Chan" <shing.wai.chan@...>
To: servlet-spec@javaee.groups.io
Date: 05/24/2017 03:23 PM
Subject: Re: [servlet-spec] setTrailerFields() throws ISE?
Sent by: servlet-spec@javaee.groups.io

In this case, I would add the following to the javadoc of

setTrailerFields:

+ * @throws IlleglStateException if it is invoked after the

response has

+ * has been committed,
+ * or the trailer is not supported in the request, for

instance,

+ * the underlying protocol is HTTP 1.0, or the response

is not

+ * in chunked encoding in HTTP 1.1.
+ *

Thanks.
Shing Wai Chan

On May 24, 2017, at 9:58 AM, Mark Thomas <markt@...> wrote:

Hi all,

Following up on another detail as I work on Tomcat's implementation.

My recollection is that if the application calls setTrailerFields() in a
situation where it will not take effect (e.g. after the response has
been committed, with a protocol that does not support it) then an
IllegalStateException will be thrown.

I don't see the ISE in the Javadoc. Am I missing something?

Thanks,

Mark

In this case, I would add the following to the javadoc of setTrailerFields:

+ * @throws IlleglStateException if it is invoked after the response has
+ * has been committed,
+ * or the trailer is not supported in the request, for instance,
+ * the underlying protocol is HTTP 1.0, or the response is not
+ * in chunked encoding in HTTP 1.1.
+ *

Thanks.
Shing Wai Chan

Hi all,

Following up on another detail as I work on Tomcat's implementation.

My recollection is that if the application calls setTrailerFields() in a
situation where it will not take effect (e.g. after the response has
been committed, with a protocol that does not support it) then an
IllegalStateException will be thrown.

I don't see the ISE in the Javadoc. Am I missing something?

Thanks,

Mark