From a1463c2bf2a6bf43e077d200b556136e2d5c48ec Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Tue, 9 Jan 2024 12:09:54 +0000 Subject: [PATCH] Do not set exception attribute if response body is set ResponseEntityExceptionHandler should not set the exception attribute when there is a response body, and the response is fully handled. Closes gh-31541 --- .../ResponseEntityExceptionHandler.java | 10 +++++----- .../ResponseEntityExceptionHandlerTests.java | 15 +++++++++------ 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandler.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandler.java index eb47bb6e048..b2638327638 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandler.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2024 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. @@ -560,14 +560,14 @@ public abstract class ResponseEntityExceptionHandler implements MessageSourceAwa } } - if (statusCode.equals(HttpStatus.INTERNAL_SERVER_ERROR)) { - request.setAttribute(WebUtils.ERROR_EXCEPTION_ATTRIBUTE, ex, WebRequest.SCOPE_REQUEST); - } - if (body == null && ex instanceof ErrorResponse errorResponse) { body = errorResponse.updateAndGetBody(this.messageSource, LocaleContextHolder.getLocale()); } + if (statusCode.equals(HttpStatus.INTERNAL_SERVER_ERROR) && body == null) { + request.setAttribute(WebUtils.ERROR_EXCEPTION_ATTRIBUTE, ex, WebRequest.SCOPE_REQUEST); + } + return createResponseEntity(body, headers, statusCode, request); } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandlerTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandlerTests.java index 12529ef528a..d30867ccab7 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandlerTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandlerTests.java @@ -32,7 +32,6 @@ import org.springframework.context.support.StaticMessageSource; import org.springframework.core.MethodParameter; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; -import org.springframework.http.HttpStatus; import org.springframework.http.HttpStatusCode; import org.springframework.http.MediaType; import org.springframework.http.ProblemDetail; @@ -267,6 +266,15 @@ public class ResponseEntityExceptionHandlerTests { testException(new AsyncRequestTimeoutException()); } + @Test // gh-14287, gh-31541 + void serverErrorWithoutBody() { + HttpStatusCode code = HttpStatusCode.valueOf(500); + Exception ex = new IllegalStateException("internal error"); + this.exceptionHandler.handleExceptionInternal(ex, null, new HttpHeaders(), code, this.request); + + assertThat(this.servletRequest.getAttribute("jakarta.servlet.error.exception")).isSameAs(ex); + } + @Test public void controllerAdvice() throws Exception { StaticWebApplicationContext ctx = new StaticWebApplicationContext(); @@ -343,11 +351,6 @@ public class ResponseEntityExceptionHandlerTests { try { ResponseEntity entity = this.exceptionHandler.handleException(ex, this.request); - // SPR-9653 - if (HttpStatus.INTERNAL_SERVER_ERROR.equals(entity.getStatusCode())) { - assertThat(this.servletRequest.getAttribute("jakarta.servlet.error.exception")).isSameAs(ex); - } - // Verify DefaultHandlerExceptionResolver would set the same status this.exceptionResolver.resolveException(this.servletRequest, this.servletResponse, null, ex); assertThat(entity.getStatusCode().value()).isEqualTo(this.servletResponse.getStatus());