From b65d5024ef5e61301a6c8c4380bc20f4949ac60d Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 23 May 2022 12:35:02 -0400 Subject: [PATCH] fix: read raw stdin to prevent buffering (regression) (#14704) --- cli/tests/integration/task_tests.rs | 6 ++++ cli/tests/testdata/task/deno.json | 1 + cli/tests/testdata/task/task_no_args.out | 2 ++ cli/tests/testdata/task/task_non_existent.out | 2 ++ cli/tests/testdata/task/task_piped_stdin.out | 3 ++ runtime/ops/io.rs | 32 +++++++++---------- 6 files changed, 29 insertions(+), 17 deletions(-) create mode 100644 cli/tests/testdata/task/task_piped_stdin.out diff --git a/cli/tests/integration/task_tests.rs b/cli/tests/integration/task_tests.rs index 16370dabbe..e2adc85d9f 100644 --- a/cli/tests/integration/task_tests.rs +++ b/cli/tests/integration/task_tests.rs @@ -85,3 +85,9 @@ itest!(task_deno_exe_no_env { envs: vec![("NO_COLOR".to_string(), "1".to_string())], env_clear: true, }); + +itest!(task_piped_stdin { + args_vec: vec!["task", "-q", "--config", "task/deno.json", "piped"], + output: "task/task_piped_stdin.out", + envs: vec![("NO_COLOR".to_string(), "1".to_string())], +}); diff --git a/cli/tests/testdata/task/deno.json b/cli/tests/testdata/task/deno.json index 8dfc2d79c6..279c7ea8b2 100644 --- a/cli/tests/testdata/task/deno.json +++ b/cli/tests/testdata/task/deno.json @@ -4,6 +4,7 @@ "echo": "echo 1", "deno_echo": "deno eval 'console.log(5)'", "strings": "deno run main.ts && deno eval \"console.log(\\\"test\\\")\"", + "piped": "echo 12345 | (deno eval 'const b = new Uint8Array(1);Deno.stdin.readSync(b);console.log(b)' && deno eval 'const b = new Uint8Array(1);Deno.stdin.readSync(b);console.log(b)')", "exit_code_5": "echo $(echo 10 ; exit 2) && exit 5" } } diff --git a/cli/tests/testdata/task/task_no_args.out b/cli/tests/testdata/task/task_no_args.out index 7951700bb0..e8c034a2dd 100644 --- a/cli/tests/testdata/task/task_no_args.out +++ b/cli/tests/testdata/task/task_no_args.out @@ -7,5 +7,7 @@ Available tasks: echo 1 - exit_code_5 echo $(echo 10 ; exit 2) && exit 5 +- piped + echo 12345 | (deno eval 'const b = new Uint8Array(1);Deno.stdin.readSync(b);console.log(b)' && deno eval 'const b = new Uint8Array(1);Deno.stdin.readSync(b);console.log(b)') - strings deno run main.ts && deno eval "console.log(\"test\")" diff --git a/cli/tests/testdata/task/task_non_existent.out b/cli/tests/testdata/task/task_non_existent.out index 8022d8dbe1..bd4b73c6f5 100644 --- a/cli/tests/testdata/task/task_non_existent.out +++ b/cli/tests/testdata/task/task_non_existent.out @@ -9,5 +9,7 @@ Available tasks: echo 1 - exit_code_5 echo $(echo 10 ; exit 2) && exit 5 +- piped + echo 12345 | (deno eval 'const b = new Uint8Array(1);Deno.stdin.readSync(b);console.log(b)' && deno eval 'const b = new Uint8Array(1);Deno.stdin.readSync(b);console.log(b)') - strings deno run main.ts && deno eval "console.log(\"test\")" diff --git a/cli/tests/testdata/task/task_piped_stdin.out b/cli/tests/testdata/task/task_piped_stdin.out new file mode 100644 index 0000000000..f0a236c867 --- /dev/null +++ b/cli/tests/testdata/task/task_piped_stdin.out @@ -0,0 +1,3 @@ +[WILDCARD] +Uint8Array(1) [ 49 ] +Uint8Array(1) [ 50 ] diff --git a/runtime/ops/io.rs b/runtime/ops/io.rs index b43e42e328..78263ce281 100644 --- a/runtime/ops/io.rs +++ b/runtime/ops/io.rs @@ -116,7 +116,9 @@ pub fn init_stdio(stdio: Stdio) -> Extension { let t = &mut state.resource_table; t.add(StdFileResource::stdio( match stdio.stdin { - StdioPipe::Inherit => StdFileResourceInner::Stdin, + StdioPipe::Inherit => StdFileResourceInner::Stdin(Arc::new( + Mutex::new(STDIN_HANDLE.try_clone().unwrap()), + )), StdioPipe::File(pipe) => StdFileResourceInner::file(pipe), }, "stdin", @@ -296,14 +298,14 @@ impl Resource for ChildStderrResource { #[derive(Clone)] enum StdFileResourceInner { + File(Arc>), + Stdin(Arc>), // Ideally we would store stdio as an StdFile, but we get some Windows // specific functionality for free by using Rust std's wrappers. So we // take a bit of a complexity hit here in order to not have to duplicate // the functionality in Rust's std/src/sys/windows/stdio.rs - Stdin, Stdout, Stderr, - File(Arc>), } impl StdFileResourceInner { @@ -313,13 +315,12 @@ impl StdFileResourceInner { pub fn with_file(&self, mut f: impl FnMut(&mut StdFile) -> R) -> R { match self { - Self::Stdin => f(&mut STDIN_HANDLE.try_clone().unwrap()), - Self::Stdout => f(&mut STDOUT_HANDLE.try_clone().unwrap()), - Self::Stderr => f(&mut STDERR_HANDLE.try_clone().unwrap()), - Self::File(file) => { + Self::File(file) | Self::Stdin(file) => { let mut file = file.lock(); f(&mut file) } + Self::Stdout => f(&mut STDOUT_HANDLE.try_clone().unwrap()), + Self::Stderr => f(&mut STDERR_HANDLE.try_clone().unwrap()), } } @@ -344,10 +345,9 @@ impl StdFileResourceInner { impl Read for StdFileResourceInner { fn read(&mut self, buf: &mut [u8]) -> std::io::Result { match self { + Self::File(file) | Self::Stdin(file) => file.lock().read(buf), Self::Stdout => Err(ErrorKind::Unsupported.into()), Self::Stderr => Err(ErrorKind::Unsupported.into()), - Self::Stdin => std::io::stdin().read(buf), - Self::File(file) => file.lock().read(buf), } } } @@ -355,19 +355,19 @@ impl Read for StdFileResourceInner { impl Write for StdFileResourceInner { fn write(&mut self, buf: &[u8]) -> std::io::Result { match self { + Self::File(file) => file.lock().write(buf), + Self::Stdin(_) => Err(ErrorKind::Unsupported.into()), Self::Stdout => std::io::stdout().write(buf), Self::Stderr => std::io::stderr().write(buf), - Self::Stdin => Err(ErrorKind::Unsupported.into()), - Self::File(file) => file.lock().write(buf), } } fn flush(&mut self) -> std::io::Result<()> { match self { + Self::File(file) => file.lock().flush(), + Self::Stdin(_) => Err(ErrorKind::Unsupported.into()), Self::Stdout => std::io::stdout().flush(), Self::Stderr => std::io::stderr().flush(), - Self::Stdin => Err(ErrorKind::Unsupported.into()), - Self::File(file) => file.lock().flush(), } } } @@ -397,10 +397,8 @@ impl StdFileResource { pub fn std_file(&self) -> Arc> { match &self.inner { - StdFileResourceInner::File(fs_file) => fs_file.clone(), - StdFileResourceInner::Stdin => { - Arc::new(Mutex::new(STDIN_HANDLE.try_clone().unwrap())) - } + StdFileResourceInner::File(fs_file) + | StdFileResourceInner::Stdin(fs_file) => fs_file.clone(), StdFileResourceInner::Stdout => { Arc::new(Mutex::new(STDOUT_HANDLE.try_clone().unwrap())) }