feat: redirect process stdio to file (#2554)

Author: bartlomieju

cli/msg.fbs

@@ -538,6 +538,9 @@ table Run {
   stdin: ProcessStdio;
   stdout: ProcessStdio;
   stderr: ProcessStdio;
+  stdin_rid: uint32;
+  stdout_rid: uint32;
+  stderr_rid: uint32;
 }
 
 table RunRes {

cli/ops.rs

@@ -1793,9 +1793,27 @@ fn op_run(
     c.env(entry.key().unwrap(), entry.value().unwrap());
   });
 
-  c.stdin(subprocess_stdio_map(inner.stdin()));
-  c.stdout(subprocess_stdio_map(inner.stdout()));
-  c.stderr(subprocess_stdio_map(inner.stderr()));
+  // TODO: make this work with other resources, eg. sockets
+  let stdin_rid = inner.stdin_rid();
+  if stdin_rid > 0 {
+    c.stdin(resources::get_file(stdin_rid)?);
+  } else {
+    c.stdin(subprocess_stdio_map(inner.stdin()));
+  }
+
+  let stdout_rid = inner.stdout_rid();
+  if stdout_rid > 0 {
+    c.stdout(resources::get_file(stdout_rid)?);
+  } else {
+    c.stdout(subprocess_stdio_map(inner.stdout()));
+  }
+
+  let stderr_rid = inner.stderr_rid();
+  if stderr_rid > 0 {
+    c.stderr(resources::get_file(stderr_rid)?);
+  } else {
+    c.stderr(subprocess_stdio_map(inner.stderr()));
+  }
 
   // Spawn the command.
   let child = c.spawn_async().map_err(DenoError::from)?;

cli/resources.rs

@@ -492,29 +492,19 @@ pub fn get_repl(rid: ResourceId) -> DenoResult<Arc<Mutex<Repl>>> {
   }
 }
 
-pub fn lookup(rid: ResourceId) -> Option<Resource> {
-  debug!("resource lookup {}", rid);
-  let table = RESOURCE_TABLE.lock().unwrap();
-  table.get(&rid).map(|_| Resource { rid })
-}
-
-// TODO(kevinkassimo): revamp this after the following lands:
+// TODO: revamp this after the following lands:
 // https://github.com/tokio-rs/tokio/pull/785
-pub fn seek(
-  resource: Resource,
-  offset: i32,
-  whence: u32,
-) -> Box<dyn Future<Item = (), Error = DenoError> + Send> {
+pub fn get_file(rid: ResourceId) -> DenoResult<std::fs::File> {
   let mut table = RESOURCE_TABLE.lock().unwrap();
   // We take ownership of File here.
   // It is put back below while still holding the lock.
-  let maybe_repr = table.remove(&resource.rid);
+  let maybe_repr = table.remove(&rid);
+
   match maybe_repr {
-    None => panic!("bad rid"),
-    Some(Repr::FsFile(f)) => {
+    Some(Repr::FsFile(r)) => {
       // Trait Clone not implemented on tokio::fs::File,
       // so convert to std File first.
-      let std_file = f.into_std();
+      let std_file = r.into_std();
       // Create a copy and immediately put back.
       // We don't want to block other resource ops.
       // try_clone() would yield a copy containing the same
@@ -523,36 +513,49 @@ pub fn seek(
       // to write back.
       let maybe_std_file_copy = std_file.try_clone();
       // Insert the entry back with the same rid.
-      table.insert(
-        resource.rid,
-        Repr::FsFile(tokio_fs::File::from_std(std_file)),
-      );
-      // Translate seek mode to Rust repr.
-      let seek_from = match whence {
-        0 => SeekFrom::Start(offset as u64),
-        1 => SeekFrom::Current(i64::from(offset)),
-        2 => SeekFrom::End(i64::from(offset)),
-        _ => {
-          return Box::new(futures::future::err(deno_error::new(
-            deno_error::ErrorKind::InvalidSeekMode,
-            format!("Invalid seek mode: {}", whence),
-          )));
-        }
-      };
+      table.insert(rid, Repr::FsFile(tokio_fs::File::from_std(std_file)));
+
       if maybe_std_file_copy.is_err() {
-        return Box::new(futures::future::err(DenoError::from(
-          maybe_std_file_copy.unwrap_err(),
-        )));
+        return Err(DenoError::from(maybe_std_file_copy.unwrap_err()));
       }
-      let mut std_file_copy = maybe_std_file_copy.unwrap();
-      Box::new(futures::future::lazy(move || {
-        let result = std_file_copy
-          .seek(seek_from)
-          .map(|_| {})
-          .map_err(DenoError::from);
-        futures::future::result(result)
-      }))
+
+      let std_file_copy = maybe_std_file_copy.unwrap();
+
+      Ok(std_file_copy)
     }
-    _ => panic!("cannot seek"),
+    _ => Err(bad_resource()),
+  }
+}
+
+pub fn lookup(rid: ResourceId) -> Option<Resource> {
+  debug!("resource lookup {}", rid);
+  let table = RESOURCE_TABLE.lock().unwrap();
+  table.get(&rid).map(|_| Resource { rid })
+}
+
+pub fn seek(
+  resource: Resource,
+  offset: i32,
+  whence: u32,
+) -> Box<dyn Future<Item = (), Error = DenoError> + Send> {
+  // Translate seek mode to Rust repr.
+  let seek_from = match whence {
+    0 => SeekFrom::Start(offset as u64),
+    1 => SeekFrom::Current(i64::from(offset)),
+    2 => SeekFrom::End(i64::from(offset)),
+    _ => {
+      return Box::new(futures::future::err(deno_error::new(
+        deno_error::ErrorKind::InvalidSeekMode,
+        format!("Invalid seek mode: {}", whence),
+      )));
+    }
+  };
+
+  match get_file(resource.rid) {
+    Ok(mut file) => Box::new(futures::future::lazy(move || {
+      let result = file.seek(seek_from).map(|_| {}).map_err(DenoError::from);
+      futures::future::result(result)
+    })),
+    Err(err) => Box::new(futures::future::err(err)),
   }
 }

js/process.ts

@@ -28,9 +28,9 @@ export interface RunOptions {
   args: string[];
   cwd?: string;
   env?: { [key: string]: string };
-  stdout?: ProcessStdio;
-  stderr?: ProcessStdio;
-  stdin?: ProcessStdio;
+  stdout?: ProcessStdio | number;
+  stderr?: ProcessStdio | number;
+  stdin?: ProcessStdio | number;
 }
 
 async function runStatus(rid: number): Promise<ProcessStatus> {
@@ -149,6 +149,10 @@ function stdioMap(s: ProcessStdio): msg.ProcessStdio {
   }
 }
 
+function isRid(arg: unknown): arg is number {
+  return !isNaN(arg as number);
+}
+
 /**
  * Spawns new subprocess.
  *
@@ -159,7 +163,8 @@ function stdioMap(s: ProcessStdio): msg.ProcessStdio {
  * mapping.
  *
  * By default subprocess inherits stdio of parent process. To change that
- * `opt.stdout`, `opt.stderr` and `opt.stdin` can be specified independently.
+ * `opt.stdout`, `opt.stderr` and `opt.stdin` can be specified independently -
+ * they can be set to either `ProcessStdio` or `rid` of open file.
  */
 export function run(opt: RunOptions): Process {
   const builder = flatbuffers.createBuilder();
@@ -177,14 +182,49 @@ export function run(opt: RunOptions): Process {
     }
   }
   const envOffset = msg.Run.createEnvVector(builder, kvOffset);
+
+  let stdInOffset = stdioMap("inherit");
+  let stdOutOffset = stdioMap("inherit");
+  let stdErrOffset = stdioMap("inherit");
+  let stdinRidOffset = 0;
+  let stdoutRidOffset = 0;
+  let stderrRidOffset = 0;
+
+  if (opt.stdin) {
+    if (isRid(opt.stdin)) {
+      stdinRidOffset = opt.stdin;
+    } else {
+      stdInOffset = stdioMap(opt.stdin);
+    }
+  }
+
+  if (opt.stdout) {
+    if (isRid(opt.stdout)) {
+      stdoutRidOffset = opt.stdout;
+    } else {
+      stdOutOffset = stdioMap(opt.stdout);
+    }
+  }
+
+  if (opt.stderr) {
+    if (isRid(opt.stderr)) {
+      stderrRidOffset = opt.stderr;
+    } else {
+      stdErrOffset = stdioMap(opt.stderr);
+    }
+  }
+
   const inner = msg.Run.createRun(
     builder,
     argsOffset,
     cwdOffset,
     envOffset,
-    opt.stdin ? stdioMap(opt.stdin) : stdioMap("inherit"),
-    opt.stdout ? stdioMap(opt.stdout) : stdioMap("inherit"),
-    opt.stderr ? stdioMap(opt.stderr) : stdioMap("inherit")
+    stdInOffset,
+    stdOutOffset,
+    stdErrOffset,
+    stdinRidOffset,
+    stdoutRidOffset,
+    stderrRidOffset
   );
   const baseRes = dispatch.sendSync(builder, msg.Any.Run, inner);
   assert(baseRes != null);

js/process_test.ts

@@ -1,6 +1,21 @@
 // Copyright 2018-2019 the Deno authors. All rights reserved. MIT license.
-import { test, testPerm, assert, assertEquals } from "./test_util.ts";
-const { kill, run, DenoError, ErrorKind } = Deno;
+import {
+  test,
+  testPerm,
+  assert,
+  assertEquals,
+  assertStrContains
+} from "./test_util.ts";
+const {
+  kill,
+  run,
+  DenoError,
+  ErrorKind,
+  readFile,
+  open,
+  makeTempDir,
+  writeFile
+} = Deno;
 
 test(function runPermissions(): void {
   let caughtError = false;
@@ -71,7 +86,7 @@ testPerm(
   { write: true, run: true },
   async function runWithCwdIsAsync(): Promise<void> {
     const enc = new TextEncoder();
-    const cwd = Deno.makeTempDirSync({ prefix: "deno_command_test" });
+    const cwd = await makeTempDir({ prefix: "deno_command_test" });
 
     const exitCodeFile = "deno_was_here";
     const pyProgramFile = "poll_exit.py";
@@ -205,6 +220,57 @@ testPerm({ run: true }, async function runStderrOutput(): Promise<void> {
   p.close();
 });
 
+testPerm(
+  { run: true, write: true, read: true },
+  async function runRedirectStdoutStderr(): Promise<void> {
+    const tempDir = await makeTempDir();
+    const fileName = tempDir + "/redirected_stdio.txt";
+    const file = await open(fileName, "w");
+
+    const p = run({
+      args: [
+        "python",
+        "-c",
+        "import sys; sys.stderr.write('error\\n'); sys.stdout.write('output\\n');"
+      ],
+      stdout: file.rid,
+      stderr: file.rid
+    });
+
+    await p.status();
+    p.close();
+    file.close();
+
+    const fileContents = await readFile(fileName);
+    const decoder = new TextDecoder();
+    const text = decoder.decode(fileContents);
+
+    assertStrContains(text, "error");
+    assertStrContains(text, "output");
+  }
+);
+
+testPerm(
+  { run: true, write: true, read: true },
+  async function runRedirectStdin(): Promise<void> {
+    const tempDir = await makeTempDir();
+    const fileName = tempDir + "/redirected_stdio.txt";
+    const encoder = new TextEncoder();
+    await writeFile(fileName, encoder.encode("hello"));
+    const file = await open(fileName, "r");
+
+    const p = run({
+      args: ["python", "-c", "import sys; assert 'hello' == sys.stdin.read();"],
+      stdin: file.rid
+    });
+
+    const status = await p.status();
+    assertEquals(status.code, 0);
+    p.close();
+    file.close();
+  }
+);
+
 testPerm({ run: true }, async function runEnv(): Promise<void> {
   const p = run({
     args: [

js/test_util.ts

@@ -16,7 +16,8 @@ export {
   assert,
   assertEquals,
   assertNotEquals,
-  assertStrictEq
+  assertStrictEq,
+  assertStrContains
 } from "./deps/https/deno.land/std/testing/asserts.ts";
 
 interface TestPermissions {