fix(node): Implement os.userInfo properly, add missing toPrimitive (

#24702) Fixes the implementation of `os.userInfo`, and adds a missing `toPrimitive` for `tmpdir`. This allows us to enable the corresponding node_compat test.
denoland · Nov 5, 2024 · 8c70b61 · 8c70b61
1 parent 68cee70
commit 8c70b61
Show file tree

Hide file tree

Showing 9 changed files with 469 additions and 54 deletions.
diff --git a/ext/node/lib.rs b/ext/node/lib.rs
@@ -348,7 +348,7 @@ deno_core::extension!(deno_node,
     ops::http2::op_http2_send_response,
     ops::os::op_node_os_get_priority<P>,
     ops::os::op_node_os_set_priority<P>,
-    ops::os::op_node_os_username<P>,
+    ops::os::op_node_os_user_info<P>,
     ops::os::op_geteuid<P>,
     ops::os::op_getegid<P>,
     ops::os::op_cpus<P>,

diff --git a/ext/node/ops/os/mod.rs b/ext/node/ops/os/mod.rs
@@ -1,5 +1,7 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 
+use std::mem::MaybeUninit;
+
 use crate::NodePermissions;
 use deno_core::op2;
 use deno_core::OpState;
@@ -15,6 +17,8 @@ pub enum OsError {
   Permission(deno_core::error::AnyError),
   #[error("Failed to get cpu info")]
   FailedToGetCpuInfo,
+  #[error("Failed to get user info")]
+  FailedToGetUserInfo(#[source] std::io::Error),
 }
 
 #[op2(fast)]
@@ -54,20 +58,162 @@ where
   priority::set_priority(pid, priority).map_err(OsError::Priority)
 }
 
+#[derive(serde::Serialize)]
+pub struct UserInfo {
+  username: String,
+  homedir: String,
+  shell: Option<String>,
+}
+
+#[cfg(unix)]
+fn get_user_info(uid: u32) -> Result<UserInfo, OsError> {
+  use std::ffi::CStr;
+  let mut pw: MaybeUninit<libc::passwd> = MaybeUninit::uninit();
+  let mut result: *mut libc::passwd = std::ptr::null_mut();
+  // SAFETY: libc call, no invariants
+  let max_buf_size = unsafe { libc::sysconf(libc::_SC_GETPW_R_SIZE_MAX) };
+  let buf_size = if max_buf_size < 0 {
+    // from the man page
+    16_384
+  } else {
+    max_buf_size as usize
+  };
+  let mut buf = {
+    let mut b = Vec::<MaybeUninit<libc::c_char>>::with_capacity(buf_size);
+    // SAFETY: MaybeUninit has no initialization invariants, and len == cap
+    unsafe {
+      b.set_len(buf_size);
+    }
+    b
+  };
+  // SAFETY: libc call, args are correct
+  let s = unsafe {
+    libc::getpwuid_r(
+      uid,
+      pw.as_mut_ptr(),
+      buf.as_mut_ptr().cast(),
+      buf_size,
+      std::ptr::addr_of_mut!(result),
+    )
+  };
+  if result.is_null() {
+    if s != 0 {
+      return Err(
+        OsError::FailedToGetUserInfo(std::io::Error::last_os_error()),
+      );
+    } else {
+      return Err(OsError::FailedToGetUserInfo(std::io::Error::from(
+        std::io::ErrorKind::NotFound,
+      )));
+    }
+  }
+  // SAFETY: pw was initialized by the call to `getpwuid_r` above
+  let pw = unsafe { pw.assume_init() };
+  // SAFETY: initialized above, pw alive until end of function, nul terminated
+  let username = unsafe { CStr::from_ptr(pw.pw_name) };
+  // SAFETY: initialized above, pw alive until end of function, nul terminated
+  let homedir = unsafe { CStr::from_ptr(pw.pw_dir) };
+  // SAFETY: initialized above, pw alive until end of function, nul terminated
+  let shell = unsafe { CStr::from_ptr(pw.pw_shell) };
+  Ok(UserInfo {
+    username: username.to_string_lossy().into_owned(),
+    homedir: homedir.to_string_lossy().into_owned(),
+    shell: Some(shell.to_string_lossy().into_owned()),
+  })
+}
+
+#[cfg(windows)]
+fn get_user_info(_uid: u32) -> Result<UserInfo, OsError> {
+  use std::ffi::OsString;
+  use std::os::windows::ffi::OsStringExt;
+
+  use windows_sys::Win32::Foundation::CloseHandle;
+  use windows_sys::Win32::Foundation::GetLastError;
+  use windows_sys::Win32::Foundation::ERROR_INSUFFICIENT_BUFFER;
+  use windows_sys::Win32::Foundation::HANDLE;
+  use windows_sys::Win32::System::Threading::GetCurrentProcess;
+  use windows_sys::Win32::System::Threading::OpenProcessToken;
+  use windows_sys::Win32::UI::Shell::GetUserProfileDirectoryW;
+  struct Handle(HANDLE);
+  impl Drop for Handle {
+    fn drop(&mut self) {
+      // SAFETY: win32 call
+      unsafe {
+        CloseHandle(self.0);
+      }
+    }
+  }
+  let mut token: MaybeUninit<HANDLE> = MaybeUninit::uninit();
+
+  // Get a handle to the current process
+  // SAFETY: win32 call
+  unsafe {
+    if OpenProcessToken(
+      GetCurrentProcess(),
+      windows_sys::Win32::Security::TOKEN_READ,
+      token.as_mut_ptr(),
+    ) == 0
+    {
+      return Err(
+        OsError::FailedToGetUserInfo(std::io::Error::last_os_error()),
+      );
+    }
+  }
+
+  // SAFETY: initialized by call above
+  let token = Handle(unsafe { token.assume_init() });
+
+  let mut bufsize = 0;
+  // get the size for the homedir buf (it'll end up in `bufsize`)
+  // SAFETY: win32 call
+  unsafe {
+    GetUserProfileDirectoryW(token.0, std::ptr::null_mut(), &mut bufsize);
+    let err = GetLastError();
+    if err != ERROR_INSUFFICIENT_BUFFER {
+      return Err(OsError::FailedToGetUserInfo(
+        std::io::Error::from_raw_os_error(err as i32),
+      ));
+    }
+  }
+  let mut path = vec![0; bufsize as usize];
+  // Actually get the homedir
+  // SAFETY: path is `bufsize` elements
+  unsafe {
+    if GetUserProfileDirectoryW(token.0, path.as_mut_ptr(), &mut bufsize) == 0 {
+      return Err(
+        OsError::FailedToGetUserInfo(std::io::Error::last_os_error()),
+      );
+    }
+  }
+  // remove trailing nul
+  path.pop();
+  let homedir_wide = OsString::from_wide(&path);
+  let homedir = homedir_wide.to_string_lossy().into_owned();
+
+  Ok(UserInfo {
+    username: deno_whoami::username(),
+    homedir,
+    shell: None,
+  })
+}
+
 #[op2]
-#[string]
-pub fn op_node_os_username<P>(
+#[serde]
+pub fn op_node_os_user_info<P>(
   state: &mut OpState,
-) -> Result<String, deno_core::error::AnyError>
+  #[smi] uid: u32,
+) -> Result<UserInfo, OsError>
 where
   P: NodePermissions + 'static,
 {
   {
     let permissions = state.borrow_mut::<P>();
-    permissions.check_sys("username", "node:os.userInfo()")?;
+    permissions
+      .check_sys("userInfo", "node:os.userInfo()")
+      .map_err(OsError::Permission)?;
   }
 
-  Ok(deno_whoami::username())
+  get_user_info(uid)
 }
 
 #[op2(fast)]

diff --git a/ext/node/polyfills/internal/errors.ts b/ext/node/polyfills/internal/errors.ts
@@ -2558,19 +2558,6 @@ export class ERR_FS_RMDIR_ENOTDIR extends NodeSystemError {
   }
 }
 
-export class ERR_OS_NO_HOMEDIR extends NodeSystemError {
-  constructor() {
-    const code = isWindows ? "ENOENT" : "ENOTDIR";
-    const ctx: NodeSystemErrorCtx = {
-      message: "not a directory",
-      syscall: "home",
-      code,
-      errno: isWindows ? osConstants.errno.ENOENT : osConstants.errno.ENOTDIR,
-    };
-    super(code, ctx, "Path is not a directory");
-  }
-}
-
 export class ERR_HTTP_SOCKET_ASSIGNED extends NodeError {
   constructor() {
     super(

diff --git a/ext/node/polyfills/os.ts b/ext/node/polyfills/os.ts
@@ -28,16 +28,17 @@ import {
   op_homedir,
   op_node_os_get_priority,
   op_node_os_set_priority,
-  op_node_os_username,
+  op_node_os_user_info,
 } from "ext:core/ops";
 
 import { validateIntegerRange } from "ext:deno_node/_utils.ts";
 import process from "node:process";
 import { isWindows } from "ext:deno_node/_util/os.ts";
-import { ERR_OS_NO_HOMEDIR } from "ext:deno_node/internal/errors.ts";
 import { os } from "ext:deno_node/internal_binding/constants.ts";
 import { osUptime } from "ext:runtime/30_os.js";
 import { Buffer } from "ext:deno_node/internal/buffer.mjs";
+import { primordials } from "ext:core/mod.js";
+const { StringPrototypeEndsWith, StringPrototypeSlice } = primordials;
 
 export const constants = os;
 
@@ -136,6 +137,8 @@ export function arch(): string {
 (uptime as any)[Symbol.toPrimitive] = (): number => uptime();
 // deno-lint-ignore no-explicit-any
 (machine as any)[Symbol.toPrimitive] = (): string => machine();
+// deno-lint-ignore no-explicit-any
+(tmpdir as any)[Symbol.toPrimitive] = (): string | null => tmpdir();
 
 export function cpus(): CPUCoreInfo[] {
   return op_cpus();
@@ -268,26 +271,27 @@ export function setPriority(pid: number, priority?: number) {
 export function tmpdir(): string | null {
   /* This follows the node js implementation, but has a few
      differences:
-     * On windows, if none of the environment variables are defined,
-       we return null.
-     * On unix we use a plain Deno.env.get, instead of safeGetenv,
+     * We use a plain Deno.env.get, instead of safeGetenv,
        which special cases setuid binaries.
-     * Node removes a single trailing / or \, we remove all.
   */
   if (isWindows) {
-    const temp = Deno.env.get("TEMP") || Deno.env.get("TMP");
-    if (temp) {
-      return temp.replace(/(?<!:)[/\\]*$/, "");
-    }
-    const base = Deno.env.get("SYSTEMROOT") || Deno.env.get("WINDIR");
-    if (base) {
-      return base + "\\temp";
+    let temp = Deno.env.get("TEMP") || Deno.env.get("TMP") ||
+      (Deno.env.get("SystemRoot") || Deno.env.get("windir")) + "\\temp";
+    if (
+      temp.length > 1 && StringPrototypeEndsWith(temp, "\\") &&
+      !StringPrototypeEndsWith(temp, ":\\")
+    ) {
+      temp = StringPrototypeSlice(temp, 0, -1);
     }
-    return null;
+
+    return temp;
   } else { // !isWindows
-    const temp = Deno.env.get("TMPDIR") || Deno.env.get("TMP") ||
+    let temp = Deno.env.get("TMPDIR") || Deno.env.get("TMP") ||
       Deno.env.get("TEMP") || "/tmp";
-    return temp.replace(/(?<!^)\/*$/, "");
+    if (temp.length > 1 && StringPrototypeEndsWith(temp, "/")) {
+      temp = StringPrototypeSlice(temp, 0, -1);
+    }
+    return temp;
   }
 }
 
@@ -320,7 +324,6 @@ export function uptime(): number {
   return osUptime();
 }
 
-/** Not yet implemented */
 export function userInfo(
   options: UserInfoOptions = { encoding: "utf-8" },
 ): UserInfo {
@@ -331,28 +334,18 @@ export function userInfo(
     uid = -1;
     gid = -1;
   }
-
-  // TODO(@crowlKats): figure out how to do this correctly:
-  //  The value of homedir returned by os.userInfo() is provided by the operating system.
-  //  This differs from the result of os.homedir(), which queries environment
-  //  variables for the home directory before falling back to the operating system response.
-  let _homedir = homedir();
-  if (!_homedir) {
-    throw new ERR_OS_NO_HOMEDIR();
-  }
-  let shell = isWindows ? null : (Deno.env.get("SHELL") || null);
-  let username = op_node_os_username();
+  let { username, homedir, shell } = op_node_os_user_info(uid);
 
   if (options?.encoding === "buffer") {
-    _homedir = _homedir ? Buffer.from(_homedir) : _homedir;
+    homedir = homedir ? Buffer.from(homedir) : homedir;
     shell = shell ? Buffer.from(shell) : shell;
     username = Buffer.from(username);
   }
 
   return {
     uid,
     gid,
-    homedir: _homedir,
+    homedir,
     shell,
     username,
   };

diff --git a/runtime/errors.rs b/runtime/errors.rs
@@ -1086,6 +1086,7 @@ mod node {
       },
       OsError::Permission(e) => get_error_class_name(e).unwrap_or("Error"),
       OsError::FailedToGetCpuInfo => "TypeError",
+      OsError::FailedToGetUserInfo(e) => get_io_error_class(e),
     }
   }
 

diff --git a/runtime/permissions/lib.rs b/runtime/permissions/lib.rs
@@ -1368,8 +1368,12 @@ impl SysDescriptor {
     match kind.as_str() {
       "hostname" | "osRelease" | "osUptime" | "loadavg"
       | "networkInterfaces" | "systemMemoryInfo" | "uid" | "gid" | "cpus"
-      | "homedir" | "getegid" | "username" | "statfs" | "getPriority"
-      | "setPriority" => Ok(Self(kind)),
+      | "homedir" | "getegid" | "statfs" | "getPriority" | "setPriority"
+      | "userInfo" => Ok(Self(kind)),
+
+      // the underlying permission check changed to `userInfo` to better match the API,
+      // alias this to avoid breaking existing projects with `--allow-sys=username`
+      "username" => Ok(Self("userInfo".into())),
       _ => Err(type_error(format!("unknown system info kind \"{kind}\""))),
     }
   }

diff --git a/tests/node_compat/config.jsonc b/tests/node_compat/config.jsonc
@@ -87,8 +87,6 @@
       "test-net-server-try-ports.js",
       "test-net-socket-timeout.js",
       "test-net-write-arguments.js",
-      // TODO(nathanwhit): Disable os.userInfo is slightly incorrect
-      // "test-os.js",
       "test-path-resolve.js",
       "test-querystring.js",
       "test-readline-interface.js",
@@ -448,6 +446,7 @@
       "test-next-tick-when-exiting.js",
       "test-next-tick.js",
       "test-nodeeventtarget.js",
+      "test-os.js",
       "test-outgoing-message-destroy.js",
       "test-outgoing-message-pipe.js",
       "test-parse-args.mjs",

diff --git a/tests/node_compat/runner/TODO.md b/tests/node_compat/runner/TODO.md
@@ -1878,7 +1878,6 @@ NOTE: This file should not be manually edited. Please edit `tests/node_compat/co
 - [parallel/test-os-homedir-no-envvar.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-os-homedir-no-envvar.js)
 - [parallel/test-os-process-priority.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-os-process-priority.js)
 - [parallel/test-os-userinfo-handles-getter-errors.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-os-userinfo-handles-getter-errors.js)
-- [parallel/test-os.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-os.js)
 - [parallel/test-path-posix-relative-on-windows.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-path-posix-relative-on-windows.js)
 - [parallel/test-pending-deprecation.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-pending-deprecation.js)
 - [parallel/test-perf-gc-crash.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-perf-gc-crash.js)