fs: special input `-1` on `chown`, `lchown` and `fchown` #58836

himself65 · 2025-06-25T17:35:29Z

Fixes: #58826

-1 is a special input for chown, means won't change anything according to the spec.

https://stackoverflow.com/questions/21370094/is-the-uid-t-type-signed-or-unsigned

himself65 · 2025-06-25T17:44:41Z

I didn't see any test about valid input in chown. well do we need add one?

codecov · 2025-06-25T20:44:33Z

Codecov Report

Attention: Patch coverage is 70.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 90.07%. Comparing base (a705e24) to head (0a3e989).
Report is 31 commits behind head on main.

Files with missing lines	Patch %	Lines
src/util-inl.h	25.00%	0 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #58836      +/-   ##
==========================================
- Coverage   90.08%   90.07%   -0.01%     
==========================================
  Files         640      640              
  Lines      188375   188447      +72     
  Branches    36939    36966      +27     
==========================================
+ Hits       169698   169745      +47     
- Misses      11397    11405       +8     
- Partials     7280     7297      +17

Files with missing lines	Coverage Δ
src/node_file.cc	`75.79% <100.00%> (-0.04%)`	⬇️
src/util-inl.h	`81.90% <25.00%> (-0.28%)`	⬇️

... and 33 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

src/util-inl.h

test/parallel/test-fs-chown-negative-one.js

nodejs-github-bot · 2025-06-27T08:40:33Z

CI: https://ci.nodejs.org/job/node-test-pull-request/67677/

nodejs-github-bot · 2025-06-27T13:24:06Z

CI: https://ci.nodejs.org/job/node-test-pull-request/67681/

addaleax · 2025-06-27T15:57:40Z

src/util-inl.h

+    } else {
+      CHECK(value->IsNumber());
+    }
    return static_cast<T>(value.As<v8::Uint32>()->Value());


Pretty sure that at least according to the V8 API contract, IsUint32() is a prerequisite for this case to be valid in the first place – overall, I'd say it's better to handle these values in the *Chown functions rather than in the generic helpers here

nodejs-github-bot · 2025-06-27T17:55:22Z

CI: https://ci.nodejs.org/job/node-test-pull-request/67688/

nodejs-github-bot · 2025-06-27T20:21:59Z

CI: https://ci.nodejs.org/job/node-test-pull-request/67691/

nodejs-github-bot · 2025-06-27T21:22:30Z

Commit Queue failed

- Loading data for nodejs/node/pull/58836
✔  Done loading data for nodejs/node/pull/58836
----------------------------------- PR info ------------------------------------
Title      fs: special input `-1` on `chown`, `lchown` and `fchown` (#58836)
Author     Alex Yang <himself65@outlook.com> (@himself65)
Branch     himself65:himself65/2025/06/25/loose -> nodejs:main
Labels     c++, fs, needs-ci
Commits    1
 - fs: special input `-1` on `chown`, `lchown` and `fchown`
Committers 1
 - Alex Yang <alex@runllama.ai>
PR-URL: https://github.com/nodejs/node/pull/58836
Fixes: https://github.com/nodejs/node/issues/58826
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/58836
Fixes: https://github.com/nodejs/node/issues/58826
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Wed, 25 Jun 2025 17:35:29 GMT
   ✔  Approvals: 1
   ✔  - Joyee Cheung (@joyeecheung) (TSC): https://github.com/nodejs/node/pull/58836#pullrequestreview-2965372044
   ✘  This PR needs to wait 116 more hours to land (or 0 hours if there is one more approval)
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2025-06-27T20:21:59Z: https://ci.nodejs.org/job/node-test-pull-request/67691/
- Querying data for job/node-test-pull-request/67691/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/15936207250

nodejs-github-bot · 2025-06-28T14:04:39Z

Commit Queue failed

node:internal/process/promises:394 triggerUncaughtException(err, true /* fromPromise */); ^

Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/opt/hostedtoolcache/node/22.16.0/x64/lib/node_modules/@node-core/utils/node_modules/css-select/lib/esm/index.js' imported from /opt/hostedtoolcache/node/22.16.0/x64/lib/node_modules/@node-core/utils/node_modules/cheerio-select/lib/esm/index.js
Did you mean to import "css-select/lib/index.js"?
at finalizeResolution (node:internal/modules/esm/resolve:275:11)
at moduleResolve (node:internal/modules/esm/resolve:860:10)
at defaultResolve (node:internal/modules/esm/resolve:984:11)
at ModuleLoader.defaultResolve (node:internal/modules/esm/loader:780:12)
at #cachedDefaultResolve (node:internal/modules/esm/loader:704:25)
at ModuleLoader.resolve (node:internal/modules/esm/loader:687:38)
at ModuleLoader.getModuleJobForImport (node:internal/modules/esm/loader:305:38)
at ModuleJob._link (node:internal/modules/esm/module_job:137:49) {
code: 'ERR_MODULE_NOT_FOUND',
url: 'file:///opt/hostedtoolcache/node/22.16.0/x64/lib/node_modules/@node-core/utils/node_modules/css-select/lib/esm/index.js'
}

Node.js v22.16.0

https://github.com/nodejs/node/actions/runs/15944896023

nodejs-github-bot · 2025-06-28T18:51:56Z

Commit Queue failed

- Loading data for nodejs/node/pull/58836
✔  Done loading data for nodejs/node/pull/58836
----------------------------------- PR info ------------------------------------
Title      fs: special input `-1` on `chown`, `lchown` and `fchown` (#58836)
Author     Alex Yang <himself65@outlook.com> (@himself65)
Branch     himself65:himself65/2025/06/25/loose -> nodejs:main
Labels     c++, fs, author ready, needs-ci
Commits    1
 - fs: special input `-1` on `chown`, `lchown` and `fchown`
Committers 1
 - Alex Yang <alex@runllama.ai>
PR-URL: https://github.com/nodejs/node/pull/58836
Fixes: https://github.com/nodejs/node/issues/58826
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/58836
Fixes: https://github.com/nodejs/node/issues/58826
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Wed, 25 Jun 2025 17:35:29 GMT
   ✔  Approvals: 1
   ✔  - Joyee Cheung (@joyeecheung) (TSC): https://github.com/nodejs/node/pull/58836#pullrequestreview-2965372044
   ✘  This PR needs to wait 94 more hours to land (or 0 hours if there is one more approval)
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2025-06-27T21:22:30Z: https://ci.nodejs.org/job/node-test-pull-request/67691/
- Querying data for job/node-test-pull-request/67691/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/15947155809

nodejs-github-bot · 2025-06-29T21:47:03Z

Landed in 8b199ee

PR-URL: #58836 Fixes: #58826 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

himself65 marked this pull request as ready for review June 25, 2025 17:35

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels Jun 25, 2025

himself65 requested review from jasnell and joyeecheung June 25, 2025 17:36

himself65 force-pushed the himself65/2025/06/25/loose branch from f912834 to 6fa93e5 Compare June 25, 2025 23:34

himself65 changed the title ~~fs: edge case -1 input on chown~~ fs: special input -1 on chown, lchown and fchown Jun 25, 2025

himself65 force-pushed the himself65/2025/06/25/loose branch from 74479c1 to 64f6f95 Compare June 25, 2025 23:42

joyeecheung reviewed Jun 26, 2025

View reviewed changes

src/util-inl.h Show resolved Hide resolved

test/parallel/test-fs-chown-negative-one.js Show resolved Hide resolved

fs: special input -1 on chown, lchown and fchown

0a3e989

himself65 force-pushed the himself65/2025/06/25/loose branch from 64f6f95 to 0a3e989 Compare June 26, 2025 16:03

himself65 requested a review from joyeecheung June 26, 2025 16:04

joyeecheung approved these changes Jun 27, 2025

View reviewed changes

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 27, 2025

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 27, 2025

himself65 added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 27, 2025

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 27, 2025

addaleax reviewed Jun 27, 2025

View reviewed changes

himself65 added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 27, 2025

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 27, 2025

himself65 added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 27, 2025

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 27, 2025

himself65 added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 27, 2025

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jun 27, 2025

himself65 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 27, 2025

aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Jun 28, 2025

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jun 28, 2025

aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Jun 28, 2025

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jun 28, 2025

jasnell approved these changes Jun 29, 2025

View reviewed changes

himself65 added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 29, 2025

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 29, 2025

nodejs-github-bot merged commit 8b199ee into nodejs:main Jun 29, 2025
100 checks passed

himself65 deleted the himself65/2025/06/25/loose branch June 29, 2025 22:29

targos pushed a commit that referenced this pull request Jul 3, 2025

fs: special input -1 on chown, lchown and fchown

23bd4d1

PR-URL: #58836 Fixes: #58826 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

github-actions bot mentioned this pull request Jul 8, 2025

2025-07-09, Version 24.4.0 (Current) #58993

Merged

aduh95 pushed a commit that referenced this pull request Jul 21, 2025

fs: special input -1 on chown, lchown and fchown

bf51026

PR-URL: #58836 Fixes: #58826 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

aduh95 pushed a commit that referenced this pull request Jul 24, 2025

fs: special input -1 on chown, lchown and fchown

38ffed8

PR-URL: #58836 Fixes: #58826 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

github-actions bot mentioned this pull request Jul 28, 2025

2025-07-29, Version 22.18.0 'Jod' (LTS) #59256

Merged

Uh oh!

fs: special input -1 on chown, lchown and fchown #58836

fs: special input -1 on chown, lchown and fchown #58836

Conversation

himself65 commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

himself65 commented Jun 25, 2025

Uh oh!

codecov bot commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

nodejs-github-bot commented Jun 27, 2025

Uh oh!

nodejs-github-bot commented Jun 27, 2025

Uh oh!

addaleax Jun 27, 2025

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Jun 27, 2025

Uh oh!

nodejs-github-bot commented Jun 27, 2025

Uh oh!

nodejs-github-bot commented Jun 27, 2025

Uh oh!

nodejs-github-bot commented Jun 28, 2025

Uh oh!

nodejs-github-bot commented Jun 28, 2025

Uh oh!

Uh oh!

nodejs-github-bot commented Jun 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

fs: special input `-1` on `chown`, `lchown` and `fchown` #58836

fs: special input `-1` on `chown`, `lchown` and `fchown` #58836

himself65 commented Jun 25, 2025 •

edited

Loading

codecov bot commented Jun 25, 2025 •

edited

Loading