Browse Source

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.
https://github.com/nodejs/node-v0.x-archive/issues/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: https://github.com/npm/npm/issues/13918
PR-URL: https://github.com/nodejs/node/pull/8515
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

undo accidental change to other fields of uv_fs_stat
v7.x
Anna Henningsen 8 years ago
parent
commit
c5545f2c63
No known key found for this signature in database GPG Key ID: D8B9F5AEAE84E4CF
  1. 30
      src/node_file.cc

30
src/node_file.cc

@ -433,21 +433,19 @@ Local<Value> BuildStatsObject(Environment* env, const uv_stat_t* s) {
// crash(); // crash();
// } // }
// //
// We need to check the return value of Integer::New() and Date::New() // We need to check the return value of Number::New() and Date::New()
// and make sure that we bail out when V8 returns an empty handle. // and make sure that we bail out when V8 returns an empty handle.
// Integers. // Unsigned integers. It does not actually seem to be specified whether
// uid and gid are unsigned or not, but in practice they are unsigned,
// and Node’s (F)Chown functions do check their arguments for unsignedness.
#define X(name) \ #define X(name) \
Local<Value> name = Integer::New(env->isolate(), s->st_##name); \ Local<Value> name = Integer::NewFromUnsigned(env->isolate(), s->st_##name); \
if (name.IsEmpty()) \ if (name.IsEmpty()) \
return handle_scope.Escape(Local<Object>()); \ return Local<Object>(); \
X(dev)
X(mode)
X(nlink)
X(uid) X(uid)
X(gid) X(gid)
X(rdev)
# if defined(__POSIX__) # if defined(__POSIX__)
X(blksize) X(blksize)
# else # else
@ -455,12 +453,24 @@ Local<Value> BuildStatsObject(Environment* env, const uv_stat_t* s) {
# endif # endif
#undef X #undef X
// Integers.
#define X(name) \
Local<Value> name = Integer::New(env->isolate(), s->st_##name); \
if (name.IsEmpty()) \
return Local<Object>(); \
X(dev)
X(mode)
X(nlink)
X(rdev)
#undef X
// Numbers. // Numbers.
#define X(name) \ #define X(name) \
Local<Value> name = Number::New(env->isolate(), \ Local<Value> name = Number::New(env->isolate(), \
static_cast<double>(s->st_##name)); \ static_cast<double>(s->st_##name)); \
if (name.IsEmpty()) \ if (name.IsEmpty()) \
return handle_scope.Escape(Local<Object>()); \ return Local<Object>(); \
X(ino) X(ino)
X(size) X(size)
@ -479,7 +489,7 @@ Local<Value> BuildStatsObject(Environment* env, const uv_stat_t* s) {
(static_cast<double>(s->st_##name.tv_nsec / 1000000))); \ (static_cast<double>(s->st_##name.tv_nsec / 1000000))); \
\ \
if (name##_msec.IsEmpty()) \ if (name##_msec.IsEmpty()) \
return handle_scope.Escape(Local<Object>()); \ return Local<Object>(); \
X(atim) X(atim)
X(mtim) X(mtim)

Loading…
Cancel
Save