Skip to content

Commit

Permalink
fix(ext/node): Fix fs.access/fs.promises.access with X_OK mode …
Browse files Browse the repository at this point in the history
…parameter on Windows (#27407)

- Fixes an issue on Windows where the `fs.constants.X_OK` flag caused
`fs.access` and `fs.promises.access` to incorrectly throw a "permission
denied" error
- Introduced formatting changes due to the formatting tool
- Fixed the issue by always removing the `X_OK` bit from the mode
variable m (not sure if it's necessary to check for the presence of it
in the `mode` param first?)
- Updated unit tests to handle the mentioned constant
- `X_OK` bit is ignored in the Node implementation and should behave
like `F_OK`

fs constants Node documentation:
https://nodejs.org/api/fs.html#fsconstants

fixes #27405
  • Loading branch information
filipstev authored Dec 18, 2024
1 parent b1c685f commit 8fc4796
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 46 deletions.
102 changes: 56 additions & 46 deletions ext/node/polyfills/_fs/_fs_access.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,50 +30,58 @@ export function access(
mode = getValidMode(mode, "access");
const cb = makeCallback(callback);

Deno.lstat(path).then((info) => {
if (info.mode === null) {
// If the file mode is unavailable, we pretend it has
// the permission
cb(null);
return;
}
const m = +mode || 0;
let fileMode = +info.mode || 0;
if (Deno.build.os !== "windows" && info.uid === Deno.uid()) {
// If the user is the owner of the file, then use the owner bits of
// the file permission
fileMode >>= 6;
}
// TODO(kt3k): Also check the case when the user belong to the group
// of the file
if ((m & fileMode) === m) {
// all required flags exist
cb(null);
} else {
// some required flags don't
// deno-lint-ignore no-explicit-any
const e: any = new Error(`EACCES: permission denied, access '${path}'`);
e.path = path;
e.syscall = "access";
e.errno = codeMap.get("EACCES");
e.code = "EACCES";
cb(e);
}
}, (err) => {
if (err instanceof Deno.errors.NotFound) {
// deno-lint-ignore no-explicit-any
const e: any = new Error(
`ENOENT: no such file or directory, access '${path}'`,
);
e.path = path;
e.syscall = "access";
e.errno = codeMap.get("ENOENT");
e.code = "ENOENT";
cb(e);
} else {
cb(err);
}
});
Deno.lstat(path).then(
(info) => {
if (info.mode === null) {
// If the file mode is unavailable, we pretend it has
// the permission
cb(null);
return;
}
let m = +mode || 0;
let fileMode = +info.mode || 0;

if (Deno.build.os === "windows") {
m &= ~fs.X_OK; // Ignore the X_OK bit on Windows
} else if (info.uid === Deno.uid()) {
// If the user is the owner of the file, then use the owner bits of
// the file permission
fileMode >>= 6;
}

// TODO(kt3k): Also check the case when the user belong to the group
// of the file

if ((m & fileMode) === m) {
// all required flags exist
cb(null);
} else {
// some required flags don't
// deno-lint-ignore no-explicit-any
const e: any = new Error(`EACCES: permission denied, access '${path}'`);
e.path = path;
e.syscall = "access";
e.errno = codeMap.get("EACCES");
e.code = "EACCES";
cb(e);
}
},
(err) => {
if (err instanceof Deno.errors.NotFound) {
// deno-lint-ignore no-explicit-any
const e: any = new Error(
`ENOENT: no such file or directory, access '${path}'`,
);
e.path = path;
e.syscall = "access";
e.errno = codeMap.get("ENOENT");
e.code = "ENOENT";
cb(e);
} else {
cb(err);
}
},
);
}

export const accessPromise = promisify(access) as (
Expand All @@ -91,9 +99,11 @@ export function accessSync(path: string | Buffer | URL, mode?: number) {
// the permission
return;
}
const m = +mode! || 0;
let m = +mode! || 0;
let fileMode = +info.mode! || 0;
if (Deno.build.os !== "windows" && info.uid === Deno.uid()) {
if (Deno.build.os === "windows") {
m &= ~fs.X_OK; // Ignore the X_OK bit on Windows
} else if (info.uid === Deno.uid()) {
// If the user is the owner of the file, then use the owner bits of
// the file permission
fileMode >>= 6;
Expand Down
4 changes: 4 additions & 0 deletions tests/unit_node/_fs/_fs_access_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ Deno.test(
try {
await fs.promises.access(file, fs.constants.R_OK);
await fs.promises.access(file, fs.constants.W_OK);
await fs.promises.access(file, fs.constants.X_OK);
await fs.promises.access(file, fs.constants.F_OK);
} finally {
await Deno.remove(file);
}
Expand Down Expand Up @@ -60,6 +62,8 @@ Deno.test(
try {
fs.accessSync(file, fs.constants.R_OK);
fs.accessSync(file, fs.constants.W_OK);
fs.accessSync(file, fs.constants.X_OK);
fs.accessSync(file, fs.constants.F_OK);
} finally {
Deno.removeSync(file);
}
Expand Down

0 comments on commit 8fc4796

Please sign in to comment.