clean up

Author: fraidev

ext/node/ops/fs.rs

@@ -760,11 +760,36 @@ pub async fn op_node_rmdir(
   Ok(())
 }
 
+/// Check if a handle is managed by a non-fsFile internal Deno resource.
+/// fsFile and signal resources are allowed to be dup'd (for node:fs and
+/// native addons like node-pty). Other managed resources (SQLite, internal
+/// pipes) are denied. Unmanaged handles are allowed through.
+fn deny_internal_resource_handle(
+  state: &mut OpState,
+  handle: deno_core::ResourceHandleFd,
+) -> Result<(), FsError> {
+  for (rid, name) in state.resource_table.names() {
+    if name == "fsFile" || name == "signal" {
+      continue;
+    }
+    if matches!(
+      state.resource_table.get_handle(rid),
+      Ok(deno_core::ResourceHandle::Fd(existing)) if existing == handle
+    ) {
+      return Err(FsError::Io(std::io::Error::new(
+        std::io::ErrorKind::PermissionDenied,
+        "File descriptor is managed by an internal Deno resource",
+      )));
+    }
+  }
+  Ok(())
+}
+
 /// Create a file resource from a raw file descriptor by dup'ing it first.
 /// This is safe for cross-worker use because the dup'd fd is independently
 /// owned and can be closed without affecting the original.
 ///
-/// Safety: allows dup of fds that are either unmanaged (external: child_process
+/// Allows dup of fds that are either unmanaged (external: child_process
 /// pipes, native addons) or owned by an "fsFile" resource (opened via node:fs).
 /// Denies dup of fds managed by non-fsFile internal Deno resources (e.g.
 /// SQLite, internal pipes) to prevent unauthorized access.
@@ -790,26 +815,7 @@ fn dup_fd_impl(state: &mut OpState, fd: i32) -> Result<ResourceId, FsError> {
     )));
   }
 
-  // Check if this fd is managed by a non-fsFile internal Deno resource.
-  // fsFile resources (opened via node:fs) are safe to dup.
-  // Other managed resources (SQLite, internal pipes) are denied.
-  // Unmanaged fds (child_process, native addons) are allowed through.
-  for (rid, name) in state.resource_table.names() {
-    if name == "fsFile" || name == "signal" {
-      // node:fs and node:signal resources are allowed to be dup'd for use in
-      // native addons like node-pty
-      continue;
-    }
-    if matches!(
-      state.resource_table.get_handle(rid),
-      Ok(deno_core::ResourceHandle::Fd(existing_fd)) if existing_fd == fd
-    ) {
-      return Err(FsError::Io(std::io::Error::new(
-        std::io::ErrorKind::PermissionDenied,
-        "File descriptor is managed by an internal Deno resource",
-      )));
-    }
-  }
+  deny_internal_resource_handle(state, fd)?;
 
   // SAFETY: dup() creates a new fd pointing to the same open file description.
   let new_fd = unsafe { libc::dup(fd) };
@@ -850,6 +856,48 @@ pub fn op_node_dup_fd(
   dup_fd_impl(state, fd)
 }
 
+/// Like `libc::get_osfhandle` but safe to call with invalid fds.
+///
+/// On Windows, CRT functions invoke the "invalid parameter handler" when
+/// given a bad fd, which by default terminates the process. We temporarily
+/// install a no-op handler so `get_osfhandle` returns -1 instead of
+/// aborting. This is the same pattern libuv uses (`uv__get_osfhandle`).
+///
+/// Returns the raw HANDLE as `isize`, or -1 / -2 for invalid fds.
+#[cfg(not(unix))]
+fn safe_get_osfhandle(fd: i32) -> isize {
+  extern "C" fn silent_invalid_parameter_handler(
+    _: *const u16,
+    _: *const u16,
+    _: *const u16,
+    _: u32,
+    _: usize,
+  ) {
+  }
+
+  unsafe extern "C" {
+    fn _set_thread_local_invalid_parameter_handler(
+      handler: Option<
+        unsafe extern "C" fn(*const u16, *const u16, *const u16, u32, usize),
+      >,
+    ) -> Option<
+      unsafe extern "C" fn(*const u16, *const u16, *const u16, u32, usize),
+    >;
+  }
+
+  // SAFETY: _set_thread_local_invalid_parameter_handler and
+  // get_osfhandle are standard CRT calls. The no-op handler is
+  // thread-local and immediately restored after the call.
+  unsafe {
+    let prev = _set_thread_local_invalid_parameter_handler(Some(
+      silent_invalid_parameter_handler,
+    ));
+    let handle = libc::get_osfhandle(fd);
+    _set_thread_local_invalid_parameter_handler(prev);
+    handle
+  }
+}
+
 #[cfg(not(unix))]
 fn dup_fd_impl(state: &mut OpState, fd: i32) -> Result<ResourceId, FsError> {
   use std::fs::File as StdFile;
@@ -863,64 +911,17 @@ fn dup_fd_impl(state: &mut OpState, fd: i32) -> Result<ResourceId, FsError> {
     )));
   }
 
-  // Convert CRT fd to Windows HANDLE.
-  // get_osfhandle invokes the CRT invalid parameter handler for invalid
-  // fds, which by default terminates the process. We install a no-op
-  // handler (like libuv does) so it returns -1 instead.
-  // SAFETY: _set_thread_local_invalid_parameter_handler and
-  // get_osfhandle are safe CRT calls. The no-op handler prevents
-  // process termination for invalid fds.
-  let raw_handle = unsafe {
-    extern "C" fn silent_handler(
-      _: *const u16,
-      _: *const u16,
-      _: *const u16,
-      _: u32,
-      _: usize,
-    ) {
-    }
-    unsafe extern "C" {
-      fn _set_thread_local_invalid_parameter_handler(
-        handler: Option<
-          unsafe extern "C" fn(*const u16, *const u16, *const u16, u32, usize),
-        >,
-      ) -> Option<
-        unsafe extern "C" fn(*const u16, *const u16, *const u16, u32, usize),
-      >;
-    }
-    let prev =
-      _set_thread_local_invalid_parameter_handler(Some(silent_handler));
-    let handle = libc::get_osfhandle(fd);
-    _set_thread_local_invalid_parameter_handler(prev);
-    handle
-  };
+  let raw_handle = safe_get_osfhandle(fd);
   if raw_handle == -1 || raw_handle == -2 {
     // ERROR_INVALID_HANDLE (6) maps to EBADF via uvTranslateSysError
     return Err(FsError::Io(std::io::Error::from_raw_os_error(6)));
   }
   let handle = raw_handle as RawHandle;
 
-  // Check if this handle is managed by a non-fsFile internal Deno resource.
-  // fsFile resources (opened via node:fs) are safe to dup.
-  // Other managed resources (SQLite, internal pipes) are denied.
-  // Unmanaged fds (child_process, native addons) are allowed through.
-  for (rid, name) in state.resource_table.names() {
-    if name == "fsFile" || name == "signal" {
-      continue;
-    }
-    if matches!(
-      state.resource_table.get_handle(rid),
-      Ok(deno_core::ResourceHandle::Fd(existing_handle)) if existing_handle == handle
-    ) {
-      return Err(FsError::Io(std::io::Error::new(
-        std::io::ErrorKind::PermissionDenied,
-        "File descriptor is managed by an internal Deno resource",
-      )));
-    }
-  }
+  deny_internal_resource_handle(state, handle)?;
 
   // Duplicate the handle so it's independently owned and closeable.
-  // SAFETY: handle is valid (checked above via get_osfhandle).
+  // SAFETY: handle is valid (checked above via safe_get_osfhandle).
   let borrowed = unsafe { BorrowedHandle::borrow_raw(handle) };
   let owned = borrowed.try_clone_to_owned().map_err(FsError::Io)?;