refactor(ops/process): add StdioOrRid enum (#14393)

runtime/js/40_process.js

@@ -11,7 +11,6 @@
     ArrayPrototypeMap,
     ArrayPrototypeSlice,
     TypeError,
-    isNaN,
     ObjectEntries,
     String,
   } = window.__bootstrap.primordials;
@@ -95,10 +94,6 @@
     }
   }
 
-  function isRid(arg) {
-    return !isNaN(arg);
-  }
-
   function run({
     cmd,
     cwd = undefined,
@@ -120,12 +115,9 @@
       env: ObjectEntries(env),
       gid,
       uid,
-      stdin: isRid(stdin) ? "" : stdin,
-      stdout: isRid(stdout) ? "" : stdout,
-      stderr: isRid(stderr) ? "" : stderr,
-      stdinRid: isRid(stdin) ? stdin : 0,
-      stdoutRid: isRid(stdout) ? stdout : 0,
-      stderrRid: isRid(stderr) ? stderr : 0,
+      stdin,
+      stdout,
+      stderr,
     });
     return new Process(res);
   }

runtime/ops/io.rs

@@ -1,5 +1,6 @@
 // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license.
 
+use deno_core::error::bad_resource_id;
 use deno_core::error::not_supported;
 use deno_core::error::resource_unavailable;
 use deno_core::error::AnyError;
@@ -330,6 +331,16 @@ impl StdFileResource {
       None => Err(resource_unavailable()),
     }
   }
+
+  pub fn clone_file(
+    state: &mut OpState,
+    rid: ResourceId,
+  ) -> Result<std::fs::File, AnyError> {
+    Self::with(state, rid, move |r| match r {
+      Ok(std_file) => std_file.try_clone().map_err(AnyError::from),
+      Err(_) => Err(bad_resource_id()),
+    })
+  }
 }
 
 impl Resource for StdFileResource {

runtime/ops/process.rs

@@ -5,11 +5,10 @@ use super::io::ChildStdinResource;
 use super::io::ChildStdoutResource;
 use super::io::StdFileResource;
 use crate::permissions::Permissions;
-use deno_core::error::bad_resource_id;
-use deno_core::error::type_error;
 use deno_core::error::AnyError;
 use deno_core::op;
 
+use deno_core::serde_json;
 use deno_core::AsyncMutFuture;
 use deno_core::AsyncRefCell;
 use deno_core::Extension;
@@ -33,22 +32,72 @@ pub fn init() -> Extension {
     .build()
 }
 
-fn clone_file(
-  state: &mut OpState,
-  rid: ResourceId,
-) -> Result<std::fs::File, AnyError> {
-  StdFileResource::with(state, rid, move |r| match r {
-    Ok(std_file) => std_file.try_clone().map_err(AnyError::from),
-    Err(_) => Err(bad_resource_id()),
-  })
+#[derive(Copy, Clone, PartialEq, Deserialize)]
+#[serde(rename_all = "camelCase")]
+pub enum Stdio {
+  Inherit,
+  Piped,
+  Null,
 }
 
-fn subprocess_stdio_map(s: &str) -> Result<std::process::Stdio, AnyError> {
-  match s {
-    "inherit" => Ok(std::process::Stdio::inherit()),
-    "piped" => Ok(std::process::Stdio::piped()),
-    "null" => Ok(std::process::Stdio::null()),
-    _ => Err(type_error("Invalid resource for stdio")),
+impl Stdio {
+  pub fn as_stdio(&self) -> std::process::Stdio {
+    match &self {
+      Stdio::Inherit => std::process::Stdio::inherit(),
+      Stdio::Piped => std::process::Stdio::piped(),
+      Stdio::Null => std::process::Stdio::null(),
+    }
+  }
+}
+
+#[derive(Copy, Clone, PartialEq)]
+pub enum StdioOrRid {
+  Stdio(Stdio),
+  Rid(ResourceId),
+}
+
+impl<'de> Deserialize<'de> for StdioOrRid {
+  fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+  where
+    D: serde::Deserializer<'de>,
+  {
+    use serde_json::Value;
+    let value = Value::deserialize(deserializer)?;
+    match value {
+      Value::String(val) => match val.as_str() {
+        "inherit" => Ok(StdioOrRid::Stdio(Stdio::Inherit)),
+        "piped" => Ok(StdioOrRid::Stdio(Stdio::Piped)),
+        "null" => Ok(StdioOrRid::Stdio(Stdio::Null)),
+        val => Err(serde::de::Error::unknown_variant(
+          val,
+          &["inherit", "piped", "null"],
+        )),
+      },
+      Value::Number(val) => match val.as_u64() {
+        Some(val) if val <= ResourceId::MAX as u64 => {
+          Ok(StdioOrRid::Rid(val as ResourceId))
+        }
+        _ => Err(serde::de::Error::custom("Expected a positive integer")),
+      },
+      _ => Err(serde::de::Error::custom(
+        r#"Expected a resource id, "inherit", "piped", or "null""#,
+      )),
+    }
+  }
+}
+
+impl StdioOrRid {
+  pub fn as_stdio(
+    &self,
+    state: &mut OpState,
+  ) -> Result<std::process::Stdio, AnyError> {
+    match &self {
+      StdioOrRid::Stdio(val) => Ok(val.as_stdio()),
+      StdioOrRid::Rid(rid) => {
+        let file = StdFileResource::clone_file(state, *rid)?;
+        Ok(file.into())
+      }
+    }
   }
 }
 
@@ -63,12 +112,9 @@ pub struct RunArgs {
   gid: Option<u32>,
   #[cfg(unix)]
   uid: Option<u32>,
-  stdin: String,
-  stdout: String,
-  stderr: String,
-  stdin_rid: ResourceId,
-  stdout_rid: ResourceId,
-  stderr_rid: ResourceId,
+  stdin: StdioOrRid,
+  stdout: StdioOrRid,
+  stderr: StdioOrRid,
 }
 
 struct ChildResource {
@@ -139,26 +185,9 @@ fn op_run(state: &mut OpState, run_args: RunArgs) -> Result<RunInfo, AnyError> {
   }
 
   // TODO: make this work with other resources, eg. sockets
-  if !run_args.stdin.is_empty() {
-    c.stdin(subprocess_stdio_map(run_args.stdin.as_ref())?);
-  } else {
-    let file = clone_file(state, run_args.stdin_rid)?;
-    c.stdin(file);
-  }
-
-  if !run_args.stdout.is_empty() {
-    c.stdout(subprocess_stdio_map(run_args.stdout.as_ref())?);
-  } else {
-    let file = clone_file(state, run_args.stdout_rid)?;
-    c.stdout(file);
-  }
-
-  if !run_args.stderr.is_empty() {
-    c.stderr(subprocess_stdio_map(run_args.stderr.as_ref())?);
-  } else {
-    let file = clone_file(state, run_args.stderr_rid)?;
-    c.stderr(file);
-  }
+  c.stdin(run_args.stdin.as_stdio(state)?);
+  c.stdout(run_args.stdout.as_stdio(state)?);
+  c.stderr(run_args.stderr.as_stdio(state)?);
 
   // We want to kill child when it's closed
   c.kill_on_drop(true);
@@ -260,6 +289,7 @@ pub fn kill(pid: i32, signal: &str) -> Result<(), AnyError> {
 
 #[cfg(not(unix))]
 pub fn kill(pid: i32, signal: &str) -> Result<(), AnyError> {
+  use deno_core::error::type_error;
   use std::io::Error;
   use std::io::ErrorKind::NotFound;
   use winapi::shared::minwindef::DWORD;

runtime/ops/spawn.rs

@@ -3,6 +3,7 @@
 use super::io::ChildStderrResource;
 use super::io::ChildStdinResource;
 use super::io::ChildStdoutResource;
+use super::process::Stdio;
 use crate::permissions::Permissions;
 use deno_core::error::AnyError;
 use deno_core::op;
@@ -41,22 +42,6 @@ impl Resource for ChildResource {
   }
 }
 
-#[derive(Deserialize)]
-#[serde(rename_all = "camelCase")]
-pub enum Stdio {
-  Inherit,
-  Piped,
-  Null,
-}
-
-fn subprocess_stdio_map(s: &Stdio) -> Result<std::process::Stdio, AnyError> {
-  match s {
-    Stdio::Inherit => Ok(std::process::Stdio::inherit()),
-    Stdio::Piped => Ok(std::process::Stdio::piped()),
-    Stdio::Null => Ok(std::process::Stdio::null()),
-  }
-}
-
 #[derive(Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct SpawnArgs {
@@ -161,9 +146,9 @@ fn create_command(
     });
   }
 
-  command.stdin(subprocess_stdio_map(&args.stdio.stdin)?);
-  command.stdout(subprocess_stdio_map(&args.stdio.stdout)?);
-  command.stderr(subprocess_stdio_map(&args.stdio.stderr)?);
+  command.stdin(args.stdio.stdin.as_stdio());
+  command.stdout(args.stdio.stdout.as_stdio());
+  command.stderr(args.stdio.stderr.as_stdio());
 
   Ok(command)
 }