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
UNC-Libraries · Aug 20, 2024 · 6833f69 · 6833f69
1 parent f300ea2
commit 6833f69
Show file tree

Hide file tree

Showing 9 changed files with 126 additions and 6 deletions.
diff --git a/...-utils/src/main/java/edu/unc/lib/boxc/fcrepo/exceptions/RangeNotSatisfiableException.java b/...-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
@@ -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/...ls/src/test/java/edu/unc/lib/boxc/fcrepo/exceptions/RangeNotSatisfiableExceptionTest.java b/...ls/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
@@ -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/...ss-app/src/main/java/edu/unc/lib/boxc/web/access/controllers/FedoraContentController.java b/...ss-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<Object> handleObjectTypeMismatchException() {
     }
 
     @ExceptionHandler(value = { RuntimeException.class })
-    public ResponseEntity<Object> handleUncaught(RuntimeException ex) {
-        log.error("Uncaught exception while streaming content", ex);
+    public ResponseEntity<Object> 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<Object> handleArgumentTypeMismatch(RuntimeException ex) {
         return new ResponseEntity<>(HttpStatus.BAD_REQUEST);
     }
 
+    @ExceptionHandler(RangeNotSatisfiableException.class)
+    public ResponseEntity<Object> handleRangeNotSatisfiable() {
+        return new ResponseEntity<>(HttpStatus.REQUESTED_RANGE_NOT_SATISFIABLE);
+    }
+
     public void setFedoraContentService(FedoraContentService fedoraContentService) {
         this.fedoraContentService = fedoraContentService;
     }

diff --git a/...-app/src/test/java/edu/unc/lib/boxc/web/access/controllers/FedoraContentControllerIT.java b/...-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/...pp/src/test/java/edu/unc/lib/boxc/web/access/controllers/FedoraContentControllerTest.java b/...pp/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/...ava/edu/unc/lib/boxc/web/services/rest/exceptions/RestResponseEntityExceptionHandler.java b/...ava/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<Object> handleEofException(EOFException ex, WebRequest req
         return null;
     }
 
+    @ExceptionHandler(RangeNotSatisfiableException.class)
+    public ResponseEntity<Object> 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<Object> handleUnavailable(Exception ex, WebRequest request) {
         try {

diff --git a/...ices-app/src/test/java/edu/unc/lib/boxc/web/services/rest/DatastreamRestControllerIT.java b/...ices-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,