From e2e52a11ccb87f799e5928065557ea5e7688ffe0 Mon Sep 17 00:00:00 2001 From: Aurdo Date: Thu, 4 Jun 2020 22:01:33 +0300 Subject: [PATCH 1/2] Fix BasicErrorController include parameter parsing This commit fixes an error in BasicErrorController where the wrong property was referenced for binding error inclusion. See gh-21702 --- .../autoconfigure/web/servlet/error/BasicErrorController.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/error/BasicErrorController.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/error/BasicErrorController.java index 5486cf9f176..42bc6fa97c9 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/error/BasicErrorController.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/error/BasicErrorController.java @@ -172,7 +172,7 @@ public class BasicErrorController extends AbstractErrorController { * @return if the errors attribute should be included */ protected boolean isIncludeBindingErrors(HttpServletRequest request, MediaType produces) { - switch (getErrorProperties().getIncludeMessage()) { + switch (getErrorProperties().getIncludeBindingErrors()) { case ALWAYS: return true; case ON_PARAM: From 73aff08535ab68a982ce52274c5cff252b4f72de Mon Sep 17 00:00:00 2001 From: Scott Frederick Date: Mon, 8 Jun 2020 14:59:43 -0500 Subject: [PATCH 2/2] Improve tests for BasicErrorController This commit improves the tests for BasicErrorController by decoupling coverage for the include-message and include-binding-errors parameters to ensure the options operate properly independent of each other. See gh-21702 --- .../BasicErrorControllerIntegrationTests.java | 87 ++++++++++++++----- 1 file changed, 63 insertions(+), 24 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/error/BasicErrorControllerIntegrationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/error/BasicErrorControllerIntegrationTests.java index ac5f9ec3deb..61d6abda8c5 100755 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/error/BasicErrorControllerIntegrationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/error/BasicErrorControllerIntegrationTests.java @@ -205,50 +205,89 @@ class BasicErrorControllerIntegrationTests { } @Test - void testBindingExceptionForMachineClientWithMessageAndErrorsParamTrue() { - load("--server.error.include-exception=true", "--server.error.include-message=on-param", - "--server.error.include-binding-errors=on-param"); - bindingExceptionWithMessageAndErrors("?message=true&errors=true"); + void testBindingExceptionForMachineClientWithErrorsParamTrue() { + load("--server.error.include-exception=true", "--server.error.include-binding-errors=on-param"); + bindingExceptionWithErrors("?errors=true"); } @Test - void testBindingExceptionForMachineClientWithMessageAndErrorsParamFalse() { - load("--server.error.include-exception=true", "--server.error.include-message=on-param", - "--server.error.include-binding-errors=on-param"); - bindingExceptionWithoutMessageAndErrors("?message=false&errors=false"); + void testBindingExceptionForMachineClientWithErrorsParamFalse() { + load("--server.error.include-exception=true", "--server.error.include-binding-errors=on-param"); + bindingExceptionWithoutErrors("?errors=false"); } @Test - void testBindingExceptionForMachineClientWithMessageAndErrorsParamAbsent() { - load("--server.error.include-exception=true", "--server.error.include-message=on-param", - "--server.error.include-binding-errors=on-param"); - bindingExceptionWithoutMessageAndErrors(""); + void testBindingExceptionForMachineClientWithErrorsParamAbsent() { + load("--server.error.include-exception=true", "--server.error.include-binding-errors=on-param"); + bindingExceptionWithoutErrors(""); } @Test - void testBindingExceptionForMachineClientAlwaysMessageAndErrors() { - load("--server.error.include-exception=true", "--server.error.include-message=always", - "--server.error.include-binding-errors=always"); - bindingExceptionWithMessageAndErrors("?message=false&errors=false"); + void testBindingExceptionForMachineClientAlwaysErrors() { + load("--server.error.include-exception=true", "--server.error.include-binding-errors=always"); + bindingExceptionWithErrors("?errors=false"); } @Test - void testBindingExceptionForMachineClientNeverMessageAndErrors() { - load("--server.error.include-exception=true", "--server.error.include-message=never", - "--server.error.include-binding-errors=never"); - bindingExceptionWithoutMessageAndErrors("?message=true&errors=true"); + void testBindingExceptionForMachineClientNeverErrors() { + load("--server.error.include-exception=true", "--server.error.include-binding-errors=never"); + bindingExceptionWithoutErrors("?errors=true"); + } + + @Test + void testBindingExceptionForMachineClientWithMessageParamTrue() { + load("--server.error.include-exception=true", "--server.error.include-message=on-param"); + bindingExceptionWithMessage("?message=true"); + } + + @Test + void testBindingExceptionForMachineClientWithMessageParamFalse() { + load("--server.error.include-exception=true", "--server.error.include-message=on-param"); + bindingExceptionWithoutMessage("?message=false"); + } + + @Test + void testBindingExceptionForMachineClientWithMessageParamAbsent() { + load("--server.error.include-exception=true", "--server.error.include-message=on-param"); + bindingExceptionWithoutMessage(""); + } + + @Test + void testBindingExceptionForMachineClientAlwaysMessage() { + load("--server.error.include-exception=true", "--server.error.include-message=always"); + bindingExceptionWithMessage("?message=false"); + } + + @Test + void testBindingExceptionForMachineClientNeverMessage() { + load("--server.error.include-exception=true", "--server.error.include-message=never"); + bindingExceptionWithoutMessage("?message=true"); } @SuppressWarnings({ "rawtypes" }) - private void bindingExceptionWithMessageAndErrors(String param) { + private void bindingExceptionWithErrors(String param) { ResponseEntity entity = new TestRestTemplate().getForEntity(createUrl("/bind" + param), Map.class); - assertErrorAttributes(entity.getBody(), "400", "Bad Request", BindException.class, - "Validation failed for object='test'. Error count: 1", "/bind"); + assertErrorAttributes(entity.getBody(), "400", "Bad Request", BindException.class, "", "/bind"); assertThat(entity.getBody().containsKey("errors")).isTrue(); } @SuppressWarnings({ "rawtypes" }) - private void bindingExceptionWithoutMessageAndErrors(String param) { + private void bindingExceptionWithoutErrors(String param) { + ResponseEntity entity = new TestRestTemplate().getForEntity(createUrl("/bind" + param), Map.class); + assertErrorAttributes(entity.getBody(), "400", "Bad Request", BindException.class, "", "/bind"); + assertThat(entity.getBody().containsKey("errors")).isFalse(); + } + + @SuppressWarnings({ "rawtypes" }) + private void bindingExceptionWithMessage(String param) { + ResponseEntity entity = new TestRestTemplate().getForEntity(createUrl("/bind" + param), Map.class); + assertErrorAttributes(entity.getBody(), "400", "Bad Request", BindException.class, + "Validation failed for object='test'. Error count: 1", "/bind"); + assertThat(entity.getBody().containsKey("errors")).isFalse(); + } + + @SuppressWarnings({ "rawtypes" }) + private void bindingExceptionWithoutMessage(String param) { ResponseEntity entity = new TestRestTemplate().getForEntity(createUrl("/bind" + param), Map.class); assertErrorAttributes(entity.getBody(), "400", "Bad Request", BindException.class, "", "/bind"); assertThat(entity.getBody().containsKey("errors")).isFalse();