The goal is to replace `dirent.path` using a name that's less likely to
create confusion.
`dirent.path` value has not been stable, moving it to a different
property name should avoid breaking some upgrading user expectations.
PR-URL: https://github.com/nodejs/node/pull/50976
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
PR-URL: https://github.com/nodejs/node/pull/49884
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/50769
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Qingyu Deng <i@ayase-lab.com>
The permission model's security guarantees fall apart in the presence of
relative symbolic links. When an application attempts to create a
relative symlink, the permission model currently resolves the relative
path into an absolute path based on the process's current working
directory, checks whether the process has the relevant permissions, and
then creates the symlink using the absolute target path. This behavior
is plainly incorrect for two reasons:
1. The target path should never be resolved relative to the current
working directory. If anything, it should be resolved relative to the
symlink's location. (Of course, there is one insane exception to this
rule: on Windows, each process has a current working directory per
drive, and symlinks can be created with a target path relative to the
current working directory of a specific drive. In that case, the
relative path will be resolved relative to the current working
directory for the respective drive, and the symlink will be created
on disk with the resulting absolute path. Other relative symlinks
will be stored as-is.)
2. Silently creating an absolute symlink when the user requested a
relative symlink is wrong. The user may (or may not) rely on the
symlink being relative. For example, npm heavily relies on relative
symbolic links such that node_modules directories can be moved around
without breaking.
Because we don't know the user's intentions, we don't know if creating
an absolute symlink instead of a relative symlink is acceptable. This
patch prevents the faulty behavior by not (incorrectly) resolving
relative symlink targets when the permission model is enabled, and by
instead simply refusing the create any relative symlinks.
The fs APIs accept Uint8Array objects for paths to be able to handle
arbitrary file name charsets, however, checking whether such an object
represents a relative part in a reliable and portable manner is tricky.
Other parts of the permission model incorrectly convert such objects to
strings and then back to an Uint8Array (see 1f64147eb6),
however, for now, this bug fix will simply throw on non-string symlink
targets when the permission model is enabled. (The permission model
already breaks existing applications in various ways, so this shouldn't
be too dramatic.)
PR-URL: https://github.com/nodejs/node/pull/49156
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/50448
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Deokjin Kim <deokjin81.kim@gmail.com>
This commit adds a 'flush' option to the createWriteStream()
family of functions.
Refs: https://github.com/nodejs/node/issues/49886
PR-URL: https://github.com/nodejs/node/pull/50093
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/49928
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Previously to throw errors from C++ land, sync versions of the fs
were created by copying C++ code from the original implementation
and moving JS code to a separate file. This can lead to several
problems:
1. By moving code to a new file for the sake of moving, it would
be harder to use git blame to trace changes and harder to backport
changes to older branches.
2. Scattering the async and sync versions of fs methods in
different files makes it harder to keep them in sync and
share code in the prologues and epilogues.
3. Having two copies of code doing almost the same thing results
in duplication and can be prone to out-of-sync problems when the
prologue and epilogue get updated.
4. There is a minor cost to startup in adding an additional file.
This can add up even with the help of snapshots.
This patch moves the JS code back to lib/fs.js to stop 1, 2 & 4
and introduces C++ helpers SyncCallAndThrowIf() and
SyncCallAndThrowOnError() so that the original implementations
can be easily tweaked to allow throwing from C++ and stop 3.
PR-URL: https://github.com/nodejs/node/pull/49913
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
This prohibits invalid values (< -1 and non-integers) and
allows `filehandle.read()` to handle position up to `2n ** 63n - 1n`
PR-URL: https://github.com/nodejs/node/pull/42835
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/48828
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
The binding data holds references to the AliasedBuffers directly
from their wrappers which already ensures that the AliasedBuffers
won't be accessed when the wrappers are GC'ed. So we can just
make the global references to the AliasedBuffers weak. This way
we can simply deserialize the typed arrays when deserialize the
binding data and avoid the extra Object::Set() calls. It also
eliminates the caveat in the JS land where aliased buffers must
be dynamically read from the binding.
PR-URL: https://github.com/nodejs/node/pull/47354
Refs: https://github.com/nodejs/node/issues/47353
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
