diff --git a/src/main/java/dev/zarr/zarrjava/store/HttpStore.java b/src/main/java/dev/zarr/zarrjava/store/HttpStore.java index 2c1a1b2..a0af1e8 100644 --- a/src/main/java/dev/zarr/zarrjava/store/HttpStore.java +++ b/src/main/java/dev/zarr/zarrjava/store/HttpStore.java @@ -128,8 +128,12 @@ public InputStream getInputStream(String[] keys, long start, long end) { if (start < 0) { throw new IllegalArgumentException("Argument 'start' needs to be non-negative."); } - Request request = new Request.Builder().url(resolveKeys(keys)).header( - "Range", String.format("bytes=%d-%d", start, end - 1)).build(); + // A negative end means "until the end of the object", which HTTP expresses as an + // open-ended range. + String range = end < 0 + ? String.format("bytes=%d-", start) + : String.format("bytes=%d-%d", start, end - 1); + Request request = new Request.Builder().url(resolveKeys(keys)).header("Range", range).build(); try { // We do NOT use try-with-resources here because the stream must remain open diff --git a/src/main/java/dev/zarr/zarrjava/store/ReadOnlyZipStore.java b/src/main/java/dev/zarr/zarrjava/store/ReadOnlyZipStore.java index 689db73..2229398 100644 --- a/src/main/java/dev/zarr/zarrjava/store/ReadOnlyZipStore.java +++ b/src/main/java/dev/zarr/zarrjava/store/ReadOnlyZipStore.java @@ -50,8 +50,10 @@ private synchronized void ensureCache() { InputStream inputStream = underlyingStore.getInputStream(); if (inputStream == null) { - isCached = true; - return; + throw StoreException.readFailed( + underlyingStore.toString(), + new String[]{}, + new IOException("Could not open the ZIP archive from the underlying store")); } try (ZipArchiveInputStream zis = new ZipArchiveInputStream(inputStream)) { diff --git a/src/main/java/dev/zarr/zarrjava/store/S3Store.java b/src/main/java/dev/zarr/zarrjava/store/S3Store.java index 5eda1b8..dbb9599 100644 --- a/src/main/java/dev/zarr/zarrjava/store/S3Store.java +++ b/src/main/java/dev/zarr/zarrjava/store/S3Store.java @@ -103,10 +103,17 @@ public ByteBuffer get(String[] keys, long start) { @Nullable @Override public ByteBuffer get(String[] keys, long start, long end) { + if (start < 0) { + throw new IllegalArgumentException("Argument 'start' needs to be non-negative."); + } + // S3 ranges are inclusive; a negative end means "until the end of the object". + String range = end < 0 + ? String.format("bytes=%d-", start) + : String.format("bytes=%d-%d", start, end - 1); GetObjectRequest req = GetObjectRequest.builder() .bucket(bucketName) .key(resolveKeys(keys)) - .range(String.format("bytes=%d-%d", start, end - 1)) // S3 range is inclusive + .range(range) .build(); return get(req); } @@ -221,10 +228,17 @@ public StoreHandle resolve(String... keys) { @Override public InputStream getInputStream(String[] keys, long start, long end) { + if (start < 0) { + throw new IllegalArgumentException("Argument 'start' needs to be non-negative."); + } + // S3 ranges are inclusive; a negative end means "until the end of the object". + String range = end < 0 + ? String.format("bytes=%d-", start) + : String.format("bytes=%d-%d", start, end - 1); GetObjectRequest req = GetObjectRequest.builder() .bucket(bucketName) .key(resolveKeys(keys)) - .range(String.format("bytes=%d-%d", start, end - 1)) // S3 range is inclusive + .range(range) .build(); return s3client.getObject(req); } diff --git a/src/test/java/dev/zarr/zarrjava/store/HttpStoreTest.java b/src/test/java/dev/zarr/zarrjava/store/HttpStoreTest.java index 911d4bf..ffdf897 100644 --- a/src/test/java/dev/zarr/zarrjava/store/HttpStoreTest.java +++ b/src/test/java/dev/zarr/zarrjava/store/HttpStoreTest.java @@ -87,6 +87,28 @@ public void testNoRetryOn404() throws IOException { } } + @Test + public void testOpenEndedRangeHeader() throws IOException, InterruptedException { + try (MockWebServer server = new MockWebServer()) { + server.enqueue(new MockResponse().setBody("data").setResponseCode(206)); + server.start(); + HttpStore httpStore = new HttpStore(server.url("/").toString(), 1, 3, 10); + Assertions.assertNotNull(httpStore.getInputStream(new String[]{"path"}, 0, -1)); + Assertions.assertEquals("bytes=0-", server.takeRequest().getHeader("Range")); + } + } + + @Test + public void testBoundedRangeHeader() throws IOException, InterruptedException { + try (MockWebServer server = new MockWebServer()) { + server.enqueue(new MockResponse().setBody("dat").setResponseCode(206)); + server.start(); + HttpStore httpStore = new HttpStore(server.url("/").toString(), 1, 3, 10); + Assertions.assertNotNull(httpStore.getInputStream(new String[]{"path"}, 1, 4)); + Assertions.assertEquals("bytes=1-3", server.takeRequest().getHeader("Range")); + } + } + @Override @Test @Disabled("List is not supported in HttpStore") diff --git a/src/test/java/dev/zarr/zarrjava/store/ReadOnlyZipStoreTest.java b/src/test/java/dev/zarr/zarrjava/store/ReadOnlyZipStoreTest.java index 54b5373..b1a28d8 100644 --- a/src/test/java/dev/zarr/zarrjava/store/ReadOnlyZipStoreTest.java +++ b/src/test/java/dev/zarr/zarrjava/store/ReadOnlyZipStoreTest.java @@ -42,6 +42,13 @@ Store storeWithArrays() { } + @Test + public void testUnreadableArchiveThrows() { + ReadOnlyZipStore zipStore = new ReadOnlyZipStore(TESTOUTPUT.resolve("does_not_exist.zip")); + Assertions.assertThrows(StoreException.class, () -> zipStore.resolve().listChildren().count()); + Assertions.assertThrows(StoreException.class, () -> zipStore.resolve("array", "0.0.0").exists()); + } + @Override @Test public void testListChildren() { diff --git a/src/test/java/dev/zarr/zarrjava/store/StoreTest.java b/src/test/java/dev/zarr/zarrjava/store/StoreTest.java index f99cfe2..4f1c80a 100644 --- a/src/test/java/dev/zarr/zarrjava/store/StoreTest.java +++ b/src/test/java/dev/zarr/zarrjava/store/StoreTest.java @@ -46,6 +46,29 @@ public void testInputStream() throws IOException { Assertions.assertArrayEquals(expectedBuffer, buffer); } + @Test + public void testInputStreamOpenEnded() throws IOException { + StoreHandle storeHandle = storeHandleWithData(); + ByteBuffer expected = storeHandle.read(); + try (InputStream is = storeHandle.getInputStream()) { + Assertions.assertNotNull(is, "Open-ended getInputStream() returned null"); + byte[] actual = readFully(is); + byte[] expectedBytes = new byte[expected.remaining()]; + expected.get(expectedBytes); + Assertions.assertArrayEquals(expectedBytes, actual); + } + } + + private static byte[] readFully(InputStream is) throws IOException { + java.io.ByteArrayOutputStream baos = new java.io.ByteArrayOutputStream(); + byte[] buffer = new byte[8192]; + int len; + while ((len = is.read(buffer)) != -1) { + baos.write(buffer, 0, len); + } + return baos.toByteArray(); + } + @Test public void testExists() throws ZarrException, IOException { Assertions.assertTrue(storeHandleWithData().exists());