From 2593b60f2beab3e32aaf656158f1e480e73aae15 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 --- .../annotation/ResponseEntityExceptionHandler.java | 10 +++++----- .../ResponseEntityExceptionHandlerTests.java | 14 +++++++++----- 2 files changed, 14 insertions(+), 10 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 d591d0248e44..365c23e58c22 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-2023 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. @@ -658,14 +658,14 @@ protected ResponseEntity handleExceptionInternal( } } - 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 bde170a0d6cb..c186111642a6 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 @@ -323,6 +323,15 @@ public void maxUploadSizeExceededException() { testException(new MaxUploadSizeExceededException(1000)); } + @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(); @@ -400,11 +409,6 @@ private ResponseEntity testException(Exception ex) { ResponseEntity entity = this.exceptionHandler.handleException(ex, this.request); assertThat(entity).isNotNull(); - // 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());