From 3b4a3431d4cda6049f70a5f7e95a4b994bb59c7e Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 2 Feb 2021 11:05:06 +0100 Subject: [PATCH] Ignore null header value in MockHttpServletResponse Prior to this commit, calls to setHeader() and addHeader() in MockHttpServletResponse would result in an IllegalArgumentException or NullPointerException if the supplied header value was null. Although the Javadoc for setHeader(String, String) and addHeader(String, String) in javax.servlet.http.HttpServletResponse does not specify how a null header value should be handled, both Tomcat and Jetty simply ignore a null value. Furthermore, org.springframework.http.HttpHeaders.add(String, String) declares the headerValue parameter as @Nullable. This commit therefore updates MockHttpServletResponse to silently ignore null header values passed to setHeader() and addHeader(). Closes gh-26488 --- .../mock/web/MockHttpServletResponse.java | 18 +++++++---- .../web/MockHttpServletResponseTests.java | 30 ++++++++++++++++++- .../servlet/MockHttpServletResponse.java | 18 +++++++---- 3 files changed, 53 insertions(+), 13 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletResponse.java b/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletResponse.java index f0daaf90a62..823945b5974 100644 --- a/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletResponse.java +++ b/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletResponse.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -421,7 +421,7 @@ public class MockHttpServletResponse implements HttpServletResponse { @Override public boolean containsHeader(String name) { - return (this.headers.get(name) != null); + return this.headers.containsKey(name); } /** @@ -594,12 +594,12 @@ public class MockHttpServletResponse implements HttpServletResponse { } @Override - public void setHeader(String name, String value) { + public void setHeader(String name, @Nullable String value) { setHeaderValue(name, value); } @Override - public void addHeader(String name, String value) { + public void addHeader(String name, @Nullable String value) { addHeaderValue(name, value); } @@ -613,7 +613,10 @@ public class MockHttpServletResponse implements HttpServletResponse { addHeaderValue(name, value); } - private void setHeaderValue(String name, Object value) { + private void setHeaderValue(String name, @Nullable Object value) { + if (value == null) { + return; + } boolean replaceHeader = true; if (setSpecialHeader(name, value, replaceHeader)) { return; @@ -621,7 +624,10 @@ public class MockHttpServletResponse implements HttpServletResponse { doAddHeaderValue(name, value, replaceHeader); } - private void addHeaderValue(String name, Object value) { + private void addHeaderValue(String name, @Nullable Object value) { + if (value == null) { + return; + } boolean replaceHeader = false; if (setSpecialHeader(name, value, replaceHeader)) { return; diff --git a/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletResponseTests.java b/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletResponseTests.java index b105ea7953a..907a5cfac9f 100644 --- a/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletResponseTests.java +++ b/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletResponseTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,6 +26,8 @@ import javax.servlet.http.Cookie; import javax.servlet.http.HttpServletResponse; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.springframework.web.util.WebUtils; @@ -57,6 +59,32 @@ class MockHttpServletResponseTests { private MockHttpServletResponse response = new MockHttpServletResponse(); + @ParameterizedTest // gh-26488 + @ValueSource(strings = { + CONTENT_TYPE, + CONTENT_LENGTH, + CONTENT_LANGUAGE, + SET_COOKIE, + "enigma" + }) + void addHeaderWithNullValue(String headerName) { + response.addHeader(headerName, null); + assertThat(response.containsHeader(headerName)).isFalse(); + } + + @ParameterizedTest // gh-26488 + @ValueSource(strings = { + CONTENT_TYPE, + CONTENT_LENGTH, + CONTENT_LANGUAGE, + SET_COOKIE, + "enigma" + }) + void setHeaderWithNullValue(String headerName) { + response.setHeader(headerName, null); + assertThat(response.containsHeader(headerName)).isFalse(); + } + @Test void setContentType() { String contentType = "test/plain"; diff --git a/spring-web/src/testFixtures/java/org/springframework/web/testfixture/servlet/MockHttpServletResponse.java b/spring-web/src/testFixtures/java/org/springframework/web/testfixture/servlet/MockHttpServletResponse.java index ed9bae93376..5af669a25ec 100644 --- a/spring-web/src/testFixtures/java/org/springframework/web/testfixture/servlet/MockHttpServletResponse.java +++ b/spring-web/src/testFixtures/java/org/springframework/web/testfixture/servlet/MockHttpServletResponse.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -421,7 +421,7 @@ public class MockHttpServletResponse implements HttpServletResponse { @Override public boolean containsHeader(String name) { - return (this.headers.get(name) != null); + return this.headers.containsKey(name); } /** @@ -594,12 +594,12 @@ public class MockHttpServletResponse implements HttpServletResponse { } @Override - public void setHeader(String name, String value) { + public void setHeader(String name, @Nullable String value) { setHeaderValue(name, value); } @Override - public void addHeader(String name, String value) { + public void addHeader(String name, @Nullable String value) { addHeaderValue(name, value); } @@ -613,7 +613,10 @@ public class MockHttpServletResponse implements HttpServletResponse { addHeaderValue(name, value); } - private void setHeaderValue(String name, Object value) { + private void setHeaderValue(String name, @Nullable Object value) { + if (value == null) { + return; + } boolean replaceHeader = true; if (setSpecialHeader(name, value, replaceHeader)) { return; @@ -621,7 +624,10 @@ public class MockHttpServletResponse implements HttpServletResponse { doAddHeaderValue(name, value, replaceHeader); } - private void addHeaderValue(String name, Object value) { + private void addHeaderValue(String name, @Nullable Object value) { + if (value == null) { + return; + } boolean replaceHeader = false; if (setSpecialHeader(name, value, replaceHeader)) { return;