Skip to content

Commit 22c6d55

Browse files
authored
[WasmFS] Fix an edge case in wasmfs_fetch for reads far past end of file (#23669)
Use the minimum of the requested last chunk and the actual last chunk when accessing chunked data.
1 parent 7aabe38 commit 22c6d55

File tree

2 files changed

+17
-6
lines changed

2 files changed

+17
-6
lines changed

src/lib/libwasmfs_fetch.js

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,21 +39,19 @@ addToLibrary({
3939
offset ??= 0;
4040
len ??= chunkSize;
4141
// In which chunk does the seeked range start? E.g., 5-14 with chunksize 8 will start in chunk 0.
42-
var firstChunk = (offset / chunkSize) | 0;
43-
// In which chunk does the seeked range end? E.g., 5-14 with chunksize 8 will end in chunk 1, as will 5-16 (since byte 16 isn't requested).
44-
// This will always give us a chunk >= firstChunk since len > 0.
45-
var lastChunk = ((offset+len-1) / chunkSize) | 0;
4642
if (!(file in wasmFS$JSMemoryRanges)) {
4743
var fileInfo = await fetch(url, {method:'HEAD', headers:{'Range': 'bytes=0-'}});
4844
if (fileInfo.ok &&
4945
fileInfo.headers.has('Content-Length') &&
5046
fileInfo.headers.get('Accept-Ranges') == 'bytes' &&
5147
(parseInt(fileInfo.headers.get('Content-Length'), 10) > chunkSize*2)) {
48+
var size = parseInt(fileInfo.headers.get('Content-Length'), 10);
5249
wasmFS$JSMemoryRanges[file] = {
53-
size: parseInt(fileInfo.headers.get('Content-Length'), 10),
50+
size,
5451
chunks: [],
5552
chunkSize: chunkSize
5653
};
54+
len = Math.min(len, size-offset+1) | 0;
5755
} else {
5856
// may as well/forced to download the whole file
5957
var wholeFileReq = await fetch(url);
@@ -70,6 +68,10 @@ addToLibrary({
7068
return Promise.resolve();
7169
}
7270
}
71+
var firstChunk = (offset / chunkSize) | 0;
72+
// In which chunk does the seeked range end? E.g., 5-14 with chunksize 8 will end in chunk 1, as will 5-16 (since byte 16 isn't requested).
73+
// This will always give us a chunk >= firstChunk since len > 0.
74+
var lastChunk = ((offset+len-1) / chunkSize) | 0;
7375
var allPresent = true;
7476
var i;
7577
// Do we have all the chunks already? If so, we don't need to do any fetches.
@@ -120,7 +122,10 @@ addToLibrary({
120122

121123
// read/getSize fetch the data, then forward to the parent class.
122124
read: async (file, buffer, length, offset) => {
123-
if (length == 0) {
125+
// This function assumes that offset is non-negative and length is positive.
126+
// C read() doesn't take an offset and so doesn't have to deal with the former situation,
127+
// and if the length is 0 or the offset is negative there's no reasonable read we can make.
128+
if (offset < 0 || length <= 0) {
124129
return 0;
125130
}
126131
try {
@@ -129,6 +134,11 @@ addToLibrary({
129134
return failedResponse.status === 404 ? -{{{ cDefs.ENOENT }}} : -{{{ cDefs.EBADF }}};
130135
}
131136
var fileInfo = wasmFS$JSMemoryRanges[file];
137+
length = Math.min(length, fileInfo.size-offset+1) | 0;
138+
// As above, we check the length just in case offset was beyond size and length is now negative.
139+
if (length <= 0) {
140+
return 0;
141+
}
132142
var chunks = fileInfo.chunks;
133143
var chunkSize = fileInfo.chunkSize;
134144
var firstChunk = (offset / chunkSize) | 0;

test/wasmfs/wasmfs_fetch.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,7 @@ void test_small_chunks() {
171171
buf[size] = 0;
172172
printf("buf %s\n",buf);
173173
assert(strcmp(buf, "hello") == 0);
174+
assert(read(fd, buf+size-1, 1024) == 1);
174175

175176
assert(close(fd) == 0);
176177

0 commit comments

Comments
 (0)