Skip to content

Commit

Permalink
Avoid 406 Not Acceptable for error pages
Browse files Browse the repository at this point in the history
Prior to this commit, the `ErrorController` would override the original
error response status if the error map cannot be written due to content
negotiation with the HTTP client. In that case, the error handling
infrastructure returns a `406 Not Acceptable` response.

This commit improves the `ErrorController` so that
`HttpMediaTypeNotAcceptableException` instances thrown by that
controller are not returned as is but instead we write the error
response with an empty body and the original HTTP error status.

Fixes gh-19522
  • Loading branch information
bclozel committed Jan 3, 2020
1 parent c2f8741 commit cc154bb
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import org.springframework.http.ResponseEntity;
import org.springframework.stereotype.Controller;
import org.springframework.util.Assert;
import org.springframework.web.HttpMediaTypeNotAcceptableException;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.servlet.ModelAndView;

Expand Down Expand Up @@ -102,6 +104,12 @@ public ResponseEntity<Map<String, Object>> error(HttpServletRequest request) {
return new ResponseEntity<>(body, status);
}

@ExceptionHandler(HttpMediaTypeNotAcceptableException.class)
public ResponseEntity<String> mediaTypeNotAcceptable(HttpServletRequest request) {
HttpStatus status = getStatus(request);
return ResponseEntity.status(status).build();
}

/**
* Determine if the stacktrace attribute should be included.
* @param request the source request
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,17 @@ void testConventionTemplateMapping() {
assertThat(resp).contains("We are out of storage");
}

@Test
void testIncompatibleMediaType() {
load();
RequestEntity<?> request = RequestEntity.get(URI.create(createUrl("/incompatibleType")))
.accept(MediaType.TEXT_PLAIN).build();
ResponseEntity<String> entity = new TestRestTemplate().exchange(request, String.class);
assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST);
assertThat(entity.getHeaders().getContentType()).isNull();
assertThat(entity.getBody()).isNull();
}

private void assertErrorAttributes(Map<?, ?> content, String status, String error, Class<?> exception,
String message, String path) {
assertThat(content.get("status")).as("Wrong status").isEqualTo(status);
Expand Down Expand Up @@ -298,6 +309,11 @@ String noStorage() {
throw new InsufficientStorageException();
}

@RequestMapping(path = "/incompatibleType", produces = "text/plain")
String incompatibleType() {
throw new ExpectedException();
}

@ResponseStatus(value = HttpStatus.BAD_REQUEST, reason = "Expected!")
@SuppressWarnings("serial")
static class ExpectedException extends RuntimeException {
Expand Down

0 comments on commit cc154bb

Please sign in to comment.