jsr369-experts@servlet-spec.java.net

[jsr369-experts] Re: [SERVLET-SPEC-159] HttpServletResponse.setHeader() and null

From: Stuart Douglas <sdouglas_at_redhat.com>
Date: Mon, 5 Sep 2016 11:41:28 +1000

On Mon, Sep 5, 2016 at 10:05 AM, Stuart Douglas <sdouglas_at_redhat.com> wrote:
> On Sat, Sep 3, 2016 at 6:22 AM, Edward Burns <edward.burns_at_oracle.com> wrote:
>> Hello Volunteers,
>>
>> I'm trying to pick up the dropped threads. If you are looking for an
>> explanation for why they were dropped in the first place, I can't say
>> much, but I said something here [1].
>>
>> Stuart asks for the feature:
>>
>>>>>>> On Mon, 22 Dec 2014 18:29:16 -0500 (EST), Stuart Douglas <sdouglas_at_redhat.com> said:
>>
>> SD> At the moment there does not seem to be a way to remove a header
>> SD> from the response. It might be expected that calling
>> SD> HttpServletResponse.setHeader("name", null) will accomplish this,
>> SD> however it does not seem to be explicitly specified in the spec or
>> SD> the javadoc.
>>
>> SD> What does everyone think about adding some clarification to the
>> SD> javadoc that explicitly states that if setHeader is called with a
>> SD> null value then the header will be cleared?
>>
>> There is some discussion of what other containers do with null name
>> and/or value.
>>
>> * Tomcat and TmaxSoft ignores null values.
>>
>> Mark points out some complexities that would arise when honoring
>> Stuart's request:
>>
>> * what to do about headers added after the request leave
>> the servlet container? If the app says to remove it, but it later
>> gets added, should be disallow it?
>>
>> * Should we disallow removal of essential headers for valid HTTP?
>>
>>>>>>> On Tue, 23 Dec 2014 05:27:56 -0500 (EST), Stuart Douglas <sdouglas_at_redhat.com> said:
>>
>> SD> a set header call with a null value is currently undefined
>> SD> behaviour. I think it would be cleaner to specify what should happen
>> SD> in this case, it should either remove the header or throw an
>> SD> IllegalArgumentException.
>>
>> SD> In Undertow setHeader with null will wipe the header, addHeader will
>> SD> do nothing, and containsHeader will return true if setHeader(null)
>> SD> had been called.
>>
>>>>>>> On Wed, 31 Dec 2014 14:31:35 +0100, Greg Wilkins <gregw_at_intalio.com> said:
>>
>> GW> Jetty does the same. Or to be more precise a set header will remove the
>> GW> previous value and then ignore the null and not set a new value.
>>
>> GW> While I don't see any massive use-case to allow removing headers, I don't
>> GW> see a good reason for not doing it either. Apps can break things just as
>> GW> easily by adding/setting headers as by removing them, so no extra danger
>> GW> there. Plus they have the power to wrap if they want, so nothing new.
>>
>> GW> So I don't think we need a new delete header method, but I do think we
>> GW> should define what a null value does. I think that the behaviour as
>> GW> described by Stuart is reasonable.
>>
>>>>>>> On Wed, 31 Dec 2014 19:18:22 +0000, Mark Thomas <markt_at_apache.org> said:
>>
>> MT> I know I previously said I'd be fine with IAE in all cases. I'd be fine
>> MT> with something along the lines of the above as well but there are a few
>> MT> details I'd change:
>>
>> MT> setHeader(null, anything) -> IAE or NO-OP
>> MT> (slight preference for NO-OP on reflection)
>> MT> setHeader(X, null) -> removes header
>> MT> setHeader(X, "") -> adds header with empty value
>>
>>>>>>> On Sun, 4 Jan 2015 16:36:34 -0500 (EST), Stuart Douglas <sdouglas_at_redhat.com> said:
>>
>> SD> I think the no-op for setHeader(null, anything) is better, otherwise
>> SD> we risk breaking existing code (although ideally existing code
>> SD> should not be doing this anyway, so I am not sure how big an issue
>> SD> this is).
>>
>> MT> Note: If the header has been removed, containsHeader(X) would return false
>>
>> MT> containsHeader(null) would always return false
>>
>> MT> I'd also add something along the lines of the following clarification,
>> MT> e.g. to the setHeader() Javadoc.
>>
>> MT> Note: Containers may still add headers (e.g. for specification
>> MT> compliance) after the request/response has returned to the container.
>> MT> Where these headers are not required for specification compliance the
>> MT> container should provide a container specific option to not add the
>> MT> header.
>>
>>>>>>> On Thu, 1 Jan 2015 11:42:25 +0100, Greg Wilkins <gregw_at_intalio.com> said:
>>
>> GW> I think I'm on board with this.
>> GW> What you are saying is that a container may add headers like
>> GW> "Connection:close" for protocol compliance or like "Server: Wibble/9.9" for
>> GW> non compliance reasons. Container must always be allowed to do protocol
>> GW> compliance, but should allow other headers to be turned off.
>>
>> GW> That way we wont see the creation of filters whose job it is to remove the
>> GW> Server header and who then complain if the container adds that during
>> GW> commit.
>>
>> I oppose adding any more container specific options. I judge it is
>> better to simply say:
>>
>> Containers may still add headers after processing of the request or
>> response returns to the container.
>
> I don't really like this text, as the new headers are generally added
> when the response is commited, which could be before the control
> returns to the container.
>
> How about something like:
>
> "Containers may still add or modify headers before the response is
> committed. These will generally be headers that are required at a
> protocol level."
>
>>
>> If someone wants to request a standardized way to control such bonus
>> headers, let us address it as another feature request.
>
> I don't think we need any standardized way to control additional headers.
>
> As far as I am aware most containers already offer a non standard way
> of controlling 'unnecessary' headers such as Server, and we should not
> allow the user to mess with headers that are required for protocol
> compliance (such as Content-Length and Transfer-Encoding).
>
> For what it is worth in addition to configuration we also deal with
> this by adding additional headers before the request starts, so if the
> user really wanted to modify them they can, while protocol level
> headers are added just before commit so the user cannot mess with them
> and potentially break the connection.

Just realized I was not particularly clear about what I am describing
here, this is how Undertow deals with this situation. Optional headers
are added before Servlet processing, while required headers are added
on commit.

Stuart

>
> Stuart
>
>>
>> There was additional tangential content on the thread that I will
>> address in another email.
>>
>> Ed
>>
>> --
>> | edward.burns_at_oracle.com | office: +1 407 458 0017
>>
>>
>> [1] https://java.net/projects/javaserverfaces-spec-public/lists/jsr372-experts/archive/2016-09/message/8