java client: fail cleanly when a chunk read returns short or empty data (#10269)

* java client: fail cleanly when a chunk read returns short or empty data

readChunkView copied chunkView.size bytes out of the fetched chunk without
checking the fetched length, so an empty body (seen transiently right after an
append) threw an opaque IndexOutOfBoundsException. Retry empty chunk fetches and
bound the copy to the data actually returned.

* overflow-safe bounds check and null guard on fetched chunk data
This commit is contained in:
Chris Lu
2026-07-08 01:50:42 -07:00
committed by GitHub
parent 0ae1fdcad2
commit 24fb7e47f3
2 changed files with 44 additions and 2 deletions
@@ -111,10 +111,18 @@ public class SeaweedRead {
}
int len = (int) chunkView.size - (int) (startOffset - chunkView.logicOffset);
int srcOffset = (int) (startOffset - chunkView.logicOffset + chunkView.offset);
LOG.debug("readChunkView fid:{} chunkData.length:{} chunkView.offset:{} chunkView[{};{}) startOffset:{}",
chunkView.fileId, chunkData.length, chunkView.offset, chunkView.logicOffset,
chunkView.logicOffset + chunkView.size, startOffset);
buf.put(chunkData, (int) (startOffset - chunkView.logicOffset + chunkView.offset), len);
// The fetched chunk can be shorter than the view claims when a read briefly
// returns an empty body (seen right after an append). Fail clearly instead of
// letting ByteBuffer.put throw an opaque IndexOutOfBoundsException.
if (srcOffset < 0 || len < 0 || srcOffset > chunkData.length - len) {
throw new IOException("chunk " + chunkView.fileId + " returned " + chunkData.length
+ " bytes, need [" + srcOffset + "," + ((long) srcOffset + len) + ")");
}
buf.put(chunkData, srcOffset, len);
return len;
}
@@ -128,7 +136,15 @@ public class SeaweedRead {
for (FilerProto.Location location : locations.getLocationsList()) {
String url = filerClient.getChunkUrl(chunkView.fileId, location.getUrl(), location.getPublicUrl());
try {
data = doFetchOneFullChunkData(chunkView, url);
byte[] fetched = doFetchOneFullChunkData(chunkView, url);
// An empty (or null) body for a chunk we are reading from is never
// valid; treat it as a transient failure so the retry loop tries
// another location and backs off, rather than caching an empty chunk.
if (fetched == null || fetched.length == 0) {
lastException = new IOException("empty or null response for chunk " + chunkView.fileId + " from " + url);
continue;
}
data = fetched;
lastException = null;
break;
} catch (IOException ioe) {
@@ -4,12 +4,38 @@ import org.junit.Assert;
import org.junit.Test;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.List;
import java.util.Random;
public class SeaweedReadTest {
// A chunk fetch that briefly returns an empty body (observed right after an
// append) must not overrun the buffer with an opaque IndexOutOfBoundsException.
@Test
public void testReadChunkShorterThanViewFailsCleanly() throws IOException {
String fileId = "7,readchunkshort01";
SeaweedRead.volumeIdCache.setLocations("7",
FilerProto.Locations.newBuilder()
.addLocations(FilerProto.Location.newBuilder().setUrl("localhost:0").build())
.build());
// seed the chunk cache with an empty body so the fetch is skipped
SeaweedRead.chunkCache.setChunk(fileId, new byte[0]);
List<SeaweedRead.VisibleInterval> visibles = new ArrayList<>();
visibles.add(new SeaweedRead.VisibleInterval(0, 175, fileId, 1L, 0, true, null, false));
ByteBuffer buf = ByteBuffer.allocate(175);
try {
SeaweedRead.read(null, visibles, 0, buf, 175);
Assert.fail("expected IOException for a chunk shorter than its view");
} catch (IOException e) {
Assert.assertTrue(e.getMessage(), e.getMessage().contains(fileId));
Assert.assertTrue(e.getMessage(), e.getMessage().contains("[0,175)"));
}
}
@Test
public void testNonOverlappingVisibleIntervals() throws IOException {
List<FilerProto.FileChunk> chunks = new ArrayList<>();