From a39b940fd0a89c40a8672c94465580ecc02454c7 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 28 Jul 2020 11:31:06 +0200 Subject: [PATCH 01/13] Remove obsolete handlers first. #7134 --- .../BadRequestExceptionHandler.java | 49 ------------------- .../ForbiddenExceptionHandler.java | 36 -------------- .../InternalServerErrorExceptionHandler.java | 39 --------------- .../NotAllowedExceptionHandler.java | 31 ------------ .../NotFoundExceptionHandler.java | 43 ---------------- .../RedirectionExceptionHandler.java | 32 ------------ .../ServiceUnavailableExceptionHandler.java | 43 ---------------- 7 files changed, 273 deletions(-) delete mode 100644 src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/BadRequestExceptionHandler.java delete mode 100644 src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ForbiddenExceptionHandler.java delete mode 100644 src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/InternalServerErrorExceptionHandler.java delete mode 100644 src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/NotAllowedExceptionHandler.java delete mode 100644 src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/NotFoundExceptionHandler.java delete mode 100644 src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/RedirectionExceptionHandler.java delete mode 100644 src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ServiceUnavailableExceptionHandler.java diff --git a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/BadRequestExceptionHandler.java b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/BadRequestExceptionHandler.java deleted file mode 100644 index f6412a871ac..00000000000 --- a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/BadRequestExceptionHandler.java +++ /dev/null @@ -1,49 +0,0 @@ -/* - * To change this license header, choose License Headers in Project Properties. - * To change this template file, choose Tools | Templates - * and open the template in the editor. - */ -package edu.harvard.iq.dataverse.api.errorhandlers; - -import edu.harvard.iq.dataverse.util.BundleUtil; -import javax.json.Json; -import javax.servlet.http.HttpServletRequest; -import javax.ws.rs.BadRequestException; -import javax.ws.rs.core.Context; -import javax.ws.rs.core.Response; -import javax.ws.rs.ext.ExceptionMapper; -import javax.ws.rs.ext.Provider; - -/** - * - * @author skraffmi - */ -@Provider -public class BadRequestExceptionHandler implements ExceptionMapper { - - @Context - HttpServletRequest request; - - @Override - public Response toResponse(BadRequestException ex) { - System.out.print( ex.getMessage()); - String uri = request.getRequestURI(); - String exMessage = ex.getMessage(); - String outputMessage; - if (exMessage != null && exMessage.toLowerCase().startsWith("tabular data required")) { - outputMessage = BundleUtil.getStringFromBundle("access.api.exception.metadata.not.available.for.nontabular.file"); - } else { - outputMessage = "Bad Request. The API request cannot be completed with the parameters supplied. Please check your code for typos, or consult our API guide at http://guides.dataverse.org."; - } - return Response.status(400) - .entity( Json.createObjectBuilder() - .add("status", "ERROR") - .add("code", 400) - .add("message", "'" + uri + "' " + outputMessage) - .build()) - .type("application/json").build(); - - - } - -} diff --git a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ForbiddenExceptionHandler.java b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ForbiddenExceptionHandler.java deleted file mode 100644 index 8096b719832..00000000000 --- a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ForbiddenExceptionHandler.java +++ /dev/null @@ -1,36 +0,0 @@ -package edu.harvard.iq.dataverse.api.errorhandlers; - -import javax.json.Json; -import javax.servlet.http.HttpServletRequest; -import javax.ws.rs.ForbiddenException; -import javax.ws.rs.core.Context; -import javax.ws.rs.core.Response; -import javax.ws.rs.ext.ExceptionMapper; -import javax.ws.rs.ext.Provider; - -/** - * Produces custom 403 messages for the API. - * @author michael - */ -@Provider -public class ForbiddenExceptionHandler implements ExceptionMapper{ - - @Context - HttpServletRequest request; - - @Override - public Response toResponse(ForbiddenException ex){ - String uri = request.getRequestURI(); - return Response.status(403) - .entity( Json.createObjectBuilder() - .add("status", "ERROR") - .add("code", 403) - .add("message", "'" + uri + "' you are not authorized to access this object via this api endpoint. Please check your code for typos, or consult our API guide at http://guides.dataverse.org.") - .build()) - .type("application/json").build(); - - - } - -} - diff --git a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/InternalServerErrorExceptionHandler.java b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/InternalServerErrorExceptionHandler.java deleted file mode 100644 index f59dd5029fc..00000000000 --- a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/InternalServerErrorExceptionHandler.java +++ /dev/null @@ -1,39 +0,0 @@ -package edu.harvard.iq.dataverse.api.errorhandlers; - -import java.util.UUID; -import java.util.logging.Level; -import java.util.logging.Logger; -import javax.json.Json; -import javax.servlet.http.HttpServletRequest; -import javax.ws.rs.InternalServerErrorException; -import javax.ws.rs.core.Context; -import javax.ws.rs.core.Response; -import javax.ws.rs.ext.ExceptionMapper; -import javax.ws.rs.ext.Provider; - -/** - * Produces custom 500 messages for the API. - * @author qqmyers - */ -@Provider -public class InternalServerErrorExceptionHandler implements ExceptionMapper{ - - private static final Logger logger = Logger.getLogger(InternalServerErrorExceptionHandler.class.getName()); - - @Context - HttpServletRequest request; - - @Override - public Response toResponse(InternalServerErrorException ex){ - String incidentId = UUID.randomUUID().toString(); - logger.log(Level.SEVERE, "API internal error " + incidentId +": " + ex.getMessage(), ex); - return Response.status(500) - .entity( Json.createObjectBuilder() - .add("status", "ERROR") - .add("code", 500) - .add("message", "Internal server error. More details available at the server logs.") - .add("incidentId", incidentId) - .build()) - .type("application/json").build(); - } -} diff --git a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/NotAllowedExceptionHandler.java b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/NotAllowedExceptionHandler.java deleted file mode 100644 index 5df16c9596d..00000000000 --- a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/NotAllowedExceptionHandler.java +++ /dev/null @@ -1,31 +0,0 @@ -package edu.harvard.iq.dataverse.api.errorhandlers; - -import javax.json.Json; -import javax.servlet.http.HttpServletRequest; -import javax.ws.rs.NotAllowedException; -import javax.ws.rs.core.Context; -import javax.ws.rs.core.Response; -import javax.ws.rs.ext.ExceptionMapper; -import javax.ws.rs.ext.Provider; - -@Provider -public class NotAllowedExceptionHandler implements ExceptionMapper{ - - @Context - HttpServletRequest request; - - @Override - public Response toResponse(NotAllowedException ex){ - String uri = request.getRequestURI(); - return Response.status(405) - .entity( Json.createObjectBuilder() - .add("status", "ERROR") - .add("code", 405) - .add("message", "'" + uri + "' endpoint does not support method '"+request.getMethod()+"'. Consult our API guide at http://guides.dataverse.org.") - .build()) - .type("application/json").build(); - - - } - -} diff --git a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/NotFoundExceptionHandler.java b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/NotFoundExceptionHandler.java deleted file mode 100644 index 51c4f343f85..00000000000 --- a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/NotFoundExceptionHandler.java +++ /dev/null @@ -1,43 +0,0 @@ -package edu.harvard.iq.dataverse.api.errorhandlers; - -import javax.json.Json; -import javax.servlet.http.HttpServletRequest; -import javax.ws.rs.NotFoundException; -import javax.ws.rs.core.Context; -import javax.ws.rs.core.Response; -import javax.ws.rs.ext.ExceptionMapper; -import javax.ws.rs.ext.Provider; - -/** - * Produces custom 404 messages for the API. - * @author michael - */ -@Provider -public class NotFoundExceptionHandler implements ExceptionMapper{ - - @Context - HttpServletRequest request; - - @Override - public Response toResponse(NotFoundException ex){ - String uri = request.getRequestURI(); - String exMessage = ex.getMessage(); - String outputMessage; - if (exMessage != null && exMessage.toLowerCase().startsWith("datafile")) { - outputMessage = exMessage; - } else { - outputMessage = "endpoint does not exist on this server. Please check your code for typos, or consult our API guide at http://guides.dataverse.org."; - } - return Response.status(404) - .entity( Json.createObjectBuilder() - .add("status", "ERROR") - .add("code", 404) - .add("message", "'" + uri + "' " + outputMessage) - .build()) - .type("application/json").build(); - - - } - -} - diff --git a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/RedirectionExceptionHandler.java b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/RedirectionExceptionHandler.java deleted file mode 100644 index 21e677180be..00000000000 --- a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/RedirectionExceptionHandler.java +++ /dev/null @@ -1,32 +0,0 @@ -/* - * To change this license header, choose License Headers in Project Properties. - * To change this template file, choose Tools | Templates - * and open the template in the editor. - */ -package edu.harvard.iq.dataverse.api.errorhandlers; - -import edu.harvard.iq.dataverse.util.BundleUtil; -import javax.json.Json; -import javax.servlet.http.HttpServletRequest; -import javax.ws.rs.RedirectionException; -import javax.ws.rs.core.Context; -import javax.ws.rs.core.Response; -import javax.ws.rs.ext.ExceptionMapper; -import javax.ws.rs.ext.Provider; - -/** - * - * @author qqmyers - */ -@Provider -public class RedirectionExceptionHandler implements ExceptionMapper { - - @Context - HttpServletRequest request; - - @Override - public Response toResponse(RedirectionException ex) { - return ex.getResponse(); - } - -} diff --git a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ServiceUnavailableExceptionHandler.java b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ServiceUnavailableExceptionHandler.java deleted file mode 100644 index ebed03a13da..00000000000 --- a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ServiceUnavailableExceptionHandler.java +++ /dev/null @@ -1,43 +0,0 @@ -package edu.harvard.iq.dataverse.api.errorhandlers; - -import javax.json.Json; -import javax.servlet.http.HttpServletRequest; -import javax.ws.rs.ServiceUnavailableException; -import javax.ws.rs.core.Context; -import javax.ws.rs.core.Response; -import javax.ws.rs.ext.ExceptionMapper; -import javax.ws.rs.ext.Provider; - -/** - * Produces custom 503 messages for the API. - * @author michael - */ -@Provider -public class ServiceUnavailableExceptionHandler implements ExceptionMapper{ - - @Context - HttpServletRequest request; - - @Override - public Response toResponse(ServiceUnavailableException ex){ - String uri = request.getRequestURI(); - String exMessage = ex.getMessage(); - String outputMessage; - if (exMessage != null && exMessage.toLowerCase().startsWith("datafile")) { - outputMessage = exMessage; - } else { - outputMessage = "Requested service or method not available on the requested object"; - } - return Response.status(503) - .entity( Json.createObjectBuilder() - .add("status", "ERROR") - .add("code", 503) - .add("message", "'" + uri + "' " + outputMessage) - .build()) - .type("application/json").build(); - - - } - -} - From e86a8a40155ae751e78563084e59c686308001f3 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 28 Jul 2020 12:55:16 +0200 Subject: [PATCH 02/13] Refactor ThrowableHandler to provide a common set of logging and response formating. --- .../api/errorhandlers/ThrowableHandler.java | 87 ++++++++++++++++++- 1 file changed, 85 insertions(+), 2 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandler.java b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandler.java index 8514d099787..7b6b99daa4e 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandler.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandler.java @@ -1,6 +1,5 @@ package edu.harvard.iq.dataverse.api.errorhandlers; -import javax.annotation.Priority; import javax.json.Json; import javax.servlet.http.HttpServletRequest; import javax.ws.rs.core.Context; @@ -8,9 +7,12 @@ import javax.ws.rs.core.Response; import javax.ws.rs.ext.ExceptionMapper; import javax.ws.rs.ext.Provider; +import java.util.Map; import java.util.UUID; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.stream.Collectors; +import java.util.stream.Stream; /** * Produces a generic 500 message for the API, being a fallback handler for not specially treated exceptions. @@ -41,7 +43,88 @@ public Response toResponse(Throwable ex){ .type(MediaType.APPLICATION_JSON_TYPE).build(); } - private String getOriginalURL(HttpServletRequest req) { + /** + * Some HTTP return codes should be logged at higher levels than FINEST (which is for debugging). + * Match the code and a log level here. + */ + static Map levels = Stream.of(new Object[][] { + {500, Level.SEVERE} + }).collect(Collectors.toMap(data -> (Integer) data[0], data -> (Level) data[1])); + + /** + * Create a useful log message with parseable metadata and log it. + * Log level of message is determined by the levels map and defaults to FINEST (=debugging). + * Log messages will only be logged if the level is enabled for the logger. + * @param header A useful, human readable error message. + * @param requestMethod The HTTP method that triggered the error + * @param requestUrl The HTTP request URL that triggered the error + * @param httpStatus The response HTTP status code + * @param ex The real exception that has been thrown + * @return A random incident id used in the log message, may be forwarded to the client response message. + */ + static String handleLogging(String header, String requestMethod, String requestUrl, int httpStatus, Throwable ex) { + String incidentId = UUID.randomUUID().toString(); + String message = "error="+ex.getClass().getSimpleName()+ + ";incident="+incidentId+ + ";method="+requestMethod+ + ";url="+requestUrl+ + ";status="+httpStatus+"|\n"+ + header+"|"; + + Level level = levels.getOrDefault(httpStatus, Level.FINEST); + if (logger.isLoggable(level)) + logger.log(level, message, ex); + + return incidentId; + } + + /** + * Create a new (error) response with a human readable yet machine actionable JSON error message. + * @param status HTTP return code + * @param requestMethod The HTTP method used in the original request + * @param requestUrl The HTTP URL used in the original request + * @param errorType Ideally a exception class name, but maybe free text + * @param message A human readable message + * @param incidentId A random incident id. Should be the same as the logged id for easier error analysis + * @return A response ready to be send to the client + */ + static Response createErrorResponse(int status, String requestMethod, String requestUrl, String errorType, String message, String incidentId) { + return createErrorResponse(Response.status(status).build(), requestMethod, requestUrl, errorType, message, incidentId); + } + + /** + * Create an error response with a human readable yet machine actionable JSON error message, based on an existing + * response (necessary for things like a redirect). + * @param fromResponse The original response to enhance + * @param requestMethod The HTTP method used in the original request + * @param requestUrl The HTTP URL used in the original request + * @param errorType Ideally a exception class name, but maybe free text + * @param message A human readable message + * @param incidentId A random incident id. Should be the same as the logged id for easier error analysis + * @return A response ready to be send to the client + */ + static Response createErrorResponse(Response fromResponse, String requestMethod, String requestUrl, String errorType, String message, String incidentId) { + return Response.fromResponse(fromResponse) + .entity(Json.createObjectBuilder() + .add("status", "ERROR") + .add("code", fromResponse.getStatus()) + .add("method", requestMethod) + .add("url", requestUrl) + .add("errorType", errorType) + .add("message", message) + .add("incidentId", incidentId) + .build() + ) + .type(MediaType.APPLICATION_JSON_TYPE) + .build(); + } + + /** + * Build a complete request URL for logging purposes + * @param req The request + * @return The requests URL sent by the client + */ + static String getOriginalURL(HttpServletRequest req) { // Rebuild the original request URL: http://stackoverflow.com/a/5212336/356408 String scheme = req.getScheme(); // http String serverName = req.getServerName(); // hostname.com From b39d372e6ba34c97123e36c12815987a5072a5d7 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 28 Jul 2020 12:59:35 +0200 Subject: [PATCH 03/13] Dogfooding the new methods to the ThrowableHandler toResponse(). #7134 --- .../api/errorhandlers/ThrowableHandler.java | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandler.java b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandler.java index 7b6b99daa4e..c207e64bc83 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandler.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandler.java @@ -21,26 +21,28 @@ public class ThrowableHandler implements ExceptionMapper{ private static final Logger logger = Logger.getLogger(ThrowableHandler.class.getName()); + static final String INTERNAL_SERVER_ERROR_MESSAGE = "Internal server error. More details available at the server logs."; @Context HttpServletRequest request; @Override public Response toResponse(Throwable ex){ - String incidentId = UUID.randomUUID().toString(); - logger.log(Level.SEVERE, "Uncaught REST API exception:\n"+ - " Incident: " + incidentId +"\n"+ - " URL: "+getOriginalURL(request)+"\n"+ - " Method: "+request.getMethod(), ex); - return Response.status(Response.Status.INTERNAL_SERVER_ERROR) - .entity( Json.createObjectBuilder() - .add("status", "ERROR") - .add("code", Response.Status.INTERNAL_SERVER_ERROR.getStatusCode()) - .add("type", ex.getClass().getSimpleName()) - .add("message", "Internal server error. More details available at the server logs.") - .add("incidentId", incidentId) - .build()) - .type(MediaType.APPLICATION_JSON_TYPE).build(); + String requestMethod = request.getMethod(); + String requestUrl = ThrowableHandler.getOriginalURL(request); + + String incidentId = ThrowableHandler.handleLogging(INTERNAL_SERVER_ERROR_MESSAGE, + requestMethod, + requestUrl, + Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), + ex); + + return ThrowableHandler.createErrorResponse(Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), + requestMethod, + requestUrl, + ex.getClass().getSimpleName(), + INTERNAL_SERVER_ERROR_MESSAGE, + incidentId); } /** From 889458e029f721893ee89a10e6c3bdf7030aed8e Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 28 Jul 2020 13:02:49 +0200 Subject: [PATCH 04/13] Finally implement the WebApplicationExceptionHandler, reducing duplicated code and solve problems like #7134. --- .../WebApplicationExceptionHandler.java | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/WebApplicationExceptionHandler.java diff --git a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/WebApplicationExceptionHandler.java b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/WebApplicationExceptionHandler.java new file mode 100644 index 00000000000..924450946c5 --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/WebApplicationExceptionHandler.java @@ -0,0 +1,108 @@ +/* + * To change this license header, choose License Headers in Project Properties. + * To change this template file, choose Tools | Templates + * and open the template in the editor. + */ +package edu.harvard.iq.dataverse.api.errorhandlers; + +import edu.harvard.iq.dataverse.util.BundleUtil; + +import javax.servlet.http.HttpServletRequest; +import javax.ws.rs.WebApplicationException; +import javax.ws.rs.core.Context; +import javax.ws.rs.core.Response; +import javax.ws.rs.ext.ExceptionMapper; +import javax.ws.rs.ext.Provider; +import java.util.logging.Logger; + +/** + * Catches all types of web application exceptions like NotFoundException, etc etc and handles them properly. + */ +@Provider +public class WebApplicationExceptionHandler implements ExceptionMapper { + + static final Logger logger = Logger.getLogger(WebApplicationExceptionHandler.class.getSimpleName()); + + @Context + HttpServletRequest request; + + @Override + public Response toResponse(WebApplicationException ex) { + + String requestMethod = request.getMethod(); + String requestUrl = ThrowableHandler.getOriginalURL(request); + String message = createMessage(ex); + + String incidentId = ThrowableHandler.handleLogging(message, + requestMethod, + requestUrl, + ex.getResponse().getStatus(), + ex); + + return ThrowableHandler.createErrorResponse(ex.getResponse(), + requestMethod, + requestUrl, + ex.getClass().getSimpleName(), + message, + incidentId); + } + + /** + * Analyse the exception and generate a human readable (and helpful) error message. + * @param ex The exception thrown + * @return A human readable error message. + */ + String createMessage(WebApplicationException ex) { + + String message = ""; + String exMessage = ex.getMessage(); + + // See also https://en.wikipedia.org/wiki/List_of_HTTP_status_codes for a list of status codes. + switch (ex.getResponse().getStatus()) { + // Redirects (permanent & temporary) + case 302: + case 307: + message = ex.getResponse().getLocation().toString(); + break; + // BadRequest + case 400: + if (exMessage != null && exMessage.toLowerCase().startsWith("tabular data required")) { + message = BundleUtil.getStringFromBundle("access.api.exception.metadata.not.available.for.nontabular.file"); + } else { + message = "Bad Request. The API request cannot be completed with the parameters supplied. Please check your code for typos, or consult our API guide at http://guides.dataverse.org."; + } + break; + // Forbidden + case 403: + message = "Not authorized to access this object via this API endpoint. Please check your code for typos, or consult our API guide at http://guides.dataverse.org."; + break; + // NotFound + case 404: + if (exMessage != null && exMessage.toLowerCase().startsWith("datafile")) { + message = exMessage; + } else { + message = "API endpoint does not exist on this server. Please check your code for typos, or consult our API guide at http://guides.dataverse.org."; + } + break; + // MethodNotAllowed + case 405: + message = "API endpoint does not support this method. Consult our API guide at http://guides.dataverse.org."; + break; + // InternalServerError + case 500: + message = "Internal server error. More details available at the server logs. Please contact your dataverse administrator."; + break; + // ServiceUnavailable + case 503: + if (exMessage != null && exMessage.toLowerCase().startsWith("datafile")) { + message = exMessage; + } else { + message = "Requested service or method not available on the requested object"; + } + break; + } + + return message; + } + +} From 4accdd3af40fc9cc0b826d032b3e35abce635068 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 28 Jul 2020 13:08:36 +0200 Subject: [PATCH 05/13] Further reduce boilerplate and duplicated code by removing handlers for ArrayOutOfBoundsException, NullPointerException and ServeletException perfectly replaced by ThrowableHandler. #7134 --- .../ArrayOutOfBoundsExceptionHandler.java | 38 ------------------- .../NullPointerExceptionHandler.java | 38 ------------------- .../ServeletExceptionHandler.java | 38 ------------------- .../api/errorhandlers/ThrowableHandler.java | 3 ++ 4 files changed, 3 insertions(+), 114 deletions(-) delete mode 100644 src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ArrayOutOfBoundsExceptionHandler.java delete mode 100644 src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/NullPointerExceptionHandler.java delete mode 100644 src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ServeletExceptionHandler.java diff --git a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ArrayOutOfBoundsExceptionHandler.java b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ArrayOutOfBoundsExceptionHandler.java deleted file mode 100644 index 112f50eb3ed..00000000000 --- a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ArrayOutOfBoundsExceptionHandler.java +++ /dev/null @@ -1,38 +0,0 @@ -package edu.harvard.iq.dataverse.api.errorhandlers; - -import java.util.UUID; -import java.util.logging.Level; -import java.util.logging.Logger; -import javax.json.Json; -import javax.servlet.http.HttpServletRequest; -import javax.ws.rs.core.Context; -import javax.ws.rs.core.Response; -import javax.ws.rs.ext.ExceptionMapper; -import javax.ws.rs.ext.Provider; - -/** - * Produces custom 500 messages for the API. - * @author michael - */ -@Provider -public class ArrayOutOfBoundsExceptionHandler implements ExceptionMapper{ - - private static final Logger logger = Logger.getLogger(ServeletExceptionHandler.class.getName()); - - @Context - HttpServletRequest request; - - @Override - public Response toResponse(java.lang.ArrayIndexOutOfBoundsException ex){ - String incidentId = UUID.randomUUID().toString(); - logger.log(Level.SEVERE, "API internal error " + incidentId +": ArrayOutOfBounds:" + ex.getMessage(), ex); - return Response.status(500) - .entity( Json.createObjectBuilder() - .add("status", "ERROR") - .add("code", 500) - .add("message", "Internal server error. More details available at the server logs.") - .add("incidentId", incidentId) - .build()) - .type("application/json").build(); - } -} diff --git a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/NullPointerExceptionHandler.java b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/NullPointerExceptionHandler.java deleted file mode 100644 index 1e9c2a28690..00000000000 --- a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/NullPointerExceptionHandler.java +++ /dev/null @@ -1,38 +0,0 @@ -package edu.harvard.iq.dataverse.api.errorhandlers; - -import java.util.UUID; -import java.util.logging.Level; -import java.util.logging.Logger; -import javax.json.Json; -import javax.servlet.http.HttpServletRequest; -import javax.ws.rs.core.Context; -import javax.ws.rs.core.Response; -import javax.ws.rs.ext.ExceptionMapper; -import javax.ws.rs.ext.Provider; - -/** - * Produces custom 500 messages for the API. - * @author michael - */ -@Provider -public class NullPointerExceptionHandler implements ExceptionMapper{ - - private static final Logger logger = Logger.getLogger(ServeletExceptionHandler.class.getName()); - - @Context - HttpServletRequest request; - - @Override - public Response toResponse(java.lang.NullPointerException ex){ - String incidentId = UUID.randomUUID().toString(); - logger.log(Level.SEVERE, "API internal error " + incidentId +": Null Pointer", ex); - return Response.status(500) - .entity( Json.createObjectBuilder() - .add("status", "ERROR") - .add("code", 500) - .add("message", "Internal server error. More details available at the server logs.") - .add("incidentId", incidentId) - .build()) - .type("application/json").build(); - } -} diff --git a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ServeletExceptionHandler.java b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ServeletExceptionHandler.java deleted file mode 100644 index fa6bff57c03..00000000000 --- a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ServeletExceptionHandler.java +++ /dev/null @@ -1,38 +0,0 @@ -package edu.harvard.iq.dataverse.api.errorhandlers; - -import java.util.UUID; -import java.util.logging.Level; -import java.util.logging.Logger; -import javax.json.Json; -import javax.servlet.http.HttpServletRequest; -import javax.ws.rs.core.Context; -import javax.ws.rs.core.Response; -import javax.ws.rs.ext.ExceptionMapper; -import javax.ws.rs.ext.Provider; - -/** - * Produces custom 500 messages for the API. - * @author michael - */ -@Provider -public class ServeletExceptionHandler implements ExceptionMapper{ - - private static final Logger logger = Logger.getLogger(ServeletExceptionHandler.class.getName()); - - @Context - HttpServletRequest request; - - @Override - public Response toResponse(javax.servlet.ServletException ex){ - String incidentId = UUID.randomUUID().toString(); - logger.log(Level.SEVERE, "API internal error " + incidentId +": " + ex.getMessage(), ex); - return Response.status(500) - .entity( Json.createObjectBuilder() - .add("status", "ERROR") - .add("code", 500) - .add("message", "Internal server error. More details available at the server logs.") - .add("incidentId", incidentId) - .build()) - .type("application/json").build(); - } -} diff --git a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandler.java b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandler.java index c207e64bc83..9926a244f4d 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandler.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandler.java @@ -16,6 +16,9 @@ /** * Produces a generic 500 message for the API, being a fallback handler for not specially treated exceptions. + * + * This catches bad exceptions like ArrayOutOfBoundsExceptions, NullPointerExceptions and ServeletExceptions, + * which had formerly specialised handlers, generating a generic error message. (This is now handled here.) */ @Provider public class ThrowableHandler implements ExceptionMapper{ From cbabb7b135754a14b81b05861be3575d0e3e8c2e Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 28 Jul 2020 13:32:33 +0200 Subject: [PATCH 06/13] Mask unblock-key query parameter in logs and responses to avoid cleartext storage of secrets. #7134 --- .../iq/dataverse/api/ApiBlockingFilter.java | 2 +- .../api/errorhandlers/ThrowableHandler.java | 6 +++- .../errorhandlers/ThrowableHandlerTest.java | 32 +++++++++++++++++++ 3 files changed, 38 insertions(+), 2 deletions(-) create mode 100644 src/test/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandlerTest.java diff --git a/src/main/java/edu/harvard/iq/dataverse/api/ApiBlockingFilter.java b/src/main/java/edu/harvard/iq/dataverse/api/ApiBlockingFilter.java index 761a6c8ef77..a0f584736f1 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/ApiBlockingFilter.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/ApiBlockingFilter.java @@ -25,7 +25,7 @@ * @author michael */ public class ApiBlockingFilter implements javax.servlet.Filter { - private static final String UNBLOCK_KEY_QUERYPARAM = "unblock-key"; + public static final String UNBLOCK_KEY_QUERYPARAM = "unblock-key"; interface BlockPolicy { public void doBlock(ServletRequest sr, ServletResponse sr1, FilterChain fc) throws IOException, ServletException; diff --git a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandler.java b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandler.java index 9926a244f4d..20e3753d22d 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandler.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandler.java @@ -1,5 +1,7 @@ package edu.harvard.iq.dataverse.api.errorhandlers; +import edu.harvard.iq.dataverse.api.ApiBlockingFilter; + import javax.json.Json; import javax.servlet.http.HttpServletRequest; import javax.ws.rs.core.Context; @@ -150,7 +152,9 @@ static String getOriginalURL(HttpServletRequest req) { url.append(pathInfo); } if (queryString != null) { - url.append("?").append(queryString); + // filter for unblock-key parameter and mask the key + String maskedQueryString = queryString.replaceAll(ApiBlockingFilter.UNBLOCK_KEY_QUERYPARAM+"=.+?\\b", ApiBlockingFilter.UNBLOCK_KEY_QUERYPARAM+"=****"); + url.append("?").append(maskedQueryString); } return url.toString(); diff --git a/src/test/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandlerTest.java b/src/test/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandlerTest.java new file mode 100644 index 00000000000..50b92ba3ce7 --- /dev/null +++ b/src/test/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandlerTest.java @@ -0,0 +1,32 @@ +package edu.harvard.iq.dataverse.api.errorhandlers; + +import edu.harvard.iq.dataverse.api.ApiBlockingFilter; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EmptySource; +import org.junit.jupiter.params.provider.NullSource; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.Mockito; + +import javax.servlet.http.HttpServletRequest; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +class ThrowableHandlerTest { + + @ParameterizedTest + @NullSource + @EmptySource + @ValueSource(strings = { + "test", + ApiBlockingFilter.UNBLOCK_KEY_QUERYPARAM+"=supersecret", + ApiBlockingFilter.UNBLOCK_KEY_QUERYPARAM+"=supersecret&hello=1234", + "hello=1234&"+ApiBlockingFilter.UNBLOCK_KEY_QUERYPARAM+"=supersecret", + "hello=1234&"+ApiBlockingFilter.UNBLOCK_KEY_QUERYPARAM+"=supersecret&test=1234"}) + void testMaskingOriginalURL(String query) { + HttpServletRequest test = Mockito.mock(HttpServletRequest.class); + when(test.getQueryString()).thenReturn(query); + assertFalse(ThrowableHandler.getOriginalURL(test).contains("supersecret")); + } +} \ No newline at end of file From 0db042be58932f7bfa822503d0f2e43b985a609f Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 28 Jul 2020 14:02:23 +0200 Subject: [PATCH 07/13] Add default message from Jersey WebApplicationException message to JSON responses. #7134 --- .../api/errorhandlers/WebApplicationExceptionHandler.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/WebApplicationExceptionHandler.java b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/WebApplicationExceptionHandler.java index 924450946c5..83b6aaaa33c 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/WebApplicationExceptionHandler.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/WebApplicationExceptionHandler.java @@ -100,6 +100,9 @@ String createMessage(WebApplicationException ex) { message = "Requested service or method not available on the requested object"; } break; + default: + message = ex.getMessage(); + break; } return message; From 41adbd9b5ba33b62a89ba9f2def5c3f2ff7b4fc5 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 29 Jul 2020 14:49:18 +0200 Subject: [PATCH 08/13] Create a new unified JSON responses and logging infrastructure for the API. #7134 Only supports error responses for now. --- .../api/util/JSONResponseBuilder.java | 267 ++++++++++++++++++ .../JSONResponseBuilderTest.java} | 8 +- 2 files changed, 271 insertions(+), 4 deletions(-) create mode 100644 src/main/java/edu/harvard/iq/dataverse/api/util/JSONResponseBuilder.java rename src/test/java/edu/harvard/iq/dataverse/api/{errorhandlers/ThrowableHandlerTest.java => util/JSONResponseBuilderTest.java} (81%) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/util/JSONResponseBuilder.java b/src/main/java/edu/harvard/iq/dataverse/api/util/JSONResponseBuilder.java new file mode 100644 index 00000000000..eea46eada2e --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/api/util/JSONResponseBuilder.java @@ -0,0 +1,267 @@ +package edu.harvard.iq.dataverse.api.util; + +import edu.harvard.iq.dataverse.api.ApiBlockingFilter; + +import javax.json.Json; +import javax.json.JsonObjectBuilder; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; +import java.io.IOException; +import java.util.Optional; +import java.util.UUID; +import java.util.logging.Level; +import java.util.logging.Logger; + +public class JSONResponseBuilder { + + private JsonObjectBuilder entityBuilder = Json.createObjectBuilder(); + private Response.ResponseBuilder jerseyResponseBuilder; + private boolean alreadyLogged = false; + + private JSONResponseBuilder() {} + + /** + * Create an error response from an numeric error code (should be >= 400) + * @param httpStatusCode Numerical HTTP status code + * @return A builder with a basic JSON body + * @throws IllegalArgumentException if fromResponse isn't an error response + */ + public static JSONResponseBuilder error(int httpStatusCode) { + if (httpStatusCode < 400) { + throw new IllegalArgumentException("A status code < 400 cannot be an error indicating response."); + } + + JSONResponseBuilder b = new JSONResponseBuilder(); + b.jerseyResponseBuilder = Response.status(httpStatusCode); + b.entityBuilder.add("status", "ERROR"); + b.entityBuilder.add("code", httpStatusCode); + // include default message if present + getDefaultMessage(httpStatusCode).ifPresent(v -> b.entityBuilder.add("message", v)); + + return b; + } + + /** + * Create an error response from a Response.Status. + * @param status A JAX-RS Response.Status object + * @return A builder with a basic JSON body + * @throws IllegalArgumentException if fromResponse isn't an error response + */ + public static JSONResponseBuilder error(Response.Status status) { + JSONResponseBuilder b = error(status.getStatusCode()); + b.jerseyResponseBuilder = Response.status(status); + return b; + } + + /** + * Create an error response from an existing response. + * @param fromResponse An existing JAX-RS Response + * @return A builder with a basic JSON body + * @throws IllegalArgumentException if fromResponse isn't an error response + */ + public static JSONResponseBuilder error(Response fromResponse) { + JSONResponseBuilder b = error(fromResponse.getStatus()); + b.jerseyResponseBuilder = Response.fromResponse(fromResponse); + return b; + } + + /** + * Add a human friendly message to the response + * @param message A human readable message + * @return The enhanced builder + */ + public JSONResponseBuilder message(String message) { + this.entityBuilder.add("message", message); + return this; + } + + /** + * Set an identifier for this (usually included in logs, too). + * @param id A String containing an (ideally unique) identifier + * @return The enhanced builder + */ + public JSONResponseBuilder incidentId(String id) { + this.entityBuilder.add("incidentId", id); + return this; + } + + /** + * Add a UUID random identifier for errors. Will overwrite existing. + * @return The enhanced builder + */ + public JSONResponseBuilder randomIncidentId() { + this.entityBuilder.add("incidentId", UUID.randomUUID().toString()); + return this; + } + + /** + * Add more details about the original request: what URL was used, + * what HTTP method involved? + * @param request The original request (usually provided from a context) + * @return The enhanced builder + */ + public JSONResponseBuilder request(HttpServletRequest request) { + this.entityBuilder.add("requestUrl", getOriginalURL(request)); + this.entityBuilder.add("requestMethod", request.getMethod()); + return this; + } + + /** + * Add more details about the original request: what content type was sent? + * @param request The original request (usually provided from a context) + * @return The enhanced builder + */ + public JSONResponseBuilder requestContentType(HttpServletRequest request) { + this.entityBuilder.add("requestContentType", request.getContentType()); + return this; + } + + /** + * Add more details about internal errors (exceptions) to the response. + * Will include a detail about the cause if exception has one. + * @param ex An exception. + * @return The enhanced builder + */ + public JSONResponseBuilder internalError(Throwable ex) { + this.entityBuilder.add("interalError", ex.getClass().getSimpleName()); + if (ex.getCause() != null) { + this.entityBuilder.add("internalCause", ex.getCause().getClass().getSimpleName()); + } + return this; + } + + /** + * Finish building a Jersey JAX-RS response with JSON message + * @return JAX-RS response including JSON message + */ + public Response build() { + return jerseyResponseBuilder.type(MediaType.APPLICATION_JSON_TYPE) + .entity(this.entityBuilder.build()) + .build(); + } + + /** + * For usage in non-Jersey areas like servlet filters, blocks, etc., + * apply the response to the Servlet provided response object. + * @param response The ServletResponse from the context + * @throws IOException + */ + public void apply(ServletResponse response) throws IOException { + HttpServletResponse httpServletResponse = (HttpServletResponse) response; + apply(httpServletResponse); + } + + /** + * For usage in non-Jersey areas like servlet filters, blocks, etc., + * apply the response to the Servlet provided response object. + * @param response The HttpServletResponse from the context + * @throws IOException + */ + public void apply(HttpServletResponse response) throws IOException { + Response jersey = jerseyResponseBuilder.build(); + response.setStatus(jersey.getStatus()); + response.setContentType(MediaType.APPLICATION_JSON); + response.getWriter().print(entityBuilder.build().toString()); + response.getWriter().flush(); + } + + /** + * Log this JSON response as a useful log message. + * Should be done before calling build(), but after adding any decorations. + * + * The log message will contain a message with the flattened JSON entity: + * { "status": "ERROR", "code": 401 } -> _status=ERROR;_code=401 + * + * Will prevent logging the same response twice. + * + * @param logger Provide a logger instance to write to + * @param level Provide a level at which this should be logged + * @return The unmodified builder. + */ + public JSONResponseBuilder log(Logger logger, Level level) { + return this.log(logger, level, Optional.empty()); + } + + /** + * Log this JSON response as a useful log message. + * Should be done before calling build(), but after adding any decorations. + * + * The log message will contain a message with the flattened JSON entity: + * { "status": "ERROR", "code": 401 } -> _status=ERROR;_code=401 + * + * If an exception is given, it will be folled by a "|" and the exception message + * formatted by the logging system itself. + * + * Will prevent logging the same response twice. + * + * @param logger Provide a logger instance to write to + * @param level Provide a level at which this should be logged + * @param ex An optional exception to be included in the log message. + * @return The unmodified builder. + */ + public JSONResponseBuilder log(Logger logger, Level level, Optional ex) { + if ( ! logger.isLoggable(level) || alreadyLogged ) + return this; + + StringBuilder metadata = new StringBuilder(""); + this.entityBuilder.build() + .forEach((k,v) -> metadata.append("_"+k+"="+v.toString()+";")); + // remove trailing ; + metadata.deleteCharAt(metadata.length()-1); + + if (ex.isPresent()) { + metadata.append("|"); + logger.log(level, metadata.toString(), ex); + } else { + logger.log(level, metadata.toString()); + } + + this.alreadyLogged = true; + return this; + } + + /** + * Build a complete request URL for logging purposes. + * Masks query parameter "unblock-key" if present to avoid leaking secrets. + * @param req The request + * @return The requests URL sent by the client + */ + public static String getOriginalURL(HttpServletRequest req) { + // Rebuild the original request URL: http://stackoverflow.com/a/5212336/356408 + String scheme = req.getScheme(); // http + String serverName = req.getServerName(); // hostname.com + int serverPort = req.getServerPort(); // 80 + String contextPath = req.getContextPath(); // /mywebapp + String servletPath = req.getServletPath(); // /servlet/MyServlet + String pathInfo = req.getPathInfo(); // /a/b;c=123 + String queryString = req.getQueryString(); // d=789 + + // Reconstruct original requesting URL + StringBuilder url = new StringBuilder(); + url.append(scheme).append("://").append(serverName); + if (serverPort != 80 && serverPort != 443) { + url.append(":").append(serverPort); + } + url.append(contextPath).append(servletPath); + if (pathInfo != null) { + url.append(pathInfo); + } + if (queryString != null) { + // filter for unblock-key parameter and mask the key + String maskedQueryString = queryString.replaceAll(ApiBlockingFilter.UNBLOCK_KEY_QUERYPARAM+"=.+?\\b", ApiBlockingFilter.UNBLOCK_KEY_QUERYPARAM+"=****"); + url.append("?").append(maskedQueryString); + } + + return url.toString(); + } + + public static Optional getDefaultMessage(int httpStatusCode) { + switch (httpStatusCode) { + case 500: return Optional.of("Internal server error. More details available at the server logs."); + default: return Optional.empty(); + } + } +} diff --git a/src/test/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandlerTest.java b/src/test/java/edu/harvard/iq/dataverse/api/util/JSONResponseBuilderTest.java similarity index 81% rename from src/test/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandlerTest.java rename to src/test/java/edu/harvard/iq/dataverse/api/util/JSONResponseBuilderTest.java index 50b92ba3ce7..29e0536f73c 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandlerTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/util/JSONResponseBuilderTest.java @@ -1,7 +1,7 @@ -package edu.harvard.iq.dataverse.api.errorhandlers; +package edu.harvard.iq.dataverse.api.util; import edu.harvard.iq.dataverse.api.ApiBlockingFilter; -import org.junit.jupiter.api.Test; +import edu.harvard.iq.dataverse.api.errorhandlers.ThrowableHandler; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.EmptySource; import org.junit.jupiter.params.provider.NullSource; @@ -13,7 +13,7 @@ import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.*; -class ThrowableHandlerTest { +class JSONResponseBuilderTest { @ParameterizedTest @NullSource @@ -27,6 +27,6 @@ class ThrowableHandlerTest { void testMaskingOriginalURL(String query) { HttpServletRequest test = Mockito.mock(HttpServletRequest.class); when(test.getQueryString()).thenReturn(query); - assertFalse(ThrowableHandler.getOriginalURL(test).contains("supersecret")); + assertFalse(JSONResponseBuilder.getOriginalURL(test).contains("supersecret")); } } \ No newline at end of file From 5385a0d97c513ac72cc45b8c17ec9e08028eb89d Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 29 Jul 2020 15:39:59 +0200 Subject: [PATCH 09/13] Switch ThrowableHandler and WebApplicationExceptionHandler to the new JSONResponseBuilder for crafting responses and logging. #7134 --- .../api/errorhandlers/ThrowableHandler.java | 137 +----------------- .../WebApplicationExceptionHandler.java | 85 +++++------ 2 files changed, 47 insertions(+), 175 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandler.java b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandler.java index 20e3753d22d..74330c04c4d 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandler.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandler.java @@ -1,15 +1,14 @@ package edu.harvard.iq.dataverse.api.errorhandlers; -import edu.harvard.iq.dataverse.api.ApiBlockingFilter; +import edu.harvard.iq.dataverse.api.util.JSONResponseBuilder; -import javax.json.Json; import javax.servlet.http.HttpServletRequest; import javax.ws.rs.core.Context; -import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import javax.ws.rs.ext.ExceptionMapper; import javax.ws.rs.ext.Provider; import java.util.Map; +import java.util.Optional; import java.util.UUID; import java.util.logging.Level; import java.util.logging.Logger; @@ -26,137 +25,17 @@ public class ThrowableHandler implements ExceptionMapper{ private static final Logger logger = Logger.getLogger(ThrowableHandler.class.getName()); - static final String INTERNAL_SERVER_ERROR_MESSAGE = "Internal server error. More details available at the server logs."; @Context HttpServletRequest request; @Override public Response toResponse(Throwable ex){ - String requestMethod = request.getMethod(); - String requestUrl = ThrowableHandler.getOriginalURL(request); - - String incidentId = ThrowableHandler.handleLogging(INTERNAL_SERVER_ERROR_MESSAGE, - requestMethod, - requestUrl, - Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), - ex); - - return ThrowableHandler.createErrorResponse(Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), - requestMethod, - requestUrl, - ex.getClass().getSimpleName(), - INTERNAL_SERVER_ERROR_MESSAGE, - incidentId); - } - - /** - * Some HTTP return codes should be logged at higher levels than FINEST (which is for debugging). - * Match the code and a log level here. - */ - static Map levels = Stream.of(new Object[][] { - {500, Level.SEVERE} - }).collect(Collectors.toMap(data -> (Integer) data[0], data -> (Level) data[1])); - - /** - * Create a useful log message with parseable metadata and log it. - * Log level of message is determined by the levels map and defaults to FINEST (=debugging). - * Log messages will only be logged if the level is enabled for the logger. - * @param header A useful, human readable error message. - * @param requestMethod The HTTP method that triggered the error - * @param requestUrl The HTTP request URL that triggered the error - * @param httpStatus The response HTTP status code - * @param ex The real exception that has been thrown - * @return A random incident id used in the log message, may be forwarded to the client response message. - */ - static String handleLogging(String header, String requestMethod, String requestUrl, int httpStatus, Throwable ex) { - String incidentId = UUID.randomUUID().toString(); - String message = "error="+ex.getClass().getSimpleName()+ - ";incident="+incidentId+ - ";method="+requestMethod+ - ";url="+requestUrl+ - ";status="+httpStatus+"|\n"+ - header+"|"; - - Level level = levels.getOrDefault(httpStatus, Level.FINEST); - if (logger.isLoggable(level)) - logger.log(level, message, ex); - - return incidentId; - } - - /** - * Create a new (error) response with a human readable yet machine actionable JSON error message. - * @param status HTTP return code - * @param requestMethod The HTTP method used in the original request - * @param requestUrl The HTTP URL used in the original request - * @param errorType Ideally a exception class name, but maybe free text - * @param message A human readable message - * @param incidentId A random incident id. Should be the same as the logged id for easier error analysis - * @return A response ready to be send to the client - */ - static Response createErrorResponse(int status, String requestMethod, String requestUrl, String errorType, String message, String incidentId) { - return createErrorResponse(Response.status(status).build(), requestMethod, requestUrl, errorType, message, incidentId); - } - - /** - * Create an error response with a human readable yet machine actionable JSON error message, based on an existing - * response (necessary for things like a redirect). - * @param fromResponse The original response to enhance - * @param requestMethod The HTTP method used in the original request - * @param requestUrl The HTTP URL used in the original request - * @param errorType Ideally a exception class name, but maybe free text - * @param message A human readable message - * @param incidentId A random incident id. Should be the same as the logged id for easier error analysis - * @return A response ready to be send to the client - */ - static Response createErrorResponse(Response fromResponse, String requestMethod, String requestUrl, String errorType, String message, String incidentId) { - return Response.fromResponse(fromResponse) - .entity(Json.createObjectBuilder() - .add("status", "ERROR") - .add("code", fromResponse.getStatus()) - .add("method", requestMethod) - .add("url", requestUrl) - .add("errorType", errorType) - .add("message", message) - .add("incidentId", incidentId) - .build() - ) - .type(MediaType.APPLICATION_JSON_TYPE) - .build(); - } - - /** - * Build a complete request URL for logging purposes - * @param req The request - * @return The requests URL sent by the client - */ - static String getOriginalURL(HttpServletRequest req) { - // Rebuild the original request URL: http://stackoverflow.com/a/5212336/356408 - String scheme = req.getScheme(); // http - String serverName = req.getServerName(); // hostname.com - int serverPort = req.getServerPort(); // 80 - String contextPath = req.getContextPath(); // /mywebapp - String servletPath = req.getServletPath(); // /servlet/MyServlet - String pathInfo = req.getPathInfo(); // /a/b;c=123 - String queryString = req.getQueryString(); // d=789 - - // Reconstruct original requesting URL - StringBuilder url = new StringBuilder(); - url.append(scheme).append("://").append(serverName); - if (serverPort != 80 && serverPort != 443) { - url.append(":").append(serverPort); - } - url.append(contextPath).append(servletPath); - if (pathInfo != null) { - url.append(pathInfo); - } - if (queryString != null) { - // filter for unblock-key parameter and mask the key - String maskedQueryString = queryString.replaceAll(ApiBlockingFilter.UNBLOCK_KEY_QUERYPARAM+"=.+?\\b", ApiBlockingFilter.UNBLOCK_KEY_QUERYPARAM+"=****"); - url.append("?").append(maskedQueryString); - } - - return url.toString(); + return JSONResponseBuilder.error(Response.Status.INTERNAL_SERVER_ERROR) + .randomIncidentId() + .internalError(ex) + .request(request) + .log(logger, Level.SEVERE, Optional.of(ex)) + .build(); } } diff --git a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/WebApplicationExceptionHandler.java b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/WebApplicationExceptionHandler.java index 83b6aaaa33c..4cb88bf2c7a 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/WebApplicationExceptionHandler.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/WebApplicationExceptionHandler.java @@ -5,14 +5,18 @@ */ package edu.harvard.iq.dataverse.api.errorhandlers; +import edu.harvard.iq.dataverse.api.util.JSONResponseBuilder; import edu.harvard.iq.dataverse.util.BundleUtil; import javax.servlet.http.HttpServletRequest; +import javax.swing.text.html.Option; import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.Context; import javax.ws.rs.core.Response; import javax.ws.rs.ext.ExceptionMapper; import javax.ws.rs.ext.Provider; +import java.util.Optional; +import java.util.logging.Level; import java.util.logging.Logger; /** @@ -29,83 +33,72 @@ public class WebApplicationExceptionHandler implements ExceptionMapper Date: Mon, 10 Aug 2020 13:06:48 +0200 Subject: [PATCH 10/13] Rename JSONResponseBuilder into more CamelCased JsonResponseBuilder. #7134 --- .../api/errorhandlers/ThrowableHandler.java | 8 ++--- .../WebApplicationExceptionHandler.java | 5 ++- ...eBuilder.java => JsonResponseBuilder.java} | 32 +++++++++---------- ...Test.java => JsonResponseBuilderTest.java} | 5 ++- 4 files changed, 22 insertions(+), 28 deletions(-) rename src/main/java/edu/harvard/iq/dataverse/api/util/{JSONResponseBuilder.java => JsonResponseBuilder.java} (91%) rename src/test/java/edu/harvard/iq/dataverse/api/util/{JSONResponseBuilderTest.java => JsonResponseBuilderTest.java} (87%) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandler.java b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandler.java index 74330c04c4d..7b7f1f08fad 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandler.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ThrowableHandler.java @@ -1,19 +1,15 @@ package edu.harvard.iq.dataverse.api.errorhandlers; -import edu.harvard.iq.dataverse.api.util.JSONResponseBuilder; +import edu.harvard.iq.dataverse.api.util.JsonResponseBuilder; import javax.servlet.http.HttpServletRequest; import javax.ws.rs.core.Context; import javax.ws.rs.core.Response; import javax.ws.rs.ext.ExceptionMapper; import javax.ws.rs.ext.Provider; -import java.util.Map; import java.util.Optional; -import java.util.UUID; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.stream.Collectors; -import java.util.stream.Stream; /** * Produces a generic 500 message for the API, being a fallback handler for not specially treated exceptions. @@ -31,7 +27,7 @@ public class ThrowableHandler implements ExceptionMapper{ @Override public Response toResponse(Throwable ex){ - return JSONResponseBuilder.error(Response.Status.INTERNAL_SERVER_ERROR) + return JsonResponseBuilder.error(Response.Status.INTERNAL_SERVER_ERROR) .randomIncidentId() .internalError(ex) .request(request) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/WebApplicationExceptionHandler.java b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/WebApplicationExceptionHandler.java index 4cb88bf2c7a..b6229e58192 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/WebApplicationExceptionHandler.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/WebApplicationExceptionHandler.java @@ -5,11 +5,10 @@ */ package edu.harvard.iq.dataverse.api.errorhandlers; -import edu.harvard.iq.dataverse.api.util.JSONResponseBuilder; +import edu.harvard.iq.dataverse.api.util.JsonResponseBuilder; import edu.harvard.iq.dataverse.util.BundleUtil; import javax.servlet.http.HttpServletRequest; -import javax.swing.text.html.Option; import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.Context; import javax.ws.rs.core.Response; @@ -38,7 +37,7 @@ public Response toResponse(WebApplicationException ex) { return ex.getResponse(); // Otherwise, do stuff. - JSONResponseBuilder jrb = JSONResponseBuilder.error(ex.getResponse()); + JsonResponseBuilder jrb = JsonResponseBuilder.error(ex.getResponse()); // See also https://en.wikipedia.org/wiki/List_of_HTTP_status_codes for a list of status codes. switch (ex.getResponse().getStatus()) { diff --git a/src/main/java/edu/harvard/iq/dataverse/api/util/JSONResponseBuilder.java b/src/main/java/edu/harvard/iq/dataverse/api/util/JsonResponseBuilder.java similarity index 91% rename from src/main/java/edu/harvard/iq/dataverse/api/util/JSONResponseBuilder.java rename to src/main/java/edu/harvard/iq/dataverse/api/util/JsonResponseBuilder.java index eea46eada2e..1cc9e8d77f5 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/util/JSONResponseBuilder.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/util/JsonResponseBuilder.java @@ -15,13 +15,13 @@ import java.util.logging.Level; import java.util.logging.Logger; -public class JSONResponseBuilder { +public class JsonResponseBuilder { private JsonObjectBuilder entityBuilder = Json.createObjectBuilder(); private Response.ResponseBuilder jerseyResponseBuilder; private boolean alreadyLogged = false; - private JSONResponseBuilder() {} + private JsonResponseBuilder() {} /** * Create an error response from an numeric error code (should be >= 400) @@ -29,12 +29,12 @@ private JSONResponseBuilder() {} * @return A builder with a basic JSON body * @throws IllegalArgumentException if fromResponse isn't an error response */ - public static JSONResponseBuilder error(int httpStatusCode) { + public static JsonResponseBuilder error(int httpStatusCode) { if (httpStatusCode < 400) { throw new IllegalArgumentException("A status code < 400 cannot be an error indicating response."); } - JSONResponseBuilder b = new JSONResponseBuilder(); + JsonResponseBuilder b = new JsonResponseBuilder(); b.jerseyResponseBuilder = Response.status(httpStatusCode); b.entityBuilder.add("status", "ERROR"); b.entityBuilder.add("code", httpStatusCode); @@ -50,8 +50,8 @@ public static JSONResponseBuilder error(int httpStatusCode) { * @return A builder with a basic JSON body * @throws IllegalArgumentException if fromResponse isn't an error response */ - public static JSONResponseBuilder error(Response.Status status) { - JSONResponseBuilder b = error(status.getStatusCode()); + public static JsonResponseBuilder error(Response.Status status) { + JsonResponseBuilder b = error(status.getStatusCode()); b.jerseyResponseBuilder = Response.status(status); return b; } @@ -62,8 +62,8 @@ public static JSONResponseBuilder error(Response.Status status) { * @return A builder with a basic JSON body * @throws IllegalArgumentException if fromResponse isn't an error response */ - public static JSONResponseBuilder error(Response fromResponse) { - JSONResponseBuilder b = error(fromResponse.getStatus()); + public static JsonResponseBuilder error(Response fromResponse) { + JsonResponseBuilder b = error(fromResponse.getStatus()); b.jerseyResponseBuilder = Response.fromResponse(fromResponse); return b; } @@ -73,7 +73,7 @@ public static JSONResponseBuilder error(Response fromResponse) { * @param message A human readable message * @return The enhanced builder */ - public JSONResponseBuilder message(String message) { + public JsonResponseBuilder message(String message) { this.entityBuilder.add("message", message); return this; } @@ -83,7 +83,7 @@ public JSONResponseBuilder message(String message) { * @param id A String containing an (ideally unique) identifier * @return The enhanced builder */ - public JSONResponseBuilder incidentId(String id) { + public JsonResponseBuilder incidentId(String id) { this.entityBuilder.add("incidentId", id); return this; } @@ -92,7 +92,7 @@ public JSONResponseBuilder incidentId(String id) { * Add a UUID random identifier for errors. Will overwrite existing. * @return The enhanced builder */ - public JSONResponseBuilder randomIncidentId() { + public JsonResponseBuilder randomIncidentId() { this.entityBuilder.add("incidentId", UUID.randomUUID().toString()); return this; } @@ -103,7 +103,7 @@ public JSONResponseBuilder randomIncidentId() { * @param request The original request (usually provided from a context) * @return The enhanced builder */ - public JSONResponseBuilder request(HttpServletRequest request) { + public JsonResponseBuilder request(HttpServletRequest request) { this.entityBuilder.add("requestUrl", getOriginalURL(request)); this.entityBuilder.add("requestMethod", request.getMethod()); return this; @@ -114,7 +114,7 @@ public JSONResponseBuilder request(HttpServletRequest request) { * @param request The original request (usually provided from a context) * @return The enhanced builder */ - public JSONResponseBuilder requestContentType(HttpServletRequest request) { + public JsonResponseBuilder requestContentType(HttpServletRequest request) { this.entityBuilder.add("requestContentType", request.getContentType()); return this; } @@ -125,7 +125,7 @@ public JSONResponseBuilder requestContentType(HttpServletRequest request) { * @param ex An exception. * @return The enhanced builder */ - public JSONResponseBuilder internalError(Throwable ex) { + public JsonResponseBuilder internalError(Throwable ex) { this.entityBuilder.add("interalError", ex.getClass().getSimpleName()); if (ex.getCause() != null) { this.entityBuilder.add("internalCause", ex.getCause().getClass().getSimpleName()); @@ -181,7 +181,7 @@ public void apply(HttpServletResponse response) throws IOException { * @param level Provide a level at which this should be logged * @return The unmodified builder. */ - public JSONResponseBuilder log(Logger logger, Level level) { + public JsonResponseBuilder log(Logger logger, Level level) { return this.log(logger, level, Optional.empty()); } @@ -202,7 +202,7 @@ public JSONResponseBuilder log(Logger logger, Level level) { * @param ex An optional exception to be included in the log message. * @return The unmodified builder. */ - public JSONResponseBuilder log(Logger logger, Level level, Optional ex) { + public JsonResponseBuilder log(Logger logger, Level level, Optional ex) { if ( ! logger.isLoggable(level) || alreadyLogged ) return this; diff --git a/src/test/java/edu/harvard/iq/dataverse/api/util/JSONResponseBuilderTest.java b/src/test/java/edu/harvard/iq/dataverse/api/util/JsonResponseBuilderTest.java similarity index 87% rename from src/test/java/edu/harvard/iq/dataverse/api/util/JSONResponseBuilderTest.java rename to src/test/java/edu/harvard/iq/dataverse/api/util/JsonResponseBuilderTest.java index 29e0536f73c..a6da689da7a 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/util/JSONResponseBuilderTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/util/JsonResponseBuilderTest.java @@ -1,7 +1,6 @@ package edu.harvard.iq.dataverse.api.util; import edu.harvard.iq.dataverse.api.ApiBlockingFilter; -import edu.harvard.iq.dataverse.api.errorhandlers.ThrowableHandler; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.EmptySource; import org.junit.jupiter.params.provider.NullSource; @@ -13,7 +12,7 @@ import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.*; -class JSONResponseBuilderTest { +class JsonResponseBuilderTest { @ParameterizedTest @NullSource @@ -27,6 +26,6 @@ class JSONResponseBuilderTest { void testMaskingOriginalURL(String query) { HttpServletRequest test = Mockito.mock(HttpServletRequest.class); when(test.getQueryString()).thenReturn(query); - assertFalse(JSONResponseBuilder.getOriginalURL(test).contains("supersecret")); + assertFalse(JsonResponseBuilder.getOriginalURL(test).contains("supersecret")); } } \ No newline at end of file From 40f7130d0d835447b7a23830d8b0508a27609f5f Mon Sep 17 00:00:00 2001 From: qqmyers Date: Mon, 10 Aug 2020 16:34:24 -0400 Subject: [PATCH 11/13] Handle null content type Adding a null check on the content type to fix what @kcondon saw in testing. @poikilotherm - if there's a better fix, go ahead and revert this - just wanted to keep testing moving. --- .../edu/harvard/iq/dataverse/api/util/JsonResponseBuilder.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/util/JsonResponseBuilder.java b/src/main/java/edu/harvard/iq/dataverse/api/util/JsonResponseBuilder.java index 1cc9e8d77f5..d9248202f76 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/util/JsonResponseBuilder.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/util/JsonResponseBuilder.java @@ -115,7 +115,8 @@ public JsonResponseBuilder request(HttpServletRequest request) { * @return The enhanced builder */ public JsonResponseBuilder requestContentType(HttpServletRequest request) { - this.entityBuilder.add("requestContentType", request.getContentType()); + String type = request.getContentType(); + this.entityBuilder.add("requestContentType", ((type==null) ? JsonValue.NULL : type)); return this; } From 27479f4ac272f4982052737e0a6a3bf4ff06db60 Mon Sep 17 00:00:00 2001 From: qqmyers Date: Mon, 10 Aug 2020 16:40:51 -0400 Subject: [PATCH 12/13] add JsonValue import --- .../edu/harvard/iq/dataverse/api/util/JsonResponseBuilder.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/util/JsonResponseBuilder.java b/src/main/java/edu/harvard/iq/dataverse/api/util/JsonResponseBuilder.java index d9248202f76..86294a447b5 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/util/JsonResponseBuilder.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/util/JsonResponseBuilder.java @@ -3,6 +3,7 @@ import edu.harvard.iq.dataverse.api.ApiBlockingFilter; import javax.json.Json; +import javax.json.JsonValue; import javax.json.JsonObjectBuilder; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; From 8f879d7bb23a99b5d00071359f2c29dd6896094f Mon Sep 17 00:00:00 2001 From: qqmyers Date: Mon, 10 Aug 2020 16:49:29 -0400 Subject: [PATCH 13/13] and convert string to JsonValue --- .../edu/harvard/iq/dataverse/api/util/JsonResponseBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/util/JsonResponseBuilder.java b/src/main/java/edu/harvard/iq/dataverse/api/util/JsonResponseBuilder.java index 86294a447b5..d3c6fd2df50 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/util/JsonResponseBuilder.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/util/JsonResponseBuilder.java @@ -117,7 +117,7 @@ public JsonResponseBuilder request(HttpServletRequest request) { */ public JsonResponseBuilder requestContentType(HttpServletRequest request) { String type = request.getContentType(); - this.entityBuilder.add("requestContentType", ((type==null) ? JsonValue.NULL : type)); + this.entityBuilder.add("requestContentType", ((type==null) ? JsonValue.NULL : Json.createValue(type))); return this; }