GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — src: fixup fs SyncCall to propagate errors correctly · nodejs/node@e144b69

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

src
test/parallel
- test-fs-writesync-crash.js

Expand file treeCollapse file tree

Open diff view settings

Collapse file

`‎src/node_file-inl.h‎`

Copy file name to clipboardExpand all lines: src/node_file-inl.h

+16-10Lines changed: 16 additions & 10 deletions

Original file line number	Diff line number	Diff line change
`@@ -344,23 +344,29 @@ FSReqBase* AsyncCall(Environment* env,`
`344`	`344`	`// creating an error in the C++ land.`
`345`	`345`	`// ctx must be checked using value->IsObject() before being passed.`
`346`	`346`	`template <typename Func, typename... Args>`
`347`		`-int SyncCall(Environment* env, v8::Local<v8::Value> ctx,`
`348`		`- FSReqWrapSync* req_wrap, const char* syscall,`
`349`		`- Func fn, Args... args) {`
	`347`	`+v8::Maybe<int> SyncCall(Environment* env,`
	`348`	`+ v8::Local<v8::Value> ctx,`
	`349`	`+ FSReqWrapSync* req_wrap,`
	`350`	`+ const char* syscall,`
	`351`	`+ Func fn,`
	`352`	`+ Args... args) {`
`350`	`353`	`env->PrintSyncTrace();`
`351`	`354`	`int err = fn(env->event_loop(), &(req_wrap->req), args..., nullptr);`
`352`	`355`	`if (err < 0) {`
`353`	`356`	`v8::Local<v8::Context> context = env->context();`
`354`	`357`	`v8::Local<v8::Object> ctx_obj = ctx.As<v8::Object>();`
`355`	`358`	`v8::Isolate* isolate = env->isolate();`
`356`		`- ctx_obj->Set(context,`
`357`		`- env->errno_string(),`
`358`		`- v8::Integer::New(isolate, err)).Check();`
`359`		`- ctx_obj->Set(context,`
`360`		`- env->syscall_string(),`
`361`		`- OneByteString(isolate, syscall)).Check();`
	`359`	`+ if (ctx_obj`
	`360`	`+ ->Set(context, env->errno_string(), v8::Integer::New(isolate, err))`
	`361`	`+ .IsNothing() \|\|`
	`362`	`+ ctx_obj`
	`363`	`+ ->Set(`
	`364`	`+ context, env->syscall_string(), OneByteString(isolate, syscall))`
	`365`	`+ .IsNothing()) {`
	`366`	`+ return v8::Nothing<int>();`
	`367`	`+ }`
`362`	`368`	`}`
`363`		`- return err;`
	`369`	`+ return v8::Just(err);`
`364`	`370`	`}`
`365`	`371`
`366`	`372`	`// Similar to SyncCall but throws immediately if there is an error.`

Collapse file

`‎src/node_file.cc‎`

Copy file name to clipboardExpand all lines: src/node_file.cc

+41-6Lines changed: 41 additions & 6 deletions

Original file line number	Diff line number	Diff line change
`@@ -2239,8 +2239,19 @@ static void OpenFileHandle(const FunctionCallbackInfo<Value>& args) {`
`2239`	`2239`	`CHECK_EQ(argc, 5);`
`2240`	`2240`	`FSReqWrapSync req_wrap_sync;`
`2241`	`2241`	`FS_SYNC_TRACE_BEGIN(open);`
`2242`		`- int result = SyncCall(env, args[4], &req_wrap_sync, "open",`
`2243`		`- uv_fs_open, *path, flags, mode);`
	`2242`	`+ int result;`
	`2243`	`+ if (!SyncCall(env,`
	`2244`	`+ args[4],`
	`2245`	`+ &req_wrap_sync,`
	`2246`	`+ "open",`
	`2247`	`+ uv_fs_open,`
	`2248`	`+ *path,`
	`2249`	`+ flags,`
	`2250`	`+ mode)`
	`2251`	`+ .To(&result)) {`
	`2252`	`+ // v8 error occurred while setting the context. propagate!`
	`2253`	`+ return;`
	`2254`	`+ }`
`2244`	`2255`	`FS_SYNC_TRACE_END(open);`
`2245`	`2256`	`if (result < 0) {`
`2246`	`2257`	`return; // syscall failed, no need to continue, error info is in ctx`
`@@ -2356,8 +2367,20 @@ static void WriteBuffer(const FunctionCallbackInfo<Value>& args) {`
`2356`	`2367`	`CHECK_EQ(argc, 7);`
`2357`	`2368`	`FSReqWrapSync req_wrap_sync;`
`2358`	`2369`	`FS_SYNC_TRACE_BEGIN(write);`
`2359`		`- int bytesWritten = SyncCall(env, args[6], &req_wrap_sync, "write",`
`2360`		`- uv_fs_write, fd, &uvbuf, 1, pos);`
	`2370`	`+ int bytesWritten;`
	`2371`	`+ if (!SyncCall(env,`
	`2372`	`+ args[6],`
	`2373`	`+ &req_wrap_sync,`
	`2374`	`+ "write",`
	`2375`	`+ uv_fs_write,`
	`2376`	`+ fd,`
	`2377`	`+ &uvbuf,`
	`2378`	`+ 1,`
	`2379`	`+ pos)`
	`2380`	`+ .To(&bytesWritten)) {`
	`2381`	`+ FS_SYNC_TRACE_END(write, "bytesWritten", 0);`
	`2382`	`+ return;`
	`2383`	`+ }`
`2361`	`2384`	`FS_SYNC_TRACE_END(write, "bytesWritten", bytesWritten);`
`2362`	`2385`	`args.GetReturnValue().Set(bytesWritten);`
`2363`	`2386`	`}`
`@@ -2515,8 +2538,20 @@ static void WriteString(const FunctionCallbackInfo<Value>& args) {`
`2515`	`2538`	`uv_buf_t uvbuf = uv_buf_init(buf, len);`
`2516`	`2539`	`FSReqWrapSync req_wrap_sync("write");`
`2517`	`2540`	`FS_SYNC_TRACE_BEGIN(write);`
`2518`		`- int bytesWritten = SyncCall(env, args[5], &req_wrap_sync, "write",`
`2519`		`- uv_fs_write, fd, &uvbuf, 1, pos);`
	`2541`	`+ int bytesWritten;`
	`2542`	`+ if (!SyncCall(env,`
	`2543`	`+ args[5],`
	`2544`	`+ &req_wrap_sync,`
	`2545`	`+ "write",`
	`2546`	`+ uv_fs_write,`
	`2547`	`+ fd,`
	`2548`	`+ &uvbuf,`
	`2549`	`+ 1,`
	`2550`	`+ pos)`
	`2551`	`+ .To(&bytesWritten)) {`
	`2552`	`+ FS_SYNC_TRACE_END(write, "bytesWritten", 0);`
	`2553`	`+ return;`
	`2554`	`+ }`
`2520`	`2555`	`FS_SYNC_TRACE_END(write, "bytesWritten", bytesWritten);`
`2521`	`2556`	`args.GetReturnValue().Set(bytesWritten);`
`2522`	`2557`	`}`

Collapse file

`‎src/node_file.h‎`

Copy file name to clipboardExpand all lines: src/node_file.h

+6-3Lines changed: 6 additions & 3 deletions

Original file line number	Diff line number	Diff line change
`@@ -512,9 +512,12 @@ inline FSReqBase* AsyncCall(Environment* env,`
`512`	`512`	`// creating an error in the C++ land.`
`513`	`513`	`// ctx must be checked using value->IsObject() before being passed.`
`514`	`514`	`template <typename Func, typename... Args>`
`515`		`-inline int SyncCall(Environment* env, v8::Local<v8::Value> ctx,`
`516`		`- FSReqWrapSync* req_wrap, const char* syscall,`
`517`		`- Func fn, Args... args);`
	`515`	`+inline v8::Maybe<int> SyncCall(Environment* env,`
	`516`	`+ v8::Local<v8::Value> ctx,`
	`517`	`+ FSReqWrapSync* req_wrap,`
	`518`	`+ const char* syscall,`
	`519`	`+ Func fn,`
	`520`	`+ Args... args);`
`518`	`521`
`519`	`522`	`// Similar to SyncCall but throws immediately if there is an error.`
`520`	`523`	`template <typename Predicate, typename Func, typename... Args>`

Collapse file

`‎test/parallel/test-fs-writesync-crash.js‎`

Copy file name to clipboard

+42Lines changed: 42 additions & 0 deletions

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,42 @@`
	`1`	`+'use strict';`
	`2`	`+`
	`3`	`+require('../common');`
	`4`	`+`
	`5`	`+const {`
	`6`	`+ writeSync,`
	`7`	`+ writeFileSync,`
	`8`	`+ chmodSync,`
	`9`	`+ openSync,`
	`10`	`+} = require('node:fs');`
	`11`	`+`
	`12`	`+const {`
	`13`	`+ throws,`
	`14`	`+} = require('node:assert');`
	`15`	`+`
	`16`	`+// If a file's mode change after it is opened but before it is written to,`
	`17`	`+// and the Object.prototype is manipulated to throw an error when the errno`
	`18`	`+// or fd property is set or accessed, then the writeSync call would crash`
	`19`	`+// the process. This test verifies that the error is properly propagated`
	`20`	`+// instead.`
	`21`	`+`
	`22`	`+const tmpdir = require('../common/tmpdir');`
	`23`	`+console.log(tmpdir.path);`
	`24`	`+tmpdir.refresh();`
	`25`	+const path = `${tmpdir.path}/foo`;
	`26`	`+writeFileSync(path, '');`
	`27`	`+`
	`28`	`+// Do this after calling tmpdir.refresh() or that call will fail`
	`29`	`+// before we get to the part we want to test.`
	`30`	`+const error = new Error();`
	`31`	`+Object.defineProperty(Object.prototype, 'errno', {`
	`32`	`+ __proto__: null,`
	`33`	`+ set() {`
	`34`	`+ throw error;`
	`35`	`+ },`
	`36`	`+ get() { return 0; }`
	`37`	`+});`
	`38`	`+`
	`39`	`+const fd = openSync(path);`
	`40`	`+chmodSync(path, 0o600);`
	`41`	`+`
	`42`	`+throws(() => writeSync(fd, 'test'), error);`

0 commit comments

Comments

0 (0)

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.