Skip to content

Commit 28ccbf1

Browse files
committed
fs: fix handling of uv_stat_t fields
`FChown` and `Chown` test that the `uid` and `gid` parameters they receive are unsigned integers, but `Stat()` and `FStat()` would return the corresponding fields of `uv_stat_t` as signed integers. Applications which pass those these values directly to `Chown` may fail (e.g. for `nobody` on OS X, who has an `uid` of `-2`, see e.g. nodejs/node-v0.x-archive#5890). This patch changes the `Integer::New()` call for `uid` and `gid` to `Integer::NewFromUnsigned()`. All other fields are kept as they are, for performance, but strictly speaking the respective sizes of those fields aren’t specified, either. Ref: npm/npm#13918
1 parent 7f2c9ba commit 28ccbf1

File tree

1 file changed

+20
-10
lines changed

1 file changed

+20
-10
lines changed

src/node_file.cc

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -433,34 +433,44 @@ Local<Value> BuildStatsObject(Environment* env, const uv_stat_t* s) {
433433
// crash();
434434
// }
435435
//
436-
// We need to check the return value of Integer::New() and Date::New()
436+
// We need to check the return value of Number::New() and Date::New()
437437
// and make sure that we bail out when V8 returns an empty handle.
438438

439-
// Integers.
439+
// Unsigned integers. It does not actually seem to be specified whether
440+
// uid and gid are unsigned or not, but in practice they are unsigned,
441+
// and Node’s (F)Chown functions do check their arguments for unsignedness.
440442
#define X(name) \
441-
Local<Value> name = Integer::New(env->isolate(), s->st_##name); \
443+
Local<Value> name = Integer::NewFromUnsigned(env->isolate(), s->st_##name); \
442444
if (name.IsEmpty()) \
443-
return handle_scope.Escape(Local<Object>()); \
445+
return Local<Object>(); \
444446

445-
X(dev)
446-
X(mode)
447-
X(nlink)
448447
X(uid)
449448
X(gid)
450-
X(rdev)
451449
# if defined(__POSIX__)
452450
X(blksize)
453451
# else
454452
Local<Value> blksize = Undefined(env->isolate());
455453
# endif
456454
#undef X
457455

456+
// Integers.
457+
#define X(name) \
458+
Local<Value> name = Integer::NewFromUnsigned(env->isolate(), s->st_##name); \
459+
if (name.IsEmpty()) \
460+
return Local<Object>(); \
461+
462+
X(dev)
463+
X(mode)
464+
X(nlink)
465+
X(rdev)
466+
#undef X
467+
458468
// Numbers.
459469
#define X(name) \
460470
Local<Value> name = Number::New(env->isolate(), \
461471
static_cast<double>(s->st_##name)); \
462472
if (name.IsEmpty()) \
463-
return handle_scope.Escape(Local<Object>()); \
473+
return Local<Object>(); \
464474

465475
X(ino)
466476
X(size)
@@ -479,7 +489,7 @@ Local<Value> BuildStatsObject(Environment* env, const uv_stat_t* s) {
479489
(static_cast<double>(s->st_##name.tv_nsec / 1000000))); \
480490
\
481491
if (name##_msec.IsEmpty()) \
482-
return handle_scope.Escape(Local<Object>()); \
492+
return Local<Object>(); \
483493

484494
X(atim)
485495
X(mtim)

0 commit comments

Comments
 (0)