Skip to content

Commit c8a4a8b

Browse files
committed
fs: only back Blobs with regular files
FdEntry::Create() accepted any path that stat'd, so openAsBlob() handed out a Blob for directories and fifos. Reading a directory Blob failed later with NotReadableError and code 0, hiding the EISDIR. A fifo was worse: opening it with no writer blocked the main thread, so the event loop stopped. Only a regular file can be read through the file descriptor this entry uses, so reject everything else, reporting EISDIR for directories and EINVAL otherwise.
1 parent addcba2 commit c8a4a8b

2 files changed

Lines changed: 34 additions & 2 deletions

File tree

‎src/dataqueue/queue.cc‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -859,8 +859,6 @@ class DataQueueEntry : public EntryImpl {
859859
// results if the file just happens to get modified.
860860
class FdEntry final : public EntryImpl {
861861
// TODO(@jasnell, @flakey5):
862-
// * This should only allow reading from regular files. No directories, no
863-
// pipes, etc.
864862
// * The reader should support accepting the buffer(s) from the pull, if any.
865863
// It should
866864
// only allocate a managed buffer if the pull doesn't provide any.
@@ -882,6 +880,16 @@ class FdEntry final : public EntryImpl {
882880
return nullptr;
883881
}
884882

883+
// A file-backed Blob is read through a regular file descriptor, so only
884+
// regular files are usable here. A directory read fails with EISDIR, and
885+
// opening a fifo with no writer blocks the thread doing the open, which
886+
// for this entry is the main thread.
887+
const uint64_t type = req.statbuf.st_mode & S_IFMT;
888+
if (type != S_IFREG) {
889+
if (status != nullptr) *status = type == S_IFDIR ? UV_EISDIR : UV_EINVAL;
890+
return nullptr;
891+
}
892+
885893
return std::make_unique<FdEntry>(
886894
env, std::move(buf), req.statbuf, 0, req.statbuf.st_size);
887895
}

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ const {
1212
unlink
1313
} = require('fs/promises');
1414
const { Blob } = require('buffer');
15+
const { spawnSync } = require('child_process');
1516

1617
const tmpdir = require('../common/tmpdir');
1718
const testfile = tmpdir.resolve('test-file-backed-blob.txt');
@@ -142,6 +143,29 @@ writeFileSync(testfile5, '');
142143
});
143144
})().then(common.mustCall());
144145

146+
(async () => {
147+
// Only a regular file can back a Blob: a directory read fails with EISDIR,
148+
// and a fifo with no writer would block the thread doing the open.
149+
await assert.rejects(async () => openAsBlob(tmpdir.path), {
150+
code: 'EISDIR',
151+
syscall: 'stat',
152+
path: tmpdir.path,
153+
});
154+
})().then(common.mustCall());
155+
156+
if (!common.isWindows) {
157+
const fifo = tmpdir.resolve('test-file-backed-blob.fifo');
158+
if (spawnSync('mkfifo', [fifo]).status === 0) {
159+
(async () => {
160+
await assert.rejects(async () => openAsBlob(fifo), {
161+
code: 'EINVAL',
162+
syscall: 'stat',
163+
path: fifo,
164+
});
165+
})().then(common.mustCall());
166+
}
167+
}
168+
145169
(async () => {
146170
// We currently do not allow File-backed blobs to be cloned or transferred
147171
// across worker threads. This is largely because the underlying FdEntry

0 commit comments

Comments
 (0)