From f895d762cd7c393861d34a391b9a685e7545f305 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Tue, 25 Feb 2025 10:43:19 +0100 Subject: [PATCH] Remove duplicate Content-Type header in error cases Prior to this commit, the `DispatcherServlet` would try and reset the response buffer in case of errors, if the response is not committed already. This allows for more flexible error handling, even if the response was being handled already when it errored. Resetting the response buffer clears the body but leaves HTTP response headers intact. This is done on purpose as to not clear headers previously added by Servlet Filters. By leaving in place some headers like "Content-Type", this does not take into account the fact that the response body was cleared and that error handling will perform another round of content negotiation. While this isn't a problem for some Servlet containers which enforce a single "Content-Type" header value, this can cause multiple/duplicate values for some others. This commit ensures that the "Content-Type" response header is removed at the same time as we clear the "producible media types" attribute: another pass of content negotiation will be performed for error handling. Fixes gh-34366 --- .../web/servlet/DispatcherServlet.java | 8 +++++--- .../web/servlet/DispatcherServletTests.java | 20 ++++++++++++++++++- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java index 243148ee1efd..7b2c44271968 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 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. @@ -48,6 +48,7 @@ import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.support.PropertiesLoaderUtils; import org.springframework.core.log.LogFormatUtils; +import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; import org.springframework.http.server.RequestPath; @@ -1341,9 +1342,10 @@ protected ModelAndView processHandlerException(HttpServletRequest request, HttpS // Success and error responses may use different content types request.removeAttribute(HandlerMapping.PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE); - // Reset the response body buffer if the response is not committed already, - // leaving the response headers in place. + // Reset the response content-type header and body buffer if the response is not committed already, + // leaving the other response headers in place. try { + response.setHeader(HttpHeaders.CONTENT_TYPE, null); response.resetBuffer(); } catch (IllegalStateException illegalStateException) { diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/DispatcherServletTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/DispatcherServletTests.java index 65867661a495..bebbc9f2f1ea 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/DispatcherServletTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/DispatcherServletTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 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. @@ -924,6 +924,23 @@ void shouldAttemptToResetResponseBufferIfCommitted() throws Exception { assertThat(response.getHeader("Test-Header")).isEqualTo("spring"); } + @Test + void shouldResetContentTypeIfNotCommitted() throws Exception { + StaticWebApplicationContext context = new StaticWebApplicationContext(); + context.setServletContext(getServletContext()); + context.registerSingleton("/error", ErrorController.class); + DispatcherServlet servlet = new DispatcherServlet(context); + servlet.init(servletConfig); + + MockHttpServletRequest request = new MockHttpServletRequest(getServletContext(), "GET", "/error"); + MockHttpServletResponse response = new MockHttpServletResponse(); + assertThatThrownBy(() -> servlet.service(request, response)).isInstanceOf(ServletException.class) + .hasCauseInstanceOf(IllegalArgumentException.class); + assertThat(response.getContentAsByteArray()).isEmpty(); + assertThat(response.getStatus()).isEqualTo(400); + assertThat(response.getHeaderNames()).doesNotContain(HttpHeaders.CONTENT_TYPE); + } + public static class ControllerFromParent implements Controller { @@ -976,6 +993,7 @@ private static class ErrorController implements Controller { public ModelAndView handleRequest(HttpServletRequest request, HttpServletResponse response) throws Exception { response.setStatus(400); response.setHeader("Test-Header", "spring"); + response.addHeader("Content-Type", "application/json"); if (request.getAttribute("commit") != null) { response.flushBuffer(); }