ErrorResource.toStatus() handles AccessDeniedException (closes #321)

cantaloupe-project · Sep 23, 2019 · 6face3c · 6face3c
1 parent 0ee7c4a
commit 6face3c
Show file tree

Hide file tree

Showing 9 changed files with 89 additions and 26 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -9,6 +9,8 @@
   image endpoint responses.
 * The `?response-content-disposition` query argument correctly handles
   filenames containing spaces.
+* Image endpoints return HTTP 403 instead of 500 in response to
+  AccessDeniedExceptions from the underlying Source.
 * Fixed two separate bugs in KakaduNativeProcessor that both caused empty
   regions to appear in certain images.
 * Fixed overly restrictive signature verification in KakaduDemoProcessor and

diff --git a/src/main/java/edu/illinois/library/cantaloupe/resource/ErrorResource.java b/src/main/java/edu/illinois/library/cantaloupe/resource/ErrorResource.java
@@ -17,6 +17,7 @@
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.io.UnsupportedEncodingException;
+import java.nio.file.AccessDeniedException;
 import java.nio.file.NoSuchFileException;
 import java.util.Arrays;
 import java.util.List;
@@ -35,6 +36,31 @@ class ErrorResource extends AbstractResource {
 
     private Throwable error;
 
+    private static Status toStatus(Throwable t) {
+        Status status;
+        if (t instanceof ResourceException) {
+            status = ((ResourceException) t).getStatus();
+        } else if (t instanceof IllegalSizeException ||
+                t instanceof IllegalScaleException ||
+                t instanceof AccessDeniedException) {
+            status = Status.FORBIDDEN;
+        } else if (t instanceof ValidationException ||
+                t instanceof IllegalClientArgumentException ||
+                t instanceof UnsupportedEncodingException) {
+            status = Status.BAD_REQUEST;
+        } else if (t instanceof UnsupportedOutputFormatException) {
+            status = Status.UNSUPPORTED_MEDIA_TYPE;
+        } else if (t instanceof FileNotFoundException ||
+                t instanceof NoSuchFileException) {
+            status = Status.NOT_FOUND;
+        } else if (t instanceof UnsupportedSourceFormatException) {
+            status = Status.NOT_IMPLEMENTED;
+        } else {
+            status = Status.INTERNAL_SERVER_ERROR;
+        }
+        return status;
+    }
+
     ErrorResource(Throwable error) {
         this.error = error;
     }
@@ -100,29 +126,4 @@ private String getStackTrace() {
         }
     }
 
-    private Status toStatus(Throwable t) {
-        Status status;
-
-        if (t instanceof ResourceException) {
-            status = ((ResourceException) t).getStatus();
-        } else if (t instanceof IllegalSizeException ||
-                t instanceof IllegalScaleException) {
-            status = Status.FORBIDDEN;
-        } else if (t instanceof ValidationException ||
-                t instanceof IllegalClientArgumentException ||
-                t instanceof UnsupportedEncodingException) {
-            status = Status.BAD_REQUEST;
-        } else if (t instanceof UnsupportedOutputFormatException) {
-            status = Status.UNSUPPORTED_MEDIA_TYPE;
-        } else if (t instanceof FileNotFoundException ||
-                t instanceof NoSuchFileException) {
-            status = Status.NOT_FOUND;
-        } else if (t instanceof UnsupportedSourceFormatException) {
-            status = Status.NOT_IMPLEMENTED;
-        } else {
-            status = Status.INTERNAL_SERVER_ERROR;
-        }
-        return status;
-    }
-
 }
diff --git a/src/main/java/edu/illinois/library/cantaloupe/resource/HandlerServlet.java b/src/main/java/edu/illinois/library/cantaloupe/resource/HandlerServlet.java
@@ -145,7 +145,8 @@ private void handleError(HttpServletRequest request,
         // Try to use an ErrorResource, which will render an HTML template.
         ErrorResource resource = new ErrorResource(t);
         try {
-            response.setStatus(500);
+            // N.B.: the response status will be set by ErrorResource based on
+            // the type of Throwable.
             response.setContentType("text/html;charset=UTF-8");
             resource.setRequest(new Request(request));
             resource.setResponse(response);

diff --git a/src/test/java/edu/illinois/library/cantaloupe/resource/iiif/ImageAPIResourceTester.java b/src/test/java/edu/illinois/library/cantaloupe/resource/iiif/ImageAPIResourceTester.java
@@ -13,6 +13,7 @@
 import edu.illinois.library.cantaloupe.http.Transport;
 import edu.illinois.library.cantaloupe.image.Format;
 import edu.illinois.library.cantaloupe.image.Identifier;
+import edu.illinois.library.cantaloupe.source.AccessDeniedSource;
 import edu.illinois.library.cantaloupe.source.FileSource;
 import edu.illinois.library.cantaloupe.source.PathStreamFactory;
 import edu.illinois.library.cantaloupe.source.StreamFactory;
@@ -157,6 +158,14 @@ public void testCachingWhenCachesAreEnabledButNegativeCacheQueryArgumentIsSuppli
         }
     }
 
+    public void testForbidden(URI uri) {
+        Configuration config = Configuration.getInstance();
+        config.setProperty(Key.SOURCE_STATIC,
+                AccessDeniedSource.class.getName());
+
+        assertStatus(403, uri);
+    }
+
     public void testHTTP2(URI uri) throws Exception {
         Client client = newClient(uri);
         try {

diff --git a/src/test/java/edu/illinois/library/cantaloupe/resource/iiif/v1/ImageResourceTest.java b/src/test/java/edu/illinois/library/cantaloupe/resource/iiif/v1/ImageResourceTest.java
@@ -301,6 +301,12 @@ public void testGETMaxPixelsIgnoredWhenStreamingSource() {
         tester.testMaxPixelsIgnoredWhenStreamingSource(uri);
     }
 
+    @Test
+    public void testGETForbidden() {
+        URI uri = getHTTPURI("/forbidden/full/full/0/color.jpg");
+        tester.testForbidden(uri);
+    }
+
     @Test
     public void testGETNotFound() {
         URI uri = getHTTPURI("/invalid/full/full/0/color.jpg");

diff --git a/src/test/java/edu/illinois/library/cantaloupe/resource/iiif/v1/InformationResourceTest.java b/src/test/java/edu/illinois/library/cantaloupe/resource/iiif/v1/InformationResourceTest.java
@@ -205,6 +205,12 @@ public void testGETHTTPS2() throws Exception {
         tester.testHTTPS2(uri);
     }
 
+    @Test
+    public void testGETForbidden() {
+        URI uri = getHTTPURI("/forbidden/info.json");
+        tester.testForbidden(uri);
+    }
+
     @Test
     public void testGETNotFound() {
         URI uri = getHTTPURI("/invalid/info.json");

diff --git a/src/test/java/edu/illinois/library/cantaloupe/resource/iiif/v2/ImageResourceTest.java b/src/test/java/edu/illinois/library/cantaloupe/resource/iiif/v2/ImageResourceTest.java
@@ -375,6 +375,12 @@ public void testGETMaxPixelsIgnoredWhenStreamingSource() {
         tester.testMaxPixelsIgnoredWhenStreamingSource(uri);
     }
 
+    @Test
+    public void testGETForbidden() {
+        URI uri = getHTTPURI("/forbidden/full/full/0/color.jpg");
+        tester.testForbidden(uri);
+    }
+
     @Test
     public void testGETNotFound() {
         URI uri = getHTTPURI("/invalid/full/full/0/color.jpg");

diff --git a/src/test/java/edu/illinois/library/cantaloupe/resource/iiif/v2/InformationResourceTest.java b/src/test/java/edu/illinois/library/cantaloupe/resource/iiif/v2/InformationResourceTest.java
@@ -205,6 +205,12 @@ public void testGETHTTPS2() throws Exception {
         tester.testHTTPS2(uri);
     }
 
+    @Test
+    public void testGETForbidden() {
+        URI uri = getHTTPURI("/forbidden/info.json");
+        tester.testForbidden(uri);
+    }
+
     @Test
     public void testGETNotFound() {
         URI uri = getHTTPURI("/invalid/info.json");

diff --git a/src/test/java/edu/illinois/library/cantaloupe/source/AccessDeniedSource.java b/src/test/java/edu/illinois/library/cantaloupe/source/AccessDeniedSource.java
@@ -0,0 +1,26 @@
+package edu.illinois.library.cantaloupe.source;
+
+import edu.illinois.library.cantaloupe.image.Format;
+
+import java.io.IOException;
+import java.nio.file.AccessDeniedException;
+import java.nio.file.Path;
+
+public class AccessDeniedSource extends AbstractSource implements FileSource {
+
+    @Override
+    public void checkAccess() throws IOException {
+        throw new AccessDeniedException("");
+    }
+
+    @Override
+    public Format getFormat() throws IOException {
+        return null;
+    }
+
+    @Override
+    public Path getPath() throws IOException {
+        return null;
+    }
+
+}