FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

[v14.x backport] fs: fix write methods param validation and docs by aduh95 · Pull Request #42603 · nodejs/node · GitHub

/ node Public

[v14.x backport] fs: fix write methods param validation and docs - #42603

Closed
aduh95 wants to merge 2 commits into
nodejs:v14.x-stagingfrom
aduh95:backport-fs-abort-fix
Closed

[v14.x backport] fs: fix write methods param validation and docs#42603
aduh95 wants to merge 2 commits into
nodejs:v14.x-stagingfrom
aduh95:backport-fs-abort-fix

Conversation

aduh95 commented Apr 4, 2022

Copy link
Copy Markdown
Contributor

Backport of #42588 and #41677. This fixes a bug that makes the process crash if a non-buffer object was passed to FileHandle.prototype.write:

$ node -e 'console.log(process.version);fs.promises.open("/dev/null").then(fh => fh.write({ toString() { return "str\n" } }))'
v14.19.1
/usr/local/opt/node@14/bin/node[13975]: ../src/string_bytes.cc:319:static size_t node::StringBytes::Write(v8::Isolate *, char *, size_t, Local<v8::Value>, enum encoding, int *): Assertion `val->IsString() == true' failed.
 1: 0x1000a6e4a node::Abort() [/usr/local/Cellar/node@14/14.19.1/bin/node]
 2: 0x1000a6cd3 node::AppendExceptionLine(node::Environment*, v8::Local<v8::Value>, v8::Local<v8::Message>, node::ErrorHandlingMode) [/usr/local/Cellar/node@14/14.19.1/bin/node]
 3: 0x1001485c0 node::StringBytes::Write(v8::Isolate*, char*, unsigned long, v8::Local<v8::Value>, node::encoding, int*) [/usr/local/Cellar/node@14/14.19.1/bin/node]
 4: 0x1000b69f8 node::fs::WriteString(v8::FunctionCallbackInfo<v8::Value> const&) [/usr/local/Cellar/node@14/14.19.1/bin/node]
 5: 0x100205198 v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) [/usr/local/Cellar/node@14/14.19.1/bin/node]
 6: 0x1002048a1 v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) [/usr/local/Cellar/node@14/14.19.1/bin/node]
 7: 0x100204012 v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*) [/usr/local/Cellar/node@14/14.19.1/bin/node]
 8: 0x1007ca639 Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit [/usr/local/Cellar/node@14/14.19.1/bin/node]
 9: 0x1007610a2 Builtins_InterpreterEntryTrampoline [/usr/local/Cellar/node@14/14.19.1/bin/node]
10: 0x1007610a2 Builtins_InterpreterEntryTrampoline [/usr/local/Cellar/node@14/14.19.1/bin/node]
11: 0x10075afd9 Builtins_ArgumentsAdaptorTrampoline [/usr/local/Cellar/node@14/14.19.1/bin/node]
[1]    13975 abort      /usr/local/opt/node@14/bin/node -e
$ out/Release/node -e 'console.log(process.version);fs.promises.open("/dev/null").then(fh => fh.write({ toString() { return "str\n" } }))'
v14.19.2-pre
(node:16354) UnhandledPromiseRejectionWarning: TypeError [ERR_INVALID_ARG_TYPE]: The "buffer" argument must be of type string or an instance of Buffer, TypedArray, or DataView. Received an instance of Object
    at write (internal/fs/promises.js:464:3)
    at fsCall (internal/fs/promises.js:243:18)
    at FileHandle.write (internal/fs/promises.js:160:12)
    at [eval]:1:74
(Use `node --trace-warnings ...` to show where the warning was created)
(node:16354) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 3)
(node:16354) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
(node:16354) Warning: Closing file descriptor 23 on garbage collection
(node:16354) [DEP0137] DeprecationWarning: Closing a FileHandle object on garbage collection is deprecated. Please close FileHandle objects explicitly using FileHandle.prototype.close(). In the future, an error will be thrown if a file descriptor is closed during garbage collection.

PR-URL: nodejs#42588
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
nodejs-github-bot added fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. v14.x labels Apr 4, 2022
The FS docs wrongfully indicated support for passing object with an own
`toString` function property to `FileHandle.prototype.appendFile`,
`FileHandle.prototype.writeFile`, `FileHandle.prototype.write`,
`fsPromises.writeFile`, and `fs.writeSync`. This commit fixes that, and
adds some test to ensure the actual behavior is aligned with the docs.
It also fixes a bug that makes the process crash if a non-buffer object
was passed to `FileHandle.prototype.write`.

Refs: nodejs#34993
PR-URL: nodejs#41677
Refs: nodejs#41666
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 force-pushed the backport-fs-abort-fix branch from 02018d8 to 18ac4af Compare April 4, 2022 12:12
aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 28, 2022
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 28, 2022

Copy link
Copy Markdown
Collaborator

aduh95 mentioned this pull request Apr 28, 2022

juanarbol left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Apr 28, 2022
edited by juanarbol
Loading

Copy link
Copy Markdown
Collaborator

juanarbol pushed a commit that referenced this pull request May 1, 2022
PR-URL: #42588
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Backport-PR-URL: #42603
juanarbol pushed a commit that referenced this pull request May 1, 2022
The FS docs wrongfully indicated support for passing object with an own
`toString` function property to `FileHandle.prototype.appendFile`,
`FileHandle.prototype.writeFile`, `FileHandle.prototype.write`,
`fsPromises.writeFile`, and `fs.writeSync`. This commit fixes that, and
adds some test to ensure the actual behavior is aligned with the docs.
It also fixes a bug that makes the process crash if a non-buffer object
was passed to `FileHandle.prototype.write`.

Refs: #34993
PR-URL: #41677
Refs: #41666
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Backport-PR-URL: #42603

Copy link
Copy Markdown
Member

Landed in 75302d3 and 77ba012 🎉

juanarbol closed this May 2, 2022
aduh95 deleted the backport-fs-abort-fix branch May 2, 2022 18:48
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants


Back | FazBrowse Home | New Git URL