Skip to content

Always make fs.Stats.uid and .gid fields unsigned#87

Merged
isaacs merged 1 commit intoisaacs:masterfrom
addaleax:signed-uid-gid
Sep 26, 2016
Merged

Always make fs.Stats.uid and .gid fields unsigned#87
isaacs merged 1 commit intoisaacs:masterfrom
addaleax:signed-uid-gid

Conversation

@addaleax
Copy link
Copy Markdown
Contributor

At the time of writing, all currently published versions of Node.js return signed 32-bit integers in their return values for the uid and gid fields of fs.Stats instances.

This is problematic, because some of Node’s other fs methods like chown expect unsigned 32-bit integer input and throw when encountering negative integers; this has broken e.g. sudo npm install -g on OS X, where nobody has a UID that would be returned as -2 by fs.stat().

Ref: nodejs/node#8515
Ref: npm/npm#13918

At the time of writing, all currently published versions of Node.js
return signed 32-bit integers in their return values for the `uid`
and `gid` fields of `fs.Stats` instances.

This is problematic, because some of Node’s other `fs` methods like
`chown` expect unsigned 32-bit integer input and throw when encountering
negative integers; this has broken e.g. `sudo npm install -g` on `OS X`,
where `nobody` has a UID that would be returned as `-2` by `fs.stat()`.

Ref: nodejs/node#8515
Ref: npm/npm#13918
@isaacs isaacs merged commit f528729 into isaacs:master Sep 26, 2016
@addaleax addaleax deleted the signed-uid-gid branch September 26, 2016 21:48
Copy link
Copy Markdown

@manjaneqx manjaneqx left a comment

Choose a reason for hiding this comment

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

🔌

Comment thread polyfills.js
fs.fstatSync = statFixSync(fs.fstatSync)
fs.lstatSync = statFixSync(fs.lstatSync)

// if lchmod/lchown do not exist, then make them no-ops
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants