Skip to content

Commit bef95d2

Browse files
jleedevmarco-ippolito
authored andcommitted
src: fix slice of slice of file-backed Blob
The value for `new_end` was wrong: While the members `start_` and `end_` refer to the entire length of the file, the parameters `start` and `end` are relative to the current slice. The new end would apparently have the current start_ subtracted from it, and the length would possibly overflow when the FdEntry is asked for its size or when get_reader is called, resulting in a subslice which extends past the current slice, which shouldn't be possible. Add a CHECK if this happens, rather than returning data outside the current slice. There aren't any C++ tests for FdEntry, and on the javascript side there isn't a way to ask the blob handle for its nominal size. That size could be a large uint64, which gets converted to int64 to when FileHandle::new is called, which interprets a negative length as unlimited. Fixes: #53908 PR-URL: #53972 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 4fb7801 commit bef95d2

File tree

2 files changed

+14
-2
lines changed

2 files changed

+14
-2
lines changed

src/dataqueue/queue.cc

+4-2
Original file line numberDiff line numberDiff line change
@@ -806,7 +806,9 @@ class FdEntry final : public EntryImpl {
806806
path_(std::move(path_)),
807807
stat_(stat),
808808
start_(start),
809-
end_(end) {}
809+
end_(end) {
810+
CHECK_LE(start, end);
811+
}
810812

811813
std::shared_ptr<DataQueue::Reader> get_reader() override {
812814
return ReaderImpl::Create(this);
@@ -817,7 +819,7 @@ class FdEntry final : public EntryImpl {
817819
uint64_t new_start = start_ + start;
818820
uint64_t new_end = end_;
819821
if (end.has_value()) {
820-
new_end = std::min(end.value(), end_);
822+
new_end = std::min(end.value() + start_, end_);
821823
}
822824

823825
CHECK(new_start >= start_);

test/parallel/test-blob-file-backed.js

+10
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,16 @@ writeFileSync(testfile5, '');
8686

8787
const res1 = blob.slice(995, 1005);
8888
strictEqual(await res1.text(), data.slice(995, 1005));
89+
90+
// Refs: https://github.com/nodejs/node/issues/53908
91+
for (const res2 of [
92+
blob.slice(995, 1005).slice(),
93+
blob.slice(995).slice(0, 10),
94+
blob.slice(0, 1005).slice(995),
95+
]) {
96+
strictEqual(await res2.text(), data.slice(995, 1005));
97+
}
98+
8999
await unlink(testfile2);
90100
})().then(common.mustCall());
91101

0 commit comments

Comments
 (0)