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 10:05:03 +1000

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.

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