Date
1 - 14 of 14
setTrailerFields() throws ISE?
Mark Thomas
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 |
|
Shing Wai Chan
In this case, I would add the following to the javadoc of setTrailerFields:
toggle quoted message
Show quoted text
+ * @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: |
|
Martin Mulholland
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. 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 > > > > > > > > > > |
|
Martin Mulholland
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. (b) we can, in response to the call, enforce the response to be chunked. 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 From: "Martin Mulholland" <mmulholl@...> To: servlet-spec@javaee.groups.io Date: 05/25/2017 08:49 AM Subject: Re: [servlet-spec] setTrailerFields() throws ISE? Sent by: servlet-spec@javaee.groups.io 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. 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 > > > > > > > > > > |
|
Mark Thomas
On 25/05/2017 14:32, Martin Mulholland wrote:
Also it is not clear to me that we would ever throw the exceptionIf 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 afterI'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
|
|
Martin Mulholland
servlet-spec@javaee.groups.io wrote on 05/25/2017 09:39:34
AM:
> From: "Mark Thomas" <markt@...> > To: servlet-spec@javaee.groups.io > Date: 05/25/2017 09:39 AM > Subject: Re: [servlet-spec] setTrailerFields() throws ISE? > Sent by: servlet-spec@javaee.groups.io > > 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. > my bad, agree. > <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? 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? Martin. > 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 > >> > > >> > > >> > > >> > >> > >> > >> > > > > > > > > > > > > > > |
|
Mark Thomas
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 agree. The current Tomcat implementation only throws the ISE if theWe would need to know the headers being sent before commit if we plannedI don't agree, the point of trailer fields is that they are set afterI'm not going to repeat the long discussions we've already had on this response is committed AND chunked encoding isn't in use. On a different tangent I think one use case would be for a filter toWe don't. And I don't see a way for an application do anything like that with the current API. Mark |
|
Greg Wilkins
On 25 May 2017 at 18:14, Martin Mulholland <mmulholl@...> wrote: servlet-spec@javaee.groups.io wrote on 05/25/2017 09:39:34 AM: For portability. Different container make different decisions on when to use chunking, often driven by things like buffer size configuration. That an applications might work on once container because it happens to choose chunking and then not work on another than is able to determine content-length or is configured to always Connection:close etc. Basically I can't see why an app cannot decide before commit that it might send trailers, and I see no reason to allow apps to say they might send trailers at any time after commit. On a different tangent I think one use case would be for a filter to There was a brief discussion about this. I think it was decided this was something that Frameworks should do. However, it may be a good idea to add a getter for the trailers supplier so apps/frameworks can know if a supplier has already been set and do supplier composition themselves if they want. regards Martin. |
|
Shing Wai Chan
MT> I don't see the ISE in the Javadoc. Am I missing something? SW> In this case, I would add the following to the javadoc of setTrailerFields: SW> + * @throws IlleglStateException if it is invoked after the response has SW> + * has been committed, SW> + * or the trailer is not supported in the request, for instance, SW> + * the underlying protocol is HTTP 1.0, or the response is not SW> + * in chunked encoding in HTTP 1.1. I. throw ISE if trailer fields supplier is set after the response has been committed MM> I don't agree, the point of trailer fields is that they are set after MM> the response is committed, because the values are not known MM> when the response is committed. If you have to set before commit you can MM> use normal headers. MT> I'm not going to repeat the long discussions we've already had on this MT> apart from to re-iterate that the reason a Supplier is used is that it MT> is expected that the requirement to use trailers is known before the MT> commit (and we need to signal to the container that chunking must be MT> used) but the values of the trailers are not. MM> We would need to know the headers being sent before commit if we planned MM> to send the trailer header. However we agreed both not to do that and also MM> to not limit trailers to headers specified in the trailer header. This means MM> from a practical standpoint we don't need to know anything about which trailer MM> headers will be sent until after commit. OK, you can do that in the current api MM> by calling setTrailers before commit and them modifying the supplier after commit MM> but it seems unnecessary to me to reject a setTrailer calls if it is made after MM> commit even if the response is chunked. Why introduce artificial restrictions? MT> I agree. The current Tomcat implementation only throws the ISE if the MT> response is committed AND chunked encoding isn't in use. The chunked encoding is not required for trailer in http2. GW> For portability. Different container make different decisions on when GW> to use chunking, often driven by things like buffer size configuration. GW> That an applications might work on once container because it happens to GW> choose chunking and then not work on another than is able to determine GW> content-length or is configured to always Connection:close etc. GW> Basically I can't see why an app cannot decide before commit that it GW> might send trailers, and I see no reason to allow apps to say they might GW> send trailers at any time after commit. MM> Illegal state exception should be thrown if trailers are not supported MM> and nothing to do with commit. MM> Also it is not clear to me that we would ever throw the exception because MM> the response is not in chunked mode. This is MM> because when the app calls setTrailers() MM> (a) we may not know if the response will be chunked or not. MT> If the response has been committed then we will know. If the response MT> hasn't been committed then we can control chunking. MM> (b) we can, in response to the call, enforce the response to be chunked. MT> If the response has been committed and chunked encoding is not being MT> used we cannot change this. MM> my bad, agree. In this case, I will keep this condition for ISE. And I will keep the current proposed javadoc for ISE. II. Framework/Filter issue MM> On a different tangent I think one use case would be for a filter to MM> add a trailer header before calling doChain() and then setting a value for MM> the trailer on return from doChain(). The filter could call setTrailers() before MM> calling doChain() but its supplier may have been replaced by the time doChain() MM> returns. Do we support multiple suppliers? MT> We don't. And I don't see a way for an application do anything like that MT> with the current API. GW> There was a brief discussion about this. I think it was decided this was GW> something that Frameworks should do. GW> However, it may be a good idea to add a getter for the trailers supplier GW> so apps/frameworks can know if a supplier has already been set and do GW> supplier composition themselves if they want. 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. Shing Wai Chan
|
|
Greg Wilkins
On 25 May 2017 at 22:59, Shing Wai Chan <shing.wai.chan@...> wrote:
I think it is a good thing to add:
cheers |
|
Martin Mulholland
servlet-spec@javaee.groups.io wrote on 05/26/2017 03:00:21 AM: > From: "Greg Wilkins" <gregw@...> > To: servlet-spec@javaee.groups.io > Date: 05/26/2017 03:00 AM > Subject: Re: [servlet-spec] setTrailerFields() throws ISE? > Sent by: servlet-spec@javaee.groups.io > >> On 25 May 2017 at 22:59, Shing Wai Chan <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 Agree. +1 ! > cheers > > -- > Greg Wilkins <gregw@...> CTO http://webtide.com Thanks, Martin. |
|
Shing Wai Chan
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. Any other comment? Thanks. Shing Wai Chan
|
|
Greg Wilkins
On 26 May 2017 at 19:15, Shing Wai Chan <shing.wai.chan@...> wrote:
+1 |
|
Mark Thomas
On 26/05/17 18:15, Shing Wai Chan wrote:
Right now, in HttpServletRequest, we have the following:+1 Mark Any other comment? |
|