Skip to content

Commit 4e4eea9

Browse files
brokkoli71brokkoli71
authored
Change Array.read Shape Argument long & Store Request Validation (#54)
* change int to long * Add path traversal validation to FilesystemStore * Enhance path validation and error handling in FilesystemStore and HttpStore; update chunk handling in IndexingUtils to support long values * better exceptions * remove redundant exist calls * returning null only on key not found * remove unused partialDecode, partialEncode as decodePartial exists * remove unused fOrderIndex and cOrderIndex
1 parent bcee844 commit 4e4eea9

20 files changed

Lines changed: 423 additions & 158 deletions

‎src/main/java/dev/zarr/zarrjava/core/Array.java‎

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ public void write(long[] offset, ucar.ma2.Array array, boolean parallel) {
9292
throw new IllegalArgumentException("'array' needs to have rank '" + metadata.ndim() + "'.");
9393
}
9494

95-
int[] shape = array.getShape();
95+
long[] shape = Utils.toLongArray(array.getShape());
9696

9797
final int[] chunkShape = metadata.chunkShape();
9898
Stream<long[]> chunkStream = Arrays.stream(IndexingUtils.computeChunkCoords(metadata.shape, chunkShape, offset, shape));
@@ -122,8 +122,14 @@ public void write(long[] offset, ucar.ma2.Array array, boolean parallel) {
122122
);
123123
}
124124
writeChunk(chunkCoords, chunkArray);
125-
} catch (ZarrException | InvalidRangeException e) {
126-
throw new RuntimeException(e);
125+
} catch (ZarrException e) {
126+
throw new RuntimeException(
127+
"Failed to write chunk at coordinates " + Arrays.toString(chunkCoords) +
128+
": " + e.getMessage(), e);
129+
} catch (InvalidRangeException e) {
130+
throw new RuntimeException(
131+
"Invalid array range when writing chunk at coordinates " + Arrays.toString(chunkCoords) +
132+
": " + e.getMessage(), e);
127133
}
128134
});
129135

@@ -221,7 +227,7 @@ public void write(ucar.ma2.Array array, boolean parallel) {
221227
*/
222228
@Nonnull
223229
public ucar.ma2.Array read() throws ZarrException {
224-
return read(new long[metadata().ndim()], Utils.toIntArray(metadata().shape));
230+
return read(new long[metadata().ndim()], metadata().shape);
225231
}
226232

227233
/**
@@ -233,7 +239,7 @@ public ucar.ma2.Array read() throws ZarrException {
233239
* @throws ZarrException throws ZarrException if the requested data is outside the array's domain or if the read fails
234240
*/
235241
@Nonnull
236-
public ucar.ma2.Array read(final long[] offset, final int[] shape) throws ZarrException {
242+
public ucar.ma2.Array read(final long[] offset, final long[] shape) throws ZarrException {
237243
return read(offset, shape, false);
238244
}
239245

@@ -245,7 +251,7 @@ public ucar.ma2.Array read(final long[] offset, final int[] shape) throws ZarrEx
245251
*/
246252
@Nonnull
247253
public ucar.ma2.Array read(final boolean parallel) throws ZarrException {
248-
return read(new long[metadata().ndim()], Utils.toIntArray(metadata().shape), parallel);
254+
return read(new long[metadata().ndim()], metadata().shape, parallel);
249255
}
250256

251257
boolean chunkIsInArray(long[] chunkCoords) {
@@ -268,7 +274,7 @@ boolean chunkIsInArray(long[] chunkCoords) {
268274
* @throws ZarrException throws ZarrException if the requested data is outside the array's domain or if the read fails
269275
*/
270276
@Nonnull
271-
public ucar.ma2.Array read(final long[] offset, final int[] shape, final boolean parallel) throws ZarrException {
277+
public ucar.ma2.Array read(final long[] offset, final long[] shape, final boolean parallel) throws ZarrException {
272278
ArrayMetadata metadata = metadata();
273279
if (offset.length != metadata.ndim()) {
274280
throw new IllegalArgumentException("'offset' needs to have rank '" + metadata.ndim() + "'.");
@@ -288,7 +294,7 @@ public ucar.ma2.Array read(final long[] offset, final int[] shape, final boolean
288294
}
289295

290296
final ucar.ma2.Array outputArray = ucar.ma2.Array.factory(metadata.dataType().getMA2DataType(),
291-
shape);
297+
Utils.toIntArray(shape));
292298
Stream<long[]> chunkStream = Arrays.stream(IndexingUtils.computeChunkCoords(metadata.shape, chunkShape, offset, shape));
293299
if (parallel) {
294300
chunkStream = chunkStream.parallel();
@@ -311,18 +317,19 @@ public ucar.ma2.Array read(final long[] offset, final int[] shape, final boolean
311317
final String[] chunkKeys = metadata.chunkKeyEncoding().encodeChunkKey(chunkCoords);
312318
final StoreHandle chunkHandle = storeHandle.resolve(chunkKeys);
313319

314-
if (!chunkHandle.exists()) return;
315-
316320
if (codecPipeline.supportsPartialDecode()) {
317321
final ucar.ma2.Array chunkArray = codecPipeline.decodePartial(chunkHandle,
318322
Utils.toLongArray(chunkProjection.chunkOffset), chunkProjection.shape);
319323
MultiArrayUtils.copyRegion(chunkArray, new int[metadata.ndim()], outputArray,
320324
chunkProjection.outOffset, chunkProjection.shape
321325
);
322326
} else {
323-
MultiArrayUtils.copyRegion(readChunk(chunkCoords), chunkProjection.chunkOffset,
324-
outputArray, chunkProjection.outOffset, chunkProjection.shape
325-
);
327+
ByteBuffer chunkBytes = chunkHandle.read();
328+
if (chunkBytes != null) {
329+
MultiArrayUtils.copyRegion(codecPipeline.decode(chunkBytes), chunkProjection.chunkOffset,
330+
outputArray, chunkProjection.outOffset, chunkProjection.shape
331+
);
332+
}
326333
}
327334

328335
} catch (ZarrException e) {
@@ -340,7 +347,7 @@ public static final class ArrayAccessor {
340347
@Nullable
341348
long[] offset;
342349
@Nullable
343-
int[] shape;
350+
long[] shape;
344351
@Nonnull
345352
Array array;
346353

@@ -357,13 +364,13 @@ public ArrayAccessor withOffset(@Nonnull long... offset) {
357364

358365
@Nonnull
359366
public ArrayAccessor withShape(@Nonnull int... shape) {
360-
this.shape = shape;
367+
this.shape = Utils.toLongArray(shape);
361368
return this;
362369
}
363370

364371
@Nonnull
365372
public ArrayAccessor withShape(@Nonnull long... shape) {
366-
this.shape = Utils.toIntArray(shape);
373+
this.shape = shape;
367374
return this;
368375
}
369376

‎src/main/java/dev/zarr/zarrjava/core/codec/CodecPipeline.java‎

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,9 @@ ArrayBytesCodec getArrayBytesCodec() {
6868
return (ArrayBytesCodec) codec;
6969
}
7070
}
71-
throw new RuntimeException(
72-
"Unreachable because the existence of exactly 1 ArrayBytes codec is asserted upon construction.");
71+
throw new IllegalStateException(
72+
"No ArrayBytesCodec found in codec pipeline. This should never happen as the existence " +
73+
"of exactly 1 ArrayBytesCodec is validated during construction.");
7374
}
7475

7576
BytesBytesCodec[] getBytesBytesCodecs() {
@@ -158,18 +159,4 @@ public long computeEncodedSize(long inputByteLength, CoreArrayMetadata arrayMeta
158159
}
159160
return inputByteLength;
160161
}
161-
162-
public Array partialDecode(
163-
StoreHandle valueHandle, long[] offset, int[] shape,
164-
CoreArrayMetadata arrayMetadata
165-
) {
166-
return null; // TODO
167-
}
168-
169-
public ByteBuffer partialEncode(
170-
StoreHandle oldValueHandle, Array array, long[] offset, int[] shape,
171-
CoreArrayMetadata arrayMetadata
172-
) {
173-
return null; // TODO
174-
}
175162
}

‎src/main/java/dev/zarr/zarrjava/core/codec/core/BytesCodec.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ public ByteOrder getByteOrder() {
102102
case BIG:
103103
return ByteOrder.BIG_ENDIAN;
104104
default:
105-
throw new RuntimeException("Unreachable");
105+
throw new IllegalStateException("Unknown endian type: " + this);
106106
}
107107
}
108108
}

‎src/main/java/dev/zarr/zarrjava/store/BufferedZipStore.java‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,10 @@ public BufferedZipStore(@Nonnull StoreHandle underlyingStore, @Nonnull Store.Lis
5656
try {
5757
loadBuffer();
5858
} catch (IOException e) {
59-
throw new RuntimeException("Failed to load buffer from underlying store", e);
59+
throw StoreException.readFailed(
60+
underlyingStore.toString(),
61+
new String[]{},
62+
new IOException("Failed to load ZIP buffer from underlying store: " + underlyingStore, e));
6063
}
6164
}
6265

@@ -285,7 +288,10 @@ public void set(String[] keys, ByteBuffer bytes) {
285288
try {
286289
writeBuffer();
287290
} catch (IOException e) {
288-
throw new RuntimeException("Failed to flush buffer to underlying store after set operation", e);
291+
throw StoreException.writeFailed(
292+
underlyingStore.toString(),
293+
keys,
294+
new IOException("Failed to flush ZIP buffer to underlying store after set operation", e));
289295
}
290296
}
291297
}
@@ -297,7 +303,10 @@ public void delete(String[] keys) {
297303
try {
298304
writeBuffer();
299305
} catch (IOException e) {
300-
throw new RuntimeException("Failed to flush buffer to underlying store after delete operation", e);
306+
throw StoreException.deleteFailed(
307+
underlyingStore.toString(),
308+
keys,
309+
new IOException("Failed to flush ZIP buffer to underlying store after delete operation", e));
301310
}
302311
}
303312
}

‎src/main/java/dev/zarr/zarrjava/store/FilesystemStore.java‎

Lines changed: 54 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,13 @@ Path resolveKeys(String[] keys) {
3030
for (String key : keys) {
3131
newPath = newPath.resolve(key);
3232
}
33-
return newPath;
33+
Path absRoot = path.toAbsolutePath().normalize();
34+
Path absTarget = newPath.toAbsolutePath().normalize();
35+
36+
if (!absTarget.startsWith(absRoot)) {
37+
throw new IllegalArgumentException("Key resolves outside of store root: " + absTarget);
38+
}
39+
return newPath.normalize();
3440
}
3541

3642
@Override
@@ -43,8 +49,10 @@ public boolean exists(String[] keys) {
4349
public ByteBuffer get(String[] keys) {
4450
try {
4551
return ByteBuffer.wrap(Files.readAllBytes(resolveKeys(keys)));
46-
} catch (IOException e) {
52+
} catch (NoSuchFileException e) {
4753
return null;
54+
} catch (IOException e) {
55+
throw StoreException.readFailed(this.toString(), keys, e);
4856
}
4957
}
5058

@@ -64,8 +72,10 @@ public ByteBuffer get(String[] keys, long start) {
6472
byteChannel.read(bytes);
6573
bytes.rewind();
6674
return bytes;
67-
} catch (IOException e) {
75+
} catch (NoSuchFileException e) {
6876
return null;
77+
} catch (IOException e) {
78+
throw StoreException.readFailed(this.toString(), keys, e);
6979
}
7080
}
7181

@@ -84,8 +94,10 @@ public ByteBuffer get(String[] keys, long start, long end) {
8494
byteChannel.read(bytes);
8595
bytes.rewind();
8696
return bytes;
87-
} catch (IOException e) {
97+
} catch (NoSuchFileException e) {
8898
return null;
99+
} catch (IOException e) {
100+
throw StoreException.readFailed(this.toString(), keys, e);
89101
}
90102
}
91103

@@ -96,7 +108,10 @@ public void set(String[] keys, ByteBuffer bytes) {
96108
try {
97109
Files.createDirectories(keyPath.getParent());
98110
} catch (IOException e) {
99-
throw new RuntimeException(e);
111+
throw StoreException.writeFailed(
112+
this.toString(),
113+
keys,
114+
new IOException("Failed to create parent directories for path: " + keyPath.getParent(), e));
100115
}
101116
try (SeekableByteChannel channel = Files.newByteChannel(keyPath.toAbsolutePath(),
102117
StandardOpenOption.CREATE,
@@ -105,18 +120,25 @@ public void set(String[] keys, ByteBuffer bytes) {
105120
)) {
106121
channel.write(bytes);
107122
} catch (IOException e) {
108-
throw new RuntimeException(e);
123+
throw StoreException.writeFailed(
124+
this.toString(),
125+
keys,
126+
new IOException("Failed to write " + bytes.remaining() + " bytes to file: " + keyPath, e));
109127
}
110128
}
111129

112130
@Override
113131
public void delete(String[] keys) {
132+
Path keyPath = resolveKeys(keys);
114133
try {
115-
Files.delete(resolveKeys(keys));
134+
Files.delete(keyPath);
116135
} catch (NoSuchFileException e) {
117-
// ignore
136+
// ignore - file doesn't exist, which is the desired outcome
118137
} catch (IOException e) {
119-
throw new RuntimeException(e);
138+
throw StoreException.deleteFailed(
139+
this.toString(),
140+
keys,
141+
new IOException("Failed to delete file: " + keyPath, e));
120142
}
121143
}
122144

@@ -143,7 +165,10 @@ public Stream<String[]> list(String[] prefix) {
143165
.filter(Files::isRegularFile)
144166
.map(path -> pathToKeyArray(rootPath, path, prefix));
145167
} catch (IOException e) {
146-
throw new RuntimeException("Failed to list store content", e);
168+
throw StoreException.listFailed(
169+
this.toString(),
170+
prefix,
171+
new IOException("Failed to walk directory tree at: " + rootPath, e));
147172
}
148173
}
149174

@@ -156,7 +181,10 @@ public Stream<String> listChildren(String[] prefix) {
156181
try {
157182
return Files.list(rootPath).map(path -> path.getFileName().toString());
158183
} catch (IOException e) {
159-
throw new RuntimeException("Failed to list store children", e);
184+
throw StoreException.listFailed(
185+
this.toString(),
186+
prefix,
187+
new IOException("Failed to list directory contents at: " + rootPath, e));
160188
}
161189
}
162190

@@ -175,14 +203,12 @@ public String toString() {
175203
public InputStream getInputStream(String[] keys, long start, long end) {
176204
Path keyPath = resolveKeys(keys);
177205
try {
178-
if (!Files.exists(keyPath)) {
179-
return null;
180-
}
181206
InputStream inputStream = Files.newInputStream(keyPath);
182207
if (start > 0) {
183208
long skipped = inputStream.skip(start);
184209
if (skipped < start) {
185-
throw new IOException("Unable to skip to the desired start position.");
210+
throw new IOException("Unable to skip to position " + start +
211+
", only skipped " + skipped + " bytes in file: " + keyPath);
186212
}
187213
}
188214
if (end != -1) {
@@ -191,18 +217,28 @@ public InputStream getInputStream(String[] keys, long start, long end) {
191217
} else {
192218
return inputStream;
193219
}
220+
} catch (NoSuchFileException e) {
221+
return null;
194222
} catch (IOException e) {
195-
throw new RuntimeException(e);
223+
throw StoreException.readFailed(
224+
this.toString(),
225+
keys,
226+
new IOException("Failed to open input stream for file: " + keyPath +
227+
" (start: " + start + ", end: " + end + ")", e));
196228
}
197229
}
198230

199231
public long getSize(String[] keys) {
232+
Path keyPath = resolveKeys(keys);
200233
try {
201-
return Files.size(resolveKeys(keys));
234+
return Files.size(keyPath);
202235
} catch (NoSuchFileException e) {
203236
return -1;
204237
} catch (IOException e) {
205-
throw new RuntimeException(e);
238+
throw StoreException.readFailed(
239+
this.toString(),
240+
keys,
241+
new IOException("Failed to get file size for: " + keyPath, e));
206242
}
207243
}
208244
}

0 commit comments

Comments
 (0)