From d052fd73b3279abe9f828bd305f7bbf00d7e0c30 Mon Sep 17 00:00:00 2001
From: Daniel Gerhardt <code@dgerhardt.net>
Date: Thu, 10 Jan 2019 16:07:06 +0100
Subject: [PATCH] Improve handling of Spring's own HTTP Exceptions
Slighly refactored ControllerAdvices to include handling of Spring's
HTTP Exceptions. This fixes 500 status codes being sent instead of the
correct one.
Fixes GH-67.
---
.../ControllerExceptionHandler.java | 59 +++++++++----------
...er.java => ControllerExceptionHelper.java} | 6 +-
.../DefaultControllerExceptionHandler.java | 10 +++-
3 files changed, 41 insertions(+), 34 deletions(-)
rename src/main/java/de/thm/arsnova/controller/{AbstractControllerExceptionHandler.java => ControllerExceptionHelper.java} (86%)
diff --git a/src/main/java/de/thm/arsnova/controller/ControllerExceptionHandler.java b/src/main/java/de/thm/arsnova/controller/ControllerExceptionHandler.java
index 2016f8def..29b76fcaf 100644
--- a/src/main/java/de/thm/arsnova/controller/ControllerExceptionHandler.java
+++ b/src/main/java/de/thm/arsnova/controller/ControllerExceptionHandler.java
@@ -27,19 +27,20 @@ import de.thm.arsnova.web.exceptions.PreconditionFailedException;
import de.thm.arsnova.web.exceptions.UnauthorizedException;
import org.ektorp.DocumentNotFoundException;
import org.slf4j.event.Level;
+import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
-import org.springframework.http.converter.HttpMessageNotReadableException;
+import org.springframework.http.ResponseEntity;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.authentication.AnonymousAuthenticationToken;
import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContextHolder;
-import org.springframework.web.HttpRequestMethodNotSupportedException;
import org.springframework.web.bind.annotation.ControllerAdvice;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.ResponseBody;
import org.springframework.web.bind.annotation.ResponseStatus;
-import org.springframework.web.servlet.NoHandlerFoundException;
+import org.springframework.web.context.request.WebRequest;
+import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler;
import javax.naming.OperationNotSupportedException;
import javax.servlet.http.HttpServletRequest;
@@ -50,40 +51,39 @@ import java.util.Map;
* Translates exceptions into HTTP status codes.
*/
@ControllerAdvice
-public class ControllerExceptionHandler extends AbstractControllerExceptionHandler {
+public class ControllerExceptionHandler extends ResponseEntityExceptionHandler {
+ private ControllerExceptionHelper helper;
+
+ public ControllerExceptionHandler(final ControllerExceptionHelper helper) {
+ this.helper = helper;
+ }
+
@ExceptionHandler(NoContentException.class)
@ResponseBody
@ResponseStatus(HttpStatus.NO_CONTENT)
public Map<String, Object> handleNoContentException(final Exception e, final HttpServletRequest request) {
- return handleException(e, Level.TRACE);
- }
-
- @ExceptionHandler(NoHandlerFoundException.class)
- @ResponseBody
- @ResponseStatus(HttpStatus.NOT_FOUND)
- public Map<String, Object> handleNoHandlerFoundException(final Exception e, final HttpServletRequest request) {
- return handleException(e, Level.TRACE);
+ return helper.handleException(e, Level.TRACE);
}
@ExceptionHandler(NotFoundException.class)
@ResponseBody
@ResponseStatus(HttpStatus.NOT_FOUND)
public Map<String, Object> handleNotFoundException(final Exception e, final HttpServletRequest request) {
- return handleException(e, Level.TRACE);
+ return helper.handleException(e, Level.TRACE);
}
@ExceptionHandler(UnauthorizedException.class)
@ResponseBody
@ResponseStatus(HttpStatus.UNAUTHORIZED)
public Map<String, Object> handleUnauthorizedException(final Exception e, final HttpServletRequest request) {
- return handleException(e, Level.TRACE);
+ return helper.handleException(e, Level.TRACE);
}
@ExceptionHandler(AuthenticationCredentialsNotFoundException.class)
@ResponseStatus(HttpStatus.UNAUTHORIZED)
@ResponseBody
public Map<String, Object> handleAuthenticationCredentialsNotFoundException(final Exception e, final HttpServletRequest request) {
- return handleException(e, Level.DEBUG);
+ return helper.handleException(e, Level.DEBUG);
}
@ExceptionHandler(AccessDeniedException.class)
@@ -102,49 +102,42 @@ public class ControllerExceptionHandler extends AbstractControllerExceptionHandl
response.setStatus(HttpStatus.FORBIDDEN.value());
}
- return handleException(e, Level.DEBUG);
+ return helper.handleException(e, Level.DEBUG);
}
@ExceptionHandler(ForbiddenException.class)
@ResponseBody
@ResponseStatus(HttpStatus.FORBIDDEN)
public Map<String, Object> handleForbiddenException(final Exception e, final HttpServletRequest request) {
- return handleException(e, Level.DEBUG);
+ return helper.handleException(e, Level.DEBUG);
}
- @ExceptionHandler({BadRequestException.class, HttpRequestMethodNotSupportedException.class})
+ @ExceptionHandler(BadRequestException.class)
@ResponseBody
@ResponseStatus(HttpStatus.BAD_REQUEST)
public Map<String, Object> handleBadRequestException(final Exception e, final HttpServletRequest request) {
- return handleException(e, Level.DEBUG);
+ return helper.handleException(e, Level.DEBUG);
}
@ExceptionHandler(PreconditionFailedException.class)
@ResponseBody
@ResponseStatus(HttpStatus.PRECONDITION_FAILED)
public Map<String, Object> handlePreconditionFailedException(final Exception e, final HttpServletRequest request) {
- return handleException(e, Level.DEBUG);
+ return helper.handleException(e, Level.DEBUG);
}
@ExceptionHandler({NotImplementedException.class, OperationNotSupportedException.class})
@ResponseBody
@ResponseStatus(HttpStatus.NOT_IMPLEMENTED)
public Map<String, Object> handleNotImplementedException(final Exception e, final HttpServletRequest request) {
- return handleException(e, Level.DEBUG);
+ return helper.handleException(e, Level.DEBUG);
}
@ExceptionHandler(PayloadTooLargeException.class)
@ResponseBody
@ResponseStatus(HttpStatus.PAYLOAD_TOO_LARGE)
public Map<String, Object> handlePayloadTooLargeException(final Exception e, final HttpServletRequest request) {
- return handleException(e, Level.DEBUG);
- }
-
- @ExceptionHandler(HttpMessageNotReadableException.class)
- @ResponseBody
- @ResponseStatus(HttpStatus.BAD_REQUEST)
- public Map<String, Object> handleHttpMessageNotReadableException(final Exception e, final HttpServletRequest request) {
- return handleException(e, Level.DEBUG);
+ return helper.handleException(e, Level.DEBUG);
}
/* FIXME: Wrap persistance Exceptions - do not handle persistance Exceptions at the controller layer */
@@ -152,6 +145,12 @@ public class ControllerExceptionHandler extends AbstractControllerExceptionHandl
@ResponseBody
@ResponseStatus(HttpStatus.NOT_FOUND)
public Map<String, Object> handleDocumentNotFoundException(final Exception e, final HttpServletRequest request) {
- return handleException(e, Level.TRACE);
+ return helper.handleException(e, Level.TRACE);
+ }
+
+ @Override
+ protected ResponseEntity<Object> handleExceptionInternal(final Exception ex, final Object body,
+ final HttpHeaders headers, final HttpStatus status, final WebRequest request) {
+ return new ResponseEntity<>(helper.handleException(ex, Level.TRACE), headers, status);
}
}
diff --git a/src/main/java/de/thm/arsnova/controller/AbstractControllerExceptionHandler.java b/src/main/java/de/thm/arsnova/controller/ControllerExceptionHelper.java
similarity index 86%
rename from src/main/java/de/thm/arsnova/controller/AbstractControllerExceptionHandler.java
rename to src/main/java/de/thm/arsnova/controller/ControllerExceptionHelper.java
index 58e3f907e..50abc01c0 100644
--- a/src/main/java/de/thm/arsnova/controller/AbstractControllerExceptionHandler.java
+++ b/src/main/java/de/thm/arsnova/controller/ControllerExceptionHelper.java
@@ -5,12 +5,14 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.slf4j.event.Level;
import org.springframework.beans.factory.annotation.Value;
+import org.springframework.stereotype.Component;
import java.util.HashMap;
import java.util.Map;
-public class AbstractControllerExceptionHandler {
- private static final Logger logger = LoggerFactory.getLogger(AbstractControllerExceptionHandler.class);
+@Component
+public class ControllerExceptionHelper {
+ private static final Logger logger = LoggerFactory.getLogger(ControllerExceptionHelper.class);
/* Since exception messages might contain sensitive data, they are not exposed by default. */
@Value("${api.expose-error-messages:false}") private boolean exposeMessages;
diff --git a/src/main/java/de/thm/arsnova/controller/DefaultControllerExceptionHandler.java b/src/main/java/de/thm/arsnova/controller/DefaultControllerExceptionHandler.java
index 3d1249280..a788aa791 100644
--- a/src/main/java/de/thm/arsnova/controller/DefaultControllerExceptionHandler.java
+++ b/src/main/java/de/thm/arsnova/controller/DefaultControllerExceptionHandler.java
@@ -12,7 +12,13 @@ import javax.servlet.http.HttpServletRequest;
import java.util.Map;
@ControllerAdvice
-public class DefaultControllerExceptionHandler extends AbstractControllerExceptionHandler {
+public class DefaultControllerExceptionHandler {
+ private ControllerExceptionHelper helper;
+
+ public DefaultControllerExceptionHandler(final ControllerExceptionHelper helper) {
+ this.helper = helper;
+ }
+
@ExceptionHandler
@ResponseBody
@ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR)
@@ -27,6 +33,6 @@ public class DefaultControllerExceptionHandler extends AbstractControllerExcepti
throw e;
}
- return handleException(e, Level.ERROR);
+ return helper.handleException(e, Level.ERROR);
}
}
--
GitLab