Repository navigation
Conversation
openAsBlob() and openAsBlobSync() stored the caller path and reopened it on every read. A relative path was resolved against the cwd again, so process.chdir() could read a different file of the same size and mtime, or fail the read. Pin the directory the path was opened against when the blob is created, without lexically collapsing ".." so symlink traversal stays the same. Signed-off-by: Ihar Ivaniuk <ihar.o.ivaniuk@gmail.com>
nodejs-github-bot
added
c++
|
Welcome to Node.js, and thank you for your first contribution! Before review, please take a moment to read:
Please make sure every commit is signed off. For a first pull request, GitHub Actions require collaborator approval and Jenkins CI must be started by a collaborator or triager, so an initial wait is normal. |
| return buf; | ||
| } | ||
|
|
||
| #ifdef _WIN32 |
There was a problem hiding this comment.
On Windows, could we just store the path already resolved by ToNamespacedPath() instead of adding all this?
| const { pathToFileURL } = require('node:url'); | ||
|
|
||
| if (process.argv[2] !== 'child') { | ||
| const child = spawnSync(process.execPath, [__filename, 'child'], { |
There was a problem hiding this comment.
Is this to avoid the worker thread from the test harness? You can skip instead. It's not necessary to test this under worker mode.
if (!common.isMainThread)
common.skip('process.chdir is not available in Workers');| assert.strictEqual(child.status, 0, output); | ||
| } else { | ||
| (async () => { | ||
| const root = fs.mkdtempSync(path.join(os.tmpdir(), 'openasblob-chdir-')); |
There was a problem hiding this comment.
The convention is to use common/tmpdir
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #67346 +/- ##
==========================================
+ Coverage 90.68% 90.69% +0.01%
==========================================
Files 797 797
Lines 279574 279605 +31
Branches 53778 53790 +12
==========================================
+ Hits 253525 253593 +68
+ Misses 16344 16297 -47
- Partials 9705 9715 +10
🚀 New features to boost your workflow:
|
openAsBlob() and openAsBlobSync() stored the caller path and reopened it on every read. A relative path was resolved against the cwd again, so process.chdir() could read a different file of the same size and mtime, or fail the read. Pin the directory the path was opened against when the blob is created, without lexically collapsing ".." so symlink traversal stays the same.
Verified on Linux x64 by building this tree. The new test failed on the
unfixed binary (
'from-b' !== 'from-a') and passed after the rebuild.Also passed:
test-blob-file-backed,test-fs-openAsBlobSync,test-vfs-fs-openAsBlob,test-blob-file-webidl,test-permission-fs-read, andtest-permission-fs-supported.cpplint, the include check, clang-format, and ESLint were clean.
Windows and macOS were not run.
A coding agent prepared the change. The build and the test results above
were run locally.
Fixes: #67282