From 6833f69cae1a0fc8c03bd6ab9ec6a7c03933a46c Mon Sep 17 00:00:00 2001 From: Ben Pennell Date: Tue, 20 Aug 2024 15:40:52 -0400 Subject: [PATCH] BXC-4694 - Handle RangeNotSatisfiable (#1785) * Added new RangeNotSatisfiableException, which gets thrown if the range header exceeds the size of the file. Handle this error with a 416 response * Log headers when an uncaught exception occurs in fedora content controller * Add test for exception --- .../RangeNotSatisfiableException.java | 24 ++++++++++++++ .../fcrepo/utils/ClientFaultResolver.java | 7 ++-- .../RangeNotSatisfiableExceptionTest.java | 33 +++++++++++++++++++ .../model/api/exceptions/FedoraException.java | 4 +-- .../controllers/FedoraContentController.java | 17 ++++++++-- .../FedoraContentControllerIT.java | 13 ++++++++ .../FedoraContentControllerTest.java | 11 +++++++ .../RestResponseEntityExceptionHandler.java | 8 +++++ .../rest/DatastreamRestControllerIT.java | 15 +++++++++ 9 files changed, 126 insertions(+), 6 deletions(-) create mode 100644 fcrepo-utils/src/main/java/edu/unc/lib/boxc/fcrepo/exceptions/RangeNotSatisfiableException.java create mode 100644 fcrepo-utils/src/test/java/edu/unc/lib/boxc/fcrepo/exceptions/RangeNotSatisfiableExceptionTest.java diff --git a/fcrepo-utils/src/main/java/edu/unc/lib/boxc/fcrepo/exceptions/RangeNotSatisfiableException.java b/fcrepo-utils/src/main/java/edu/unc/lib/boxc/fcrepo/exceptions/RangeNotSatisfiableException.java new file mode 100644 index 0000000000..7fb5503369 --- /dev/null +++ b/fcrepo-utils/src/main/java/edu/unc/lib/boxc/fcrepo/exceptions/RangeNotSatisfiableException.java @@ -0,0 +1,24 @@ +package edu.unc.lib.boxc.fcrepo.exceptions; + +import edu.unc.lib.boxc.model.api.exceptions.FedoraException; + +/** + * Error indicates that a HTTP range request could not be satisfied + * + * @author bbpennel + */ +public class RangeNotSatisfiableException extends FedoraException { + private static final long serialVersionUID = 1L; + + public RangeNotSatisfiableException(String message) { + super(message); + } + + public RangeNotSatisfiableException(Throwable cause) { + super(cause); + } + + public RangeNotSatisfiableException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/fcrepo-utils/src/main/java/edu/unc/lib/boxc/fcrepo/utils/ClientFaultResolver.java b/fcrepo-utils/src/main/java/edu/unc/lib/boxc/fcrepo/utils/ClientFaultResolver.java index 2fe48ab9c0..0775b5f19c 100644 --- a/fcrepo-utils/src/main/java/edu/unc/lib/boxc/fcrepo/utils/ClientFaultResolver.java +++ b/fcrepo-utils/src/main/java/edu/unc/lib/boxc/fcrepo/utils/ClientFaultResolver.java @@ -1,5 +1,6 @@ package edu.unc.lib.boxc.fcrepo.utils; +import edu.unc.lib.boxc.fcrepo.exceptions.RangeNotSatisfiableException; import org.apache.http.HttpStatus; import org.fcrepo.client.FcrepoOperationFailedException; @@ -31,9 +32,11 @@ public static FedoraException resolve(Exception ex) { case HttpStatus.SC_FORBIDDEN: return new AuthorizationException(ex); case HttpStatus.SC_NOT_FOUND: - return new NotFoundException(ex); + return new NotFoundException(ex); case HttpStatus.SC_CONFLICT: - return new ConflictException(ex); + return new ConflictException(ex); + case HttpStatus.SC_REQUESTED_RANGE_NOT_SATISFIABLE: + return new RangeNotSatisfiableException(ex); } } return new FedoraException(ex); diff --git a/fcrepo-utils/src/test/java/edu/unc/lib/boxc/fcrepo/exceptions/RangeNotSatisfiableExceptionTest.java b/fcrepo-utils/src/test/java/edu/unc/lib/boxc/fcrepo/exceptions/RangeNotSatisfiableExceptionTest.java new file mode 100644 index 0000000000..4d7ca6604b --- /dev/null +++ b/fcrepo-utils/src/test/java/edu/unc/lib/boxc/fcrepo/exceptions/RangeNotSatisfiableExceptionTest.java @@ -0,0 +1,33 @@ +package edu.unc.lib.boxc.fcrepo.exceptions; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +/** + * @author bbpennel + */ +public class RangeNotSatisfiableExceptionTest { + @Test + public void testConstructorWithMessage() { + var ex = new RangeNotSatisfiableException("Bad range"); + assertEquals("Bad range", ex.getMessage()); + assertNull(ex.getCause()); + } + + @Test + public void testConstructorWithThrowable() { + var causeEx = new RuntimeException(); + var ex = new RangeNotSatisfiableException(causeEx); + assertEquals(causeEx, ex.getCause()); + } + + @Test + public void testConstructorWithMessageThrowable() { + var causeEx = new RuntimeException(); + var ex = new RangeNotSatisfiableException("Bad range", causeEx); + assertEquals("Bad range", ex.getMessage()); + assertEquals(causeEx, ex.getCause()); + } +} diff --git a/model-api/src/main/java/edu/unc/lib/boxc/model/api/exceptions/FedoraException.java b/model-api/src/main/java/edu/unc/lib/boxc/model/api/exceptions/FedoraException.java index 7e435ddabc..9039f70ce1 100644 --- a/model-api/src/main/java/edu/unc/lib/boxc/model/api/exceptions/FedoraException.java +++ b/model-api/src/main/java/edu/unc/lib/boxc/model/api/exceptions/FedoraException.java @@ -8,11 +8,11 @@ public class FedoraException extends RuntimeException { private static final long serialVersionUID = 7276162681909269101L; - public FedoraException(Exception e) { + public FedoraException(Throwable e) { super(e); } - public FedoraException(String message, Exception e) { + public FedoraException(String message, Throwable e) { super(message, e); } diff --git a/web-access-app/src/main/java/edu/unc/lib/boxc/web/access/controllers/FedoraContentController.java b/web-access-app/src/main/java/edu/unc/lib/boxc/web/access/controllers/FedoraContentController.java index ae59b22654..a2a620e9a6 100644 --- a/web-access-app/src/main/java/edu/unc/lib/boxc/web/access/controllers/FedoraContentController.java +++ b/web-access-app/src/main/java/edu/unc/lib/boxc/web/access/controllers/FedoraContentController.java @@ -3,6 +3,7 @@ import edu.unc.lib.boxc.auth.api.exceptions.AccessRestrictionException; import edu.unc.lib.boxc.auth.api.models.AccessGroupSet; import edu.unc.lib.boxc.auth.api.services.AccessControlService; +import edu.unc.lib.boxc.fcrepo.exceptions.RangeNotSatisfiableException; import edu.unc.lib.boxc.model.api.exceptions.InvalidPidException; import edu.unc.lib.boxc.model.api.exceptions.NotFoundException; import edu.unc.lib.boxc.model.api.exceptions.ObjectTypeMismatchException; @@ -27,6 +28,8 @@ import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.context.request.ServletWebRequest; +import org.springframework.web.context.request.WebRequest; import org.springframework.web.method.annotation.MethodArgumentTypeMismatchException; import javax.servlet.http.HttpServletRequest; @@ -157,8 +160,13 @@ public ResponseEntity handleObjectTypeMismatchException() { } @ExceptionHandler(value = { RuntimeException.class }) - public ResponseEntity handleUncaught(RuntimeException ex) { - log.error("Uncaught exception while streaming content", ex); + public ResponseEntity handleUncaught(RuntimeException ex, WebRequest request) { + var headers = new StringBuilder(); + request.getHeaderNames().forEachRemaining(header -> { + headers.append("\n").append(header).append(" = ").append(request.getHeader(header)); + }); + var requestUri = ((ServletWebRequest) request).getRequest().getRequestURI(); + log.error("Uncaught exception while streaming content from {} headers {}", requestUri, headers, ex); return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR); } @@ -168,6 +176,11 @@ public ResponseEntity handleArgumentTypeMismatch(RuntimeException ex) { return new ResponseEntity<>(HttpStatus.BAD_REQUEST); } + @ExceptionHandler(RangeNotSatisfiableException.class) + public ResponseEntity handleRangeNotSatisfiable() { + return new ResponseEntity<>(HttpStatus.REQUESTED_RANGE_NOT_SATISFIABLE); + } + public void setFedoraContentService(FedoraContentService fedoraContentService) { this.fedoraContentService = fedoraContentService; } diff --git a/web-access-app/src/test/java/edu/unc/lib/boxc/web/access/controllers/FedoraContentControllerIT.java b/web-access-app/src/test/java/edu/unc/lib/boxc/web/access/controllers/FedoraContentControllerIT.java index c7690f644e..c146895e30 100644 --- a/web-access-app/src/test/java/edu/unc/lib/boxc/web/access/controllers/FedoraContentControllerIT.java +++ b/web-access-app/src/test/java/edu/unc/lib/boxc/web/access/controllers/FedoraContentControllerIT.java @@ -277,6 +277,19 @@ private void testGetMultipleDatastreamsWithRange(String requestPath) throws Exce assertEquals("inline; filename=\"fits.xml\"", response.getHeader(CONTENT_DISPOSITION)); } + @Test + public void testRangeExceedsFileLength() throws Exception { + PID filePid = makePid(); + FileObject fileObj = repositoryObjectFactory.createFileObject(filePid, null); + fileObj.addOriginalFile(makeContentUri(originalPid(fileObj), BINARY_CONTENT), null, "text/plain", null, null); + + mvc.perform(get("/indexablecontent/" + filePid.getId()) + .header(RANGE,"bytes=0-900000")) + .andExpect(status().isRequestedRangeNotSatisfiable()) + .andReturn(); + } + + @Test public void testGetAdministrativeDatastreamNoPermissions() throws Exception { PID filePid = makePid(); diff --git a/web-access-app/src/test/java/edu/unc/lib/boxc/web/access/controllers/FedoraContentControllerTest.java b/web-access-app/src/test/java/edu/unc/lib/boxc/web/access/controllers/FedoraContentControllerTest.java index 0ac25d7521..622cd963bf 100644 --- a/web-access-app/src/test/java/edu/unc/lib/boxc/web/access/controllers/FedoraContentControllerTest.java +++ b/web-access-app/src/test/java/edu/unc/lib/boxc/web/access/controllers/FedoraContentControllerTest.java @@ -104,4 +104,15 @@ public void getContentIOExceptionTest() throws Exception { .andExpect(status().isBadRequest()) .andReturn(); } + + @Test + public void getContentUncaughtExceptionTest() throws Exception { + PID pid = TestHelper.makePid(); + doThrow(new RuntimeException("Uncaught")) + .when(fedoraContentService) + .streamData(any(), any(), anyBoolean(), any(), any()); + mvc.perform(get("/content/" + pid.getId()).header("Range", "bad")) + .andExpect(status().isInternalServerError()) + .andReturn(); + } } diff --git a/web-services-app/src/main/java/edu/unc/lib/boxc/web/services/rest/exceptions/RestResponseEntityExceptionHandler.java b/web-services-app/src/main/java/edu/unc/lib/boxc/web/services/rest/exceptions/RestResponseEntityExceptionHandler.java index ed9f39ab6e..477497d251 100644 --- a/web-services-app/src/main/java/edu/unc/lib/boxc/web/services/rest/exceptions/RestResponseEntityExceptionHandler.java +++ b/web-services-app/src/main/java/edu/unc/lib/boxc/web/services/rest/exceptions/RestResponseEntityExceptionHandler.java @@ -1,5 +1,6 @@ package edu.unc.lib.boxc.web.services.rest.exceptions; +import edu.unc.lib.boxc.fcrepo.exceptions.RangeNotSatisfiableException; import edu.unc.lib.boxc.model.api.exceptions.InvalidOperationForObjectType; import java.io.EOFException; @@ -76,6 +77,13 @@ public ResponseEntity handleEofException(EOFException ex, WebRequest req return null; } + @ExceptionHandler(RangeNotSatisfiableException.class) + public ResponseEntity handleRangeNotSatisfiable(RuntimeException ex, WebRequest request) { + var rangeValue = request.getHeader(HttpHeaders.RANGE); + log.debug("Unsatisfiable range {} requested {}", rangeValue, getRequestUri(request), ex); + return handleExceptionInternal(ex, null, new HttpHeaders(), HttpStatus.REQUESTED_RANGE_NOT_SATISFIABLE, request); + } + @ExceptionHandler(value = { SolrRuntimeException.class }) protected ResponseEntity handleUnavailable(Exception ex, WebRequest request) { try { diff --git a/web-services-app/src/test/java/edu/unc/lib/boxc/web/services/rest/DatastreamRestControllerIT.java b/web-services-app/src/test/java/edu/unc/lib/boxc/web/services/rest/DatastreamRestControllerIT.java index d36e9f5d62..466aa355f9 100644 --- a/web-services-app/src/test/java/edu/unc/lib/boxc/web/services/rest/DatastreamRestControllerIT.java +++ b/web-services-app/src/test/java/edu/unc/lib/boxc/web/services/rest/DatastreamRestControllerIT.java @@ -12,6 +12,7 @@ import static edu.unc.lib.boxc.model.fcrepo.ids.RepositoryPaths.idToPath; import static edu.unc.lib.boxc.web.common.services.FedoraContentService.CONTENT_DISPOSITION; import static java.nio.charset.StandardCharsets.UTF_8; +import static org.apache.http.HttpHeaders.RANGE; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; @@ -355,6 +356,20 @@ public void testGetEventLogNoPermissions() throws Exception { assertEquals("", response.getContentAsString(), "Content must not be returned"); } + @Test + public void testRangeExceedsFileLength() throws Exception { + PID filePid = makePid(); + + FileObject fileObj = repositoryObjectFactory.createFileObject(filePid, null); + Path contentPath = createBinaryContent(BINARY_CONTENT); + fileObj.addOriginalFile(contentPath.toUri(), "file.txt", "text/plain", null, null); + + mvc.perform(get("/file/" + filePid.getId()) + .header(RANGE,"bytes=0-900000")) + .andExpect(status().isRequestedRangeNotSatisfiable()) + .andReturn(); + } + private File createDerivative(String id, DatastreamType dsType, byte[] content) throws Exception { String hashedPath = idToPath(id, HASHED_PATH_DEPTH, HASHED_PATH_SIZE); Path derivPath = Paths.get(derivDirPath, dsType.getId(), hashedPath,