From 4102a195851033a21c4a23daac4c78ed2329f61f Mon Sep 17 00:00:00 2001 From: Luca Casonato Date: Thu, 25 Jun 2020 18:38:19 +0200 Subject: [PATCH] fix: panic when process stdio rid is 0 or invalid (#6405) --- cli/ops/process.rs | 37 ++++++++++++++---------------- cli/tests/unit/process_test.ts | 41 ++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 20 deletions(-) diff --git a/cli/ops/process.rs b/cli/ops/process.rs index 78d9313c0c..384068e6af 100644 --- a/cli/ops/process.rs +++ b/cli/ops/process.rs @@ -33,12 +33,12 @@ fn clone_file( }) } -fn subprocess_stdio_map(s: &str) -> std::process::Stdio { +fn subprocess_stdio_map(s: &str) -> Result { match s { - "inherit" => std::process::Stdio::inherit(), - "piped" => std::process::Stdio::piped(), - "null" => std::process::Stdio::null(), - _ => unreachable!(), + "inherit" => Ok(std::process::Stdio::inherit()), + "piped" => Ok(std::process::Stdio::piped()), + "null" => Ok(std::process::Stdio::null()), + _ => Err(OpError::other("Invalid resource for stdio".to_string())), } } @@ -86,28 +86,25 @@ fn op_run( } // TODO: make this work with other resources, eg. sockets - let stdin_rid = run_args.stdin_rid; - if stdin_rid > 0 { - let file = clone_file(stdin_rid, &mut resource_table)?; + if run_args.stdin != "" { + c.stdin(subprocess_stdio_map(run_args.stdin.as_ref())?); + } else { + let file = clone_file(run_args.stdin_rid, &mut resource_table)?; c.stdin(file); - } else { - c.stdin(subprocess_stdio_map(run_args.stdin.as_ref())); } - let stdout_rid = run_args.stdout_rid; - if stdout_rid > 0 { - let file = clone_file(stdout_rid, &mut resource_table)?; + if run_args.stdout != "" { + c.stdout(subprocess_stdio_map(run_args.stdout.as_ref())?); + } else { + let file = clone_file(run_args.stdout_rid, &mut resource_table)?; c.stdout(file); - } else { - c.stdout(subprocess_stdio_map(run_args.stdout.as_ref())); } - let stderr_rid = run_args.stderr_rid; - if stderr_rid > 0 { - let file = clone_file(stderr_rid, &mut resource_table)?; - c.stderr(file); + if run_args.stderr != "" { + c.stderr(subprocess_stdio_map(run_args.stderr.as_ref())?); } else { - c.stderr(subprocess_stdio_map(run_args.stderr.as_ref())); + let file = clone_file(run_args.stderr_rid, &mut resource_table)?; + c.stderr(file); } // We want to kill child when it's closed diff --git a/cli/tests/unit/process_test.ts b/cli/tests/unit/process_test.ts index f2e6e01e7a..b82f048ee3 100644 --- a/cli/tests/unit/process_test.ts +++ b/cli/tests/unit/process_test.ts @@ -1,6 +1,7 @@ // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. import { assert, + assertThrows, assertEquals, assertStringContains, assertThrows, @@ -26,6 +27,46 @@ unitTest({ perms: { run: true } }, async function runSuccess(): Promise { p.stdout.close(); p.close(); }); +unitTest({ perms: { run: true } }, async function runStdinRid0(): Promise< + void +> { + const p = Deno.run({ + cmd: ["python", "-c", "print('hello world')"], + stdin: 0, + stdout: "piped", + stderr: "null", + }); + const status = await p.status(); + assertEquals(status.success, true); + assertEquals(status.code, 0); + assertEquals(status.signal, undefined); + p.stdout.close(); + p.close(); +}); + +unitTest({ perms: { run: true } }, function runInvalidStdio(): void { + assertThrows(() => + Deno.run({ + cmd: ["python", "-c", "print('hello world')"], + // @ts-expect-error because Deno.run should throw on invalid stdin. + stdin: "a", + }) + ); + assertThrows(() => + Deno.run({ + cmd: ["python", "-c", "print('hello world')"], + // @ts-expect-error because Deno.run should throw on invalid stdout. + stdout: "b", + }) + ); + assertThrows(() => + Deno.run({ + cmd: ["python", "-c", "print('hello world')"], + // @ts-expect-error because Deno.run should throw on invalid stderr. + stderr: "c", + }) + ); +}); unitTest( { perms: { run: true } },