Ignore null header value in MockHttpServletResponse

[imaxvell]

Hello,

I've faced an issue recently working with MockHttpServletResponse. The thing is when I have conditional (value can be null) headers in my controller, and I want to test them using MockMvc it fails, because MockHttpServletResponse checks if header value in response is null.

Below is what I am trying to do:

    @GetMapping("/example")
    public ResponseEntity<?> example() {
        String nullableHeaderValue = conditionalHeaderValue();
        return ResponseEntity.ok()
                .header("Some-header", nullableHeaderValue)
                .body("Hi");
    }

So the intention is to send header if its value is present and it works as expected. But when I test it with MockMvc it throws an IllegalArgumentException due to this check in MockHttpServletResponse:

    private void doAddHeaderValue(String name, Object value, boolean replace) {
        HeaderValueHolder header = this.headers.get(name);
        Assert.notNull(value, "Header value must not be null");
        if (header == null) {
            header = new HeaderValueHolder();
            this.headers.put(name, header);
        }
        if (replace) {
            header.setValue(value);
        }
        else {
            header.addValue(value);
        }
    }

I know that as workaround instead of nullability of value of header I can put headers themselves conditionally, but the thing is that spec of HttpServletResponse allows me to have nullable header values, while MockHttpServletResponse does not, and this behavior is surprising to say the least.

[sbrannen]

The Javadoc for addHeader() in HttpServletResponse is unclear as to whether or not null is permitted as a header value:

value the additional header value. If it contains octet string, it should be encoded according to RFC 2047

And as you pointed out, org.springframework.http.HttpHeaders.add(String, String) declares the headerValue parameter as @Nullable.

However, the Tomcat implementation (org.apache.catalina.connector.Response.addHeader(String, String, Charset)) simply ignores an attempt to add a header if the supplied value is null; whereas, the Jetty implementation (org.eclipse.jetty.server.Response.addHeader(String, String)) appears to allow null unless you're setting the Content-Type header.

So I'm a bit undecided about whether we should remove the not-null assertion in MockHttpServletResponse.

@rstoyanchev, thoughts?

[sbrannen]

On second thought, since neither Tomcat nor Jetty throws an IllegalArgumentException for a null header value and since our own HttpHeaders allows null header values as input, I think we should remove the not-null assertion in MockHttpServletResponse and simply ignore null header values.

@rstoyanchev / @jhoeller, are you OK with that?

[sbrannen]

This change will be included in 5.3.4, but feel free to try it out in an upcoming 5.3.4 snapshot.

[imaxvell]

@sbrannen Thanks, appreciate your quick response.