Skip to content

Commit

Permalink
fix(node): Implement fs.lchown (and process.getegid) (denoland#24418
Browse files Browse the repository at this point in the history
)

Closes denoland#21260.
Part of denoland#18218.

Implements `node:fs.lchown`, and enables the node_compat test for it.
The test uses `process.getegid`, which we didn't have implemented, so I
went ahead and implemented that as well to get the test working.
  • Loading branch information
nathanwhit authored and zebreus committed Jul 8, 2024
1 parent 9fc644c commit d439286
Show file tree
Hide file tree
Showing 14 changed files with 310 additions and 5 deletions.
20 changes: 20 additions & 0 deletions cli/standalone/file_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,26 @@ impl FileSystem for DenoCompileFileSystem {
RealFs.chown_async(path, uid, gid).await
}

fn lchown_sync(
&self,
path: &Path,
uid: Option<u32>,
gid: Option<u32>,
) -> FsResult<()> {
self.error_if_in_vfs(path)?;
RealFs.lchown_sync(path, uid, gid)
}

async fn lchown_async(
&self,
path: PathBuf,
uid: Option<u32>,
gid: Option<u32>,
) -> FsResult<()> {
self.error_if_in_vfs(&path)?;
RealFs.lchown_async(path, uid, gid).await
}

fn remove_sync(&self, path: &Path, recursive: bool) -> FsResult<()> {
self.error_if_in_vfs(path)?;
RealFs.remove_sync(path, recursive)
Expand Down
18 changes: 18 additions & 0 deletions ext/fs/in_memory_fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,24 @@ impl FileSystem for InMemoryFs {
self.chown_sync(&path, uid, gid)
}

fn lchown_sync(
&self,
_path: &Path,
_uid: Option<u32>,
_gid: Option<u32>,
) -> FsResult<()> {
Err(FsError::NotSupported)
}

async fn lchown_async(
&self,
path: PathBuf,
uid: Option<u32>,
gid: Option<u32>,
) -> FsResult<()> {
self.lchown_sync(&path, uid, gid)
}

fn remove_sync(&self, _path: &Path, _recursive: bool) -> FsResult<()> {
Err(FsError::NotSupported)
}
Expand Down
13 changes: 13 additions & 0 deletions ext/fs/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,19 @@ pub trait FileSystem: std::fmt::Debug + MaybeSend + MaybeSync {
gid: Option<u32>,
) -> FsResult<()>;

fn lchown_sync(
&self,
path: &Path,
uid: Option<u32>,
gid: Option<u32>,
) -> FsResult<()>;
async fn lchown_async(
&self,
path: PathBuf,
uid: Option<u32>,
gid: Option<u32>,
) -> FsResult<()>;

fn remove_sync(&self, path: &Path, recursive: bool) -> FsResult<()>;
async fn remove_async(&self, path: PathBuf, recursive: bool) -> FsResult<()>;

Expand Down
43 changes: 43 additions & 0 deletions ext/fs/std_fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,24 @@ impl FileSystem for RealFs {
.await?
}

fn lchown_sync(
&self,
path: &Path,
uid: Option<u32>,
gid: Option<u32>,
) -> FsResult<()> {
lchown(path, uid, gid)
}

async fn lchown_async(
&self,
path: PathBuf,
uid: Option<u32>,
gid: Option<u32>,
) -> FsResult<()> {
spawn_blocking(move || lchown(&path, uid, gid)).await?
}

fn write_file_sync(
&self,
path: &Path,
Expand Down Expand Up @@ -431,6 +449,31 @@ fn chown(_path: &Path, _uid: Option<u32>, _gid: Option<u32>) -> FsResult<()> {
Err(FsError::NotSupported)
}

#[cfg(unix)]
fn lchown(path: &Path, uid: Option<u32>, gid: Option<u32>) -> FsResult<()> {
use std::os::unix::ffi::OsStrExt;
let c_path = std::ffi::CString::new(path.as_os_str().as_bytes()).unwrap();
// -1 = leave unchanged
let uid = uid
.map(|uid| uid as libc::uid_t)
.unwrap_or(-1i32 as libc::uid_t);
let gid = gid
.map(|gid| gid as libc::gid_t)
.unwrap_or(-1i32 as libc::gid_t);
// SAFETY: `c_path` is a valid C string and lives throughout this function call.
let result = unsafe { libc::lchown(c_path.as_ptr(), uid, gid) };
if result != 0 {
return Err(io::Error::last_os_error().into());
}
Ok(())
}

// TODO: implement lchown for Windows
#[cfg(not(unix))]
fn lchown(_path: &Path, _uid: Option<u32>, _gid: Option<u32>) -> FsResult<()> {
Err(FsError::NotSupported)
}

fn remove(path: &Path, recursive: bool) -> FsResult<()> {
// TODO: this is racy. This should open fds, and then `unlink` those.
let metadata = fs::symlink_metadata(path)?;
Expand Down
4 changes: 4 additions & 0 deletions ext/node/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,8 @@ deno_core::extension!(deno_node,
ops::fs::op_node_fs_exists_sync<P>,
ops::fs::op_node_cp_sync<P>,
ops::fs::op_node_cp<P>,
ops::fs::op_node_lchown_sync<P>,
ops::fs::op_node_lchown<P>,
ops::fs::op_node_lutimes_sync<P>,
ops::fs::op_node_lutimes<P>,
ops::fs::op_node_statfs<P>,
Expand Down Expand Up @@ -365,6 +367,7 @@ deno_core::extension!(deno_node,
ops::os::op_node_os_set_priority<P>,
ops::os::op_node_os_username<P>,
ops::os::op_geteuid<P>,
ops::os::op_getegid<P>,
ops::os::op_cpus<P>,
ops::os::op_homedir<P>,
op_node_build_os,
Expand Down Expand Up @@ -426,6 +429,7 @@ deno_core::extension!(deno_node,
"_fs/_fs_fsync.ts",
"_fs/_fs_ftruncate.ts",
"_fs/_fs_futimes.ts",
"_fs/_fs_lchown.ts",
"_fs/_fs_link.ts",
"_fs/_fs_lstat.ts",
"_fs/_fs_lutimes.ts",
Expand Down
41 changes: 41 additions & 0 deletions ext/node/ops/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,3 +273,44 @@ where

Ok(())
}

#[op2]
pub fn op_node_lchown_sync<P>(
state: &mut OpState,
#[string] path: String,
uid: Option<u32>,
gid: Option<u32>,
) -> Result<(), AnyError>
where
P: NodePermissions + 'static,
{
let path = PathBuf::from(path);
state
.borrow_mut::<P>()
.check_write_with_api_name(&path, Some("node:fs.lchownSync"))?;
let fs = state.borrow::<FileSystemRc>();
fs.lchown_sync(&path, uid, gid)?;
Ok(())
}

#[op2(async)]
pub async fn op_node_lchown<P>(
state: Rc<RefCell<OpState>>,
#[string] path: String,
uid: Option<u32>,
gid: Option<u32>,
) -> Result<(), AnyError>
where
P: NodePermissions + 'static,
{
let path = PathBuf::from(path);
let fs = {
let mut state = state.borrow_mut();
state
.borrow_mut::<P>()
.check_write_with_api_name(&path, Some("node:fs.lchown"))?;
state.borrow::<FileSystemRc>().clone()
};
fs.lchown_async(path, uid, gid).await?;
Ok(())
}
19 changes: 19 additions & 0 deletions ext/node/ops/os/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,25 @@ where
Ok(euid)
}

#[op2(fast)]
pub fn op_getegid<P>(state: &mut OpState) -> Result<u32, AnyError>
where
P: NodePermissions + 'static,
{
{
let permissions = state.borrow_mut::<P>();
permissions.check_sys("getegid", "node:os.getegid()")?;
}

#[cfg(windows)]
let egid = 0;
#[cfg(unix)]
// SAFETY: Call to libc getegid.
let egid = unsafe { libc::getegid() };

Ok(egid)
}

#[op2]
#[serde]
pub fn op_cpus<P>(state: &mut OpState) -> Result<Vec<cpus::CpuInfo>, AnyError>
Expand Down
61 changes: 61 additions & 0 deletions ext/node/polyfills/_fs/_fs_lchown.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.

// TODO(petamoriken): enable prefer-primordials for node polyfills
// deno-lint-ignore-file prefer-primordials

import {
type CallbackWithError,
makeCallback,
} from "ext:deno_node/_fs/_fs_common.ts";
import {
getValidatedPath,
kMaxUserId,
} from "ext:deno_node/internal/fs/utils.mjs";
import * as pathModule from "node:path";
import { validateInteger } from "ext:deno_node/internal/validators.mjs";
import type { Buffer } from "node:buffer";
import { promisify } from "ext:deno_node/internal/util.mjs";
import { op_node_lchown, op_node_lchown_sync } from "ext:core/ops";

/**
* Asynchronously changes the owner and group
* of a file, without following symlinks.
*/
export function lchown(
path: string | Buffer | URL,
uid: number,
gid: number,
callback: CallbackWithError,
) {
callback = makeCallback(callback);
path = getValidatedPath(path).toString();
validateInteger(uid, "uid", -1, kMaxUserId);
validateInteger(gid, "gid", -1, kMaxUserId);

op_node_lchown(pathModule.toNamespacedPath(path), uid, gid).then(
() => callback(null),
callback,
);
}

export const lchownPromise = promisify(lchown) as (
path: string | Buffer | URL,
uid: number,
gid: number,
) => Promise<void>;

/**
* Synchronously changes the owner and group
* of a file, without following symlinks.
*/
export function lchownSync(
path: string | Buffer | URL,
uid: number,
gid: number,
) {
path = getValidatedPath(path).toString();
validateInteger(uid, "uid", -1, kMaxUserId);
validateInteger(gid, "gid", -1, kMaxUserId);

op_node_lchown_sync(pathModule.toNamespacedPath(path), uid, gid);
}
9 changes: 8 additions & 1 deletion ext/node/polyfills/fs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ import { fstat, fstatSync } from "ext:deno_node/_fs/_fs_fstat.ts";
import { fsync, fsyncSync } from "ext:deno_node/_fs/_fs_fsync.ts";
import { ftruncate, ftruncateSync } from "ext:deno_node/_fs/_fs_ftruncate.ts";
import { futimes, futimesSync } from "ext:deno_node/_fs/_fs_futimes.ts";
import {
lchown,
lchownPromise,
lchownSync,
} from "ext:deno_node/_fs/_fs_lchown.ts";
import { link, linkPromise, linkSync } from "ext:deno_node/_fs/_fs_link.ts";
import { lstat, lstatPromise, lstatSync } from "ext:deno_node/_fs/_fs_lstat.ts";
import {
Expand Down Expand Up @@ -173,7 +178,7 @@ const promises = {
unlink: unlinkPromise,
chmod: chmodPromise,
// lchmod: promisify(lchmod),
// lchown: promisify(lchown),
lchown: lchownPromise,
chown: chownPromise,
utimes: utimesPromise,
lutimes: lutimesPromise,
Expand Down Expand Up @@ -218,6 +223,8 @@ export default {
ftruncateSync,
futimes,
futimesSync,
lchown,
lchownSync,
link,
linkSync,
lstat,
Expand Down
10 changes: 8 additions & 2 deletions ext/node/polyfills/process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import { core, internals } from "ext:core/mod.js";
import { initializeDebugEnv } from "ext:deno_node/internal/util/debuglog.ts";
import {
op_getegid,
op_geteuid,
op_node_process_kill,
op_process_abort,
Expand Down Expand Up @@ -309,15 +310,16 @@ export function kill(pid: number, sig: string | number = "SIGTERM") {
return true;
}

let getgid, getuid, geteuid;
let getgid, getuid, getegid, geteuid;

if (!isWindows) {
getgid = () => Deno.gid();
getuid = () => Deno.uid();
getegid = () => op_getegid();
geteuid = () => op_geteuid();
}

export { geteuid, getgid, getuid };
export { getegid, geteuid, getgid, getuid };

const ALLOWED_FLAGS = buildAllowedFlags();

Expand Down Expand Up @@ -685,6 +687,9 @@ Process.prototype.getgid = getgid;
/** This method is removed on Windows */
Process.prototype.getuid = getuid;

/** This method is removed on Windows */
Process.prototype.getegid = getegid;

/** This method is removed on Windows */
Process.prototype.geteuid = geteuid;

Expand Down Expand Up @@ -726,6 +731,7 @@ Process.prototype.noDeprecation = false;
if (isWindows) {
delete Process.prototype.getgid;
delete Process.prototype.getuid;
delete Process.prototype.getegid;
delete Process.prototype.geteuid;
}

Expand Down
4 changes: 3 additions & 1 deletion runtime/permissions/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -915,7 +915,9 @@ impl Descriptor for SysDescriptor {
pub fn parse_sys_kind(kind: &str) -> Result<&str, AnyError> {
match kind {
"hostname" | "osRelease" | "osUptime" | "loadavg" | "networkInterfaces"
| "systemMemoryInfo" | "uid" | "gid" | "cpus" | "homedir" => Ok(kind),
| "systemMemoryInfo" | "uid" | "gid" | "cpus" | "homedir" | "getegid" => {
Ok(kind)
}
_ => Err(type_error(format!("unknown system info kind \"{kind}\""))),
}
}
Expand Down
1 change: 1 addition & 0 deletions tests/node_compat/config.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,7 @@
"test-fs-chown-type-check.js",
"test-fs-copyfile.js",
"test-fs-empty-readStream.js",
"test-fs-lchown.js",
"test-fs-mkdir.js",
"test-fs-open-flags.js",
"test-fs-open-mode-mask.js",
Expand Down
1 change: 0 additions & 1 deletion tests/node_compat/runner/TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -794,7 +794,6 @@ NOTE: This file should not be manually edited. Please edit `tests/node_compat/co
- [parallel/test-fs-fmap.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-fs-fmap.js)
- [parallel/test-fs-fsync.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-fs-fsync.js)
- [parallel/test-fs-lchmod.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-fs-lchmod.js)
- [parallel/test-fs-lchown.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-fs-lchown.js)
- [parallel/test-fs-link.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-fs-link.js)
- [parallel/test-fs-long-path.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-fs-long-path.js)
- [parallel/test-fs-make-callback.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-fs-make-callback.js)
Expand Down
Loading

0 comments on commit d439286

Please sign in to comment.