PR-URL: https://github.com/nodejs/node/pull/18642
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
The ctx.error is supposed to be handled in fs.readlinkSync,
but was handled in fs.symlinkSync by mistake.
Also fix the error number check in readlink to be consistent
with SYNC_CALL.
PR-URL: https://github.com/nodejs/node/pull/18548
Refs: https://github.com/nodejs/node/pull/18348
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
- Throw getPathFromURL() and nullCheck() errors synchronously instead
of deferring them to the next tick, since we already throw
validatePath() errors synchronously.
- Merge nullCheck() into validatePath()
- Never throws in `fs.exists()`, instead, invoke the callback with
false, or emit a warning when the callback is not a function.
This is to bring it inline with fs.existsSync(), which never throws.
- Updates the comment of rethrow()
- Throw ERR_INVALID_ARG_VALUE for null checks
PR-URL: https://github.com/nodejs/node/pull/18308
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit moves error creation helpers scattered around
under lib/ into lib/internal/errors.js in the hope of being clearer
about the differences of errors that we throw into the user land.
- Move util._errnoException and util._exceptionWithHostPort
into internal/errors.js and simplify their logic so it's
clearer what the properties these helpers create.
- Move the errnoException helper in dns.js to internal/errors.js
into internal/errors.js and rename it to dnsException. Simplify
it's logic so it no longer calls errnoException and skips
the unnecessary argument checks.
PR-URL: https://github.com/nodejs/node/pull/18546
Reviewed-By: James M Snell <jasnell@gmail.com>
We cannot make uvException a proper class due to compatibility
reasons for now, so there is no need to call new since
it only returns a newly-created Error.
PR-URL: https://github.com/nodejs/node/pull/18546
Reviewed-By: James M Snell <jasnell@gmail.com>
Added a test to ensure input validation for FD and mode for fs.fchmod.
Removed check for values lower than 0 for `mode` as it's already checked
by `validateUint32`.
PR-URL: https://github.com/nodejs/node/pull/18217
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Initial set of fs.promises APIs with documentation and one
benchmark.
PR-URL: https://github.com/nodejs/node/pull/18297
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Problem:
Node implements fs.readFile as:
- a call to stat, then
- a C++ -> libuv request to read the entire file using the stat size
Why is this bad?
The effect is to place on the libuv threadpool a potentially-large
read request, occupying the libuv thread until it completes.
While readFile certainly requires buffering the entire file contents,
it can partition the read into smaller buffers
(as is done on other read paths)
along the way to avoid threadpool exhaustion.
If the file is relatively large or stored on a slow medium, reading
the entire file in one shot seems particularly harmful,
and presents a possible DoS vector.
Solution:
Partition the read into multiple smaller requests.
Considerations:
1. Correctness
I don't think partitioning the read like this raises
any additional risk of read-write races on the FS.
If the application is concurrently readFile'ing and modifying the file,
it will already see funny behavior. Though libuv uses preadv where
available, this doesn't guarantee read atomicity in the presence of
concurrent writes.
2. Performance
Downside: Partitioning means that a single large readFile will
require into many "out and back" requests to libuv,
introducing overhead.
Upside: In between each "out and back", other work pending on the
threadpool can take a turn.
In short, although partitioning will slow down a large request,
it will lead to better throughput if the threadpool is handling
more than one type of request.
Fixes: https://github.com/nodejs/node/issues/17047
PR-URL: https://github.com/nodejs/node/pull/17054
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
lchown and lchownSync were opening file descriptors without
closing them. Looks like it has been that way for 7 years.
Does anyone actually use these functions?
PR-URL: https://github.com/nodejs/node/pull/18329
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
* Collect the error context in both JS and C++, then throw
the error in JS
* Test that the errors thrown from fs.close and fs.closeSync
includes the correct error code, error number and syscall
properties
PR-URL: https://github.com/nodejs/node/pull/17338
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
- Simplify the SyncCall template function, only collect error
number and syscall in the C++ layer and collect the rest of context
in JS for flexibility.
- Remove the stringFromPath JS helper now that the unprefixed path is
directly put into the context before the binding is invoked with the
prefixed path.
- Validate more properties in fs.access tests.
PR-URL: https://github.com/nodejs/node/pull/17338
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
There two similar error codes in lib: "ERR_VALUE_OUT_OF_RANGE"
and "ERR_OUT_OF_RANGE". This change is to reduce them into
"ERR_VALUE_OUT_OF_RANGE"
Fixes: https://github.com/nodejs/node/issues/17603
PR-URL: https://github.com/nodejs/node/pull/17648
Fixes: https://github.com/nodejs/node/issues/17603
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
- Migrate the type check of path to ERR_INVALID_ARG_TYPE
- Add template counterparts of ASYNC_CALL, ASYNC_DEST_CALL,
SYNC_CALL, SYNC_DEST_CALL
- Port StringFromPath and UVException to JavaScript
- Migrate the access binding to collect the error context in C++,
then throw the error in JS
PR-URL: https://github.com/nodejs/node/pull/17160
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Replaced _readableState.highWaterMark with a .readableHighWaterMark
getter and _writableState.highWaterMark with a .writableHighWaterMark
getter.
The getters are non-enumerable because they break some prototype
manipulation that happen in the ecosystem.
Ref: https://github.com/nodejs/node/issues/445.
PR-URL: https://github.com/nodejs/node/pull/12860
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
