Commit 3d6f107
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
PR-URL: #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_stat1 parent 8b93fdd commit 3d6f107
1 file changed
Lines changed: 20 additions & 10 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -358,34 +358,44 @@ Local<Value> BuildStatsObject(Environment* env, const uv_stat_t* s) { | |||
| 358 | 358 | // crash(); | |
| 359 | 359 | // } | |
| 360 | 360 | // | |
| 361 | - // We need to check the return value of Integer::New() and Date::New() | ||
| 361 | + // We need to check the return value of Number::New() and Date::New() | ||
| 362 | 362 | // and make sure that we bail out when V8 returns an empty handle. | |
| 363 | 363 | ||
| 364 | - // Integers. | ||
| 364 | + // Unsigned integers. It does not actually seem to be specified whether | ||
| 365 | + // uid and gid are unsigned or not, but in practice they are unsigned, | ||
| 366 | + // and Node’s (F)Chown functions do check their arguments for unsignedness. | ||
| 365 | 367 | #define X(name) \ | |
| 366 | - Local<Value> name = Integer::New(env->isolate(), s->st_##name); \ | ||
| 368 | + Local<Value> name = Integer::NewFromUnsigned(env->isolate(), s->st_##name); \ | ||
| 367 | 369 | if (name.IsEmpty()) \ | |
| 368 | - return handle_scope.Escape(Local<Object>()); \ | ||
| 370 | + return Local<Object>(); \ | ||
| 369 | 371 | ||
| 370 | - X(dev) | ||
| 371 | - X(mode) | ||
| 372 | - X(nlink) | ||
| 373 | 372 | X(uid) | |
| 374 | 373 | X(gid) | |
| 375 | - X(rdev) | ||
| 376 | 374 | # if defined(__POSIX__) | |
| 377 | 375 | X(blksize) | |
| 378 | 376 | # else | |
| 379 | 377 | Local<Value> blksize = Undefined(env->isolate()); | |
| 380 | 378 | # endif | |
| 381 | 379 | #undef X | |
| 382 | 380 | ||
| 381 | + // Integers. | ||
| 382 | + #define X(name) \ | ||
| 383 | + Local<Value> name = Integer::New(env->isolate(), s->st_##name); \ | ||
| 384 | + if (name.IsEmpty()) \ | ||
| 385 | + return Local<Object>(); \ | ||
| 386 | + | ||
| 387 | + X(dev) | ||
| 388 | + X(mode) | ||
| 389 | + X(nlink) | ||
| 390 | + X(rdev) | ||
| 391 | + #undef X | ||
| 392 | + | ||
| 383 | 393 | // Numbers. | |
| 384 | 394 | #define X(name) \ | |
| 385 | 395 | Local<Value> name = Number::New(env->isolate(), \ | |
| 386 | 396 | static_cast<double>(s->st_##name)); \ | |
| 387 | 397 | if (name.IsEmpty()) \ | |
| 388 | - return handle_scope.Escape(Local<Object>()); \ | ||
| 398 | + return Local<Object>(); \ | ||
| 389 | 399 | ||
| 390 | 400 | X(ino) | |
| 391 | 401 | X(size) | |
@@ -404,7 +414,7 @@ Local<Value> BuildStatsObject(Environment* env, const uv_stat_t* s) { | |||
| 404 | 414 | (static_cast<double>(s->st_##name.tv_nsec / 1000000))); \ | |
| 405 | 415 | \ | |
| 406 | 416 | if (name##_msec.IsEmpty()) \ | |
| 407 | - return handle_scope.Escape(Local<Object>()); \ | ||
| 417 | + return Local<Object>(); \ | ||
| 408 | 418 | ||
| 409 | 419 | X(atim) | |
| 410 | 420 | X(mtim) | |
0 commit comments