From edb7a0eead3604316b3cca2ac9122c5599445a63 Mon Sep 17 00:00:00 2001 From: Maayan Hanin Date: Thu, 9 Jul 2020 22:06:51 +0300 Subject: [PATCH] fix(cli): panic when stdio is null on windows (#6528) Fixes: #6409 --- cli/ops/io.rs | 71 +++++++++++------- cli/ops/worker_host.rs | 12 +++- cli/tests/DenoWinRunner.cs | 127 +++++++++++++++++++++++++++++++++ cli/tests/DenoWinRunner.ps1 | 10 +++ cli/tests/integration_tests.rs | 59 +++++++++++++++ cli/worker.rs | 12 +++- test_util/src/lib.rs | 31 ++++++++ 7 files changed, 289 insertions(+), 33 deletions(-) create mode 100644 cli/tests/DenoWinRunner.cs create mode 100644 cli/tests/DenoWinRunner.ps1 diff --git a/cli/ops/io.rs b/cli/ops/io.rs index 785bb42b7a..c35e0d4b67 100644 --- a/cli/ops/io.rs +++ b/cli/ops/io.rs @@ -37,36 +37,48 @@ lazy_static! { /// resource table is dropped storing reference to that handle, the handle /// itself won't be closed (so Deno.core.print) will still work. // TODO(ry) It should be possible to close stdout. - static ref STDIN_HANDLE: std::fs::File = { + static ref STDIN_HANDLE: Option = { #[cfg(not(windows))] - let stdin = unsafe { std::fs::File::from_raw_fd(0) }; + let stdin = unsafe { Some(std::fs::File::from_raw_fd(0)) }; #[cfg(windows)] let stdin = unsafe { - std::fs::File::from_raw_handle(winapi::um::processenv::GetStdHandle( + let handle = winapi::um::processenv::GetStdHandle( winapi::um::winbase::STD_INPUT_HANDLE, - )) + ); + if handle.is_null() { + return None; + } + Some(std::fs::File::from_raw_handle(handle)) }; stdin }; - static ref STDOUT_HANDLE: std::fs::File = { + static ref STDOUT_HANDLE: Option = { #[cfg(not(windows))] - let stdout = unsafe { std::fs::File::from_raw_fd(1) }; + let stdout = unsafe { Some(std::fs::File::from_raw_fd(1)) }; #[cfg(windows)] let stdout = unsafe { - std::fs::File::from_raw_handle(winapi::um::processenv::GetStdHandle( + let handle = winapi::um::processenv::GetStdHandle( winapi::um::winbase::STD_OUTPUT_HANDLE, - )) + ); + if handle.is_null() { + return None; + } + Some(std::fs::File::from_raw_handle(handle)) }; stdout }; - static ref STDERR_HANDLE: std::fs::File = { + static ref STDERR_HANDLE: Option = { #[cfg(not(windows))] - let stderr = unsafe { std::fs::File::from_raw_fd(2) }; + let stderr = unsafe { Some(std::fs::File::from_raw_fd(2)) }; #[cfg(windows)] let stderr = unsafe { - std::fs::File::from_raw_handle(winapi::um::processenv::GetStdHandle( + let handle = winapi::um::processenv::GetStdHandle( winapi::um::winbase::STD_ERROR_HANDLE, - )) + ); + if handle.is_null() { + return None; + } + Some(std::fs::File::from_raw_handle(handle)) }; stderr }; @@ -78,26 +90,31 @@ pub fn init(i: &mut CoreIsolate, s: &State) { } pub fn get_stdio() -> ( - StreamResourceHolder, - StreamResourceHolder, - StreamResourceHolder, + Option, + Option, + Option, ) { - let stdin = StreamResourceHolder::new(StreamResource::FsFile(Some({ - let stdin = STDIN_HANDLE.try_clone().unwrap(); - (tokio::fs::File::from_std(stdin), FileMetadata::default()) - }))); - let stdout = StreamResourceHolder::new(StreamResource::FsFile(Some({ - let stdout = STDOUT_HANDLE.try_clone().unwrap(); - (tokio::fs::File::from_std(stdout), FileMetadata::default()) - }))); - let stderr = StreamResourceHolder::new(StreamResource::FsFile(Some({ - let stderr = STDERR_HANDLE.try_clone().unwrap(); - (tokio::fs::File::from_std(stderr), FileMetadata::default()) - }))); + let stdin = get_stdio_stream(&STDIN_HANDLE); + let stdout = get_stdio_stream(&STDOUT_HANDLE); + let stderr = get_stdio_stream(&STDERR_HANDLE); (stdin, stdout, stderr) } +fn get_stdio_stream( + handle: &Option, +) -> Option { + match handle { + None => None, + Some(file_handle) => match file_handle.try_clone() { + Ok(clone) => Some(StreamResourceHolder::new(StreamResource::FsFile( + Some((tokio::fs::File::from_std(clone), FileMetadata::default())), + ))), + Err(_e) => None, + }, + } +} + fn no_buffer_specified() -> OpError { OpError::type_error("no buffer specified".to_string()) } diff --git a/cli/ops/worker_host.rs b/cli/ops/worker_host.rs index 950797352e..e3571f7133 100644 --- a/cli/ops/worker_host.rs +++ b/cli/ops/worker_host.rs @@ -58,9 +58,15 @@ fn create_web_worker( let state = state_rc.borrow(); let mut resource_table = state.resource_table.borrow_mut(); let (stdin, stdout, stderr) = get_stdio(); - resource_table.add("stdin", Box::new(stdin)); - resource_table.add("stdout", Box::new(stdout)); - resource_table.add("stderr", Box::new(stderr)); + if let Some(stream) = stdin { + resource_table.add("stdin", Box::new(stream)); + } + if let Some(stream) = stdout { + resource_table.add("stdout", Box::new(stream)); + } + if let Some(stream) = stderr { + resource_table.add("stderr", Box::new(stream)); + } } // Instead of using name for log we use `worker-${id}` because diff --git a/cli/tests/DenoWinRunner.cs b/cli/tests/DenoWinRunner.cs new file mode 100644 index 0000000000..7879d146dd --- /dev/null +++ b/cli/tests/DenoWinRunner.cs @@ -0,0 +1,127 @@ +using System; +using System.ComponentModel; +using System.Diagnostics; +using System.IO; +using System.Runtime.InteropServices; +using System.Threading.Tasks; + +[Flags] +public enum DenoConstraints : int +{ + None = 0, + NoStdin = 1, + NoStdout = 2, + NoStderr = 4 +} + +public class DenoWinRunner +{ + private const int STD_INPUT_HANDLE = -10; + private const int STD_OUTPUT_HANDLE = -11; + private const int STD_ERROR_HANDLE = -12; + + private const int FILE_NOT_FOUND = 2; + private const int WAIT_TIMEOUT = 258; + + [DllImport("kernel32.dll")] + private static extern void SetStdHandle(int nStdHandle, IntPtr handle); + + /// + /// Runs Deno.exe under the specified constraints + /// + /// Path to the Deno.exe file. Can be absolute or relative + /// Path to the script file Deno should run. + /// The contrainsts to apply to the Deno process + /// How long to wait for the Deno process to exit + /// The deno.exe exit code, or an exit code provided by the test runner + public static int RunDenoScript(string pathToDenoExe, string pathToTestScript, DenoConstraints constraints, uint timeoutMilliseconds = 1000) + { + try + { + if (!File.Exists(pathToDenoExe)) + { + Console.Error.WriteLine("Cannot find Deno.exe at " + pathToDenoExe); + return FILE_NOT_FOUND; + } + + if (!File.Exists(pathToTestScript)) + { + Console.Error.WriteLine("Cannot find test script at " + pathToTestScript); + return FILE_NOT_FOUND; + } + + ProcessStartInfo startInfo = new ProcessStartInfo(pathToDenoExe) + { + ErrorDialog = false, + UseShellExecute = false, + Arguments = @"run -A " + pathToTestScript, + RedirectStandardInput = !constraints.HasFlag(DenoConstraints.NoStdin), + RedirectStandardOutput = !constraints.HasFlag(DenoConstraints.NoStdout), + RedirectStandardError = !constraints.HasFlag(DenoConstraints.NoStderr) + }; + + startInfo.Environment.Add("RUST_BACKTRACE", "1"); + + if (constraints.HasFlag(DenoConstraints.NoStdin)) + { + SetStdHandle(STD_INPUT_HANDLE, (IntPtr)null); + } + + if (constraints.HasFlag(DenoConstraints.NoStdout)) + { + SetStdHandle(STD_OUTPUT_HANDLE, (IntPtr)null); + } + + if (constraints.HasFlag(DenoConstraints.NoStderr)) + { + SetStdHandle(STD_ERROR_HANDLE, (IntPtr)null); + } + + Process process = new Process { StartInfo = startInfo }; + process.Start(); + + Task stdErrTask = startInfo.RedirectStandardError ? + process.StandardError.ReadToEndAsync() : Task.FromResult(null); + Task stdOutTask = startInfo.RedirectStandardOutput ? + process.StandardOutput.ReadToEndAsync() : Task.FromResult(null); + + if (!process.WaitForExit((int)timeoutMilliseconds)) + { + Console.Error.WriteLine("Timed out waiting for Deno process to exit"); + try + { + process.Kill(); + } + catch + { + // Kill might fail, either because the process already exited or due to some other error + Console.Error.WriteLine("Failure killing the Deno process - possible Zombie Deno.exe process"); + } + return WAIT_TIMEOUT; + } + + // If the Deno process wrote to STDERR - append it to our STDERR + if (!constraints.HasFlag(DenoConstraints.NoStderr)) + { + string error = stdErrTask.Result; + if (!string.IsNullOrWhiteSpace(error)) + { + Console.Error.WriteLine(error); + } + } + + return process.ExitCode; + + } + catch (Win32Exception ex) + { + Console.Error.WriteLine("Win32Exception: code = " + ex.ErrorCode + ", message: " + ex.Message); + return ex.NativeErrorCode; + } + catch (Exception ex) + { + Console.Error.WriteLine("Exception: message: " + ex.Message); + return -1; + } + } +} \ No newline at end of file diff --git a/cli/tests/DenoWinRunner.ps1 b/cli/tests/DenoWinRunner.ps1 new file mode 100644 index 0000000000..203b5d36ca --- /dev/null +++ b/cli/tests/DenoWinRunner.ps1 @@ -0,0 +1,10 @@ +$Source = [IO.File]::ReadAllText("$PSScriptRoot\DenoWinRunner.cs") +$denoExePath = $args[0] +$scriptPath = $args[1] +$constraints = $args[2] +$timeout = 5000; +Add-Type -TypeDefinition $Source -Language CSharp +Write-Output("Running Deno script: " + $args[1]) +$code = [DenoWinRunner]::RunDenoScript($denoExePath, $scriptPath, $constraints, $timeout) +Write-Output("Deno.exe or the test wrapper has exited with code: $code") +exit $code diff --git a/cli/tests/integration_tests.rs b/cli/tests/integration_tests.rs index 87a6f9d63c..2ec25d0d05 100644 --- a/cli/tests/integration_tests.rs +++ b/cli/tests/integration_tests.rs @@ -3116,3 +3116,62 @@ fn set_raw_should_not_panic_on_no_tty() { let stderr = std::str::from_utf8(&output.stderr).unwrap().trim(); assert!(stderr.contains("BadResource")); } + +#[cfg(windows)] +enum WinProcConstraints { + NoStdIn, + NoStdOut, + NoStdErr, +} + +#[cfg(windows)] +fn run_deno_script_constrained( + script_path: std::path::PathBuf, + constraints: WinProcConstraints, +) -> Result<(), i64> { + let file_path = "cli/tests/DenoWinRunner.ps1"; + let constraints = match constraints { + WinProcConstraints::NoStdIn => "1", + WinProcConstraints::NoStdOut => "2", + WinProcConstraints::NoStdErr => "4", + }; + let deno_exe_path = util::deno_exe_path() + .into_os_string() + .into_string() + .unwrap(); + + let deno_script_path = script_path.into_os_string().into_string().unwrap(); + + let args = vec![&deno_exe_path[..], &deno_script_path[..], constraints]; + util::run_powershell_script_file(file_path, args) +} + +#[cfg(windows)] +#[test] +fn should_not_panic_on_no_stdin() { + let output = run_deno_script_constrained( + util::tests_path().join("echo.ts"), + WinProcConstraints::NoStdIn, + ); + output.unwrap(); +} + +#[cfg(windows)] +#[test] +fn should_not_panic_on_no_stdout() { + let output = run_deno_script_constrained( + util::tests_path().join("echo.ts"), + WinProcConstraints::NoStdOut, + ); + output.unwrap(); +} + +#[cfg(windows)] +#[test] +fn should_not_panic_on_no_stderr() { + let output = run_deno_script_constrained( + util::tests_path().join("echo.ts"), + WinProcConstraints::NoStdErr, + ); + output.unwrap(); +} diff --git a/cli/worker.rs b/cli/worker.rs index 4ac1ee6a62..08367da91a 100644 --- a/cli/worker.rs +++ b/cli/worker.rs @@ -300,9 +300,15 @@ impl MainWorker { let state_rc = CoreIsolate::state(&worker.isolate); let state = state_rc.borrow(); let mut t = state.resource_table.borrow_mut(); - t.add("stdin", Box::new(stdin)); - t.add("stdout", Box::new(stdout)); - t.add("stderr", Box::new(stderr)); + if let Some(stream) = stdin { + t.add("stdin", Box::new(stream)); + } + if let Some(stream) = stdout { + t.add("stdout", Box::new(stream)); + } + if let Some(stream) = stderr { + t.add("stderr", Box::new(stream)); + } } worker.execute("bootstrap.mainRuntime()")?; Ok(worker) diff --git a/test_util/src/lib.rs b/test_util/src/lib.rs index fb7b8f88e0..73a1df043d 100644 --- a/test_util/src/lib.rs +++ b/test_util/src/lib.rs @@ -591,6 +591,37 @@ pub fn run_python_script(script: &str) { } } +pub fn run_powershell_script_file( + script_file_path: &str, + args: Vec<&str>, +) -> Result<(), i64> { + let deno_dir = new_deno_dir(); + let mut command = Command::new("powershell.exe"); + + command + .env("DENO_DIR", deno_dir.path()) + .current_dir(root_path()) + .arg("-file") + .arg(script_file_path); + + for arg in args { + command.arg(arg); + } + + let output = command.output().expect("failed to spawn script"); + let stdout = String::from_utf8(output.stdout).unwrap(); + let stderr = String::from_utf8(output.stderr).unwrap(); + println!("{}", stdout); + if !output.status.success() { + panic!( + "{} executed with failing error code\n{}{}", + script_file_path, stdout, stderr + ); + } + + Ok(()) +} + #[derive(Debug, Default)] pub struct CheckOutputIntegrationTest { pub args: &'static str,