Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: fix flaky stdio_streams_are_locked_in_permission_prompt #19443

Merged
merged 1 commit into from
Jun 9, 2023
Merged

chore: fix flaky stdio_streams_are_locked_in_permission_prompt #19443

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cli/tests/integration/run_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4370,6 4370,7 @@ fn stdio_streams_are_locked_in_permission_prompt() {
std::thread::sleep(Duration::from_millis(50)); // give the other thread some time to output
console.write_line_raw("invalid");
console.expect("Unrecognized option.");
console.expect("Allow? [y/n/A] (y = yes, allow; n = no, deny; A = allow all write permissions)");
console.write_line_raw("y");
console.expect("Granted write access to");

Expand Down
1 change: 0 additions & 1 deletion cli/tests/testdata/run/stdio_streams_are_locked_in_permission_prompt/worker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 1 @@
console.clear();
Copy link
Member Author

@dsherret dsherret Jun 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not necessary to properly test this and was causing the flakiness

console.log("Are you sure you want to continue?");
88 changes: 55 additions & 33 deletions runtime/permissions/prompter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 5,10 @@ use deno_core::error::AnyError;
use deno_core::parking_lot::Mutex;
use once_cell::sync::Lazy;
use std::fmt::Write;
use std::io::BufRead;
use std::io::StderrLock;
use std::io::StdinLock;
use std::io::Write as IoWrite;

/// Helper function to strip ansi codes and ASCII control characters.
fn strip_ansi_codes_and_ascii_control(s: &str) -> std::borrow::Cow<str> {
Expand Down Expand Up @@ -85,7 89,10 @@ impl PermissionPrompter for TtyPrompter {
};

#[cfg(unix)]
fn clear_stdin() -> Result<(), AnyError> {
fn clear_stdin(
_stdin_lock: &mut StdinLock,
_stderr_lock: &mut StderrLock,
) -> Result<(), AnyError> {
// TODO(bartlomieju):
#[allow(clippy::undocumented_unsafe_blocks)]
let r = unsafe { libc::tcflush(0, libc::TCIFLUSH) };
Expand All @@ -94,7 101,10 @@ impl PermissionPrompter for TtyPrompter {
}

#[cfg(not(unix))]
fn clear_stdin() -> Result<(), AnyError> {
fn clear_stdin(
stdin_lock: &mut StdinLock,
stderr_lock: &mut StderrLock,
) -> Result<(), AnyError> {
use deno_core::anyhow::bail;
use winapi::shared::minwindef::TRUE;
use winapi::shared::minwindef::UINT;
Expand All @@ -118,11 128,11 @@ impl PermissionPrompter for TtyPrompter {
// emulate an enter key press to clear any line buffered console characters
emulate_enter_key_press(stdin)?;
// read the buffered line or enter key press
read_stdin_line()?;
read_stdin_line(stdin_lock)?;
// check if our emulated key press was executed
if is_input_buffer_empty(stdin)? {
// if so, move the cursor up to prevent a blank line
move_cursor_up()?;
move_cursor_up(stderr_lock)?;
} else {
// the emulated key press is still pending, so a buffered line was read
// and we can flush the emulated key press
Expand Down Expand Up @@ -181,37 191,36 @@ impl PermissionPrompter for TtyPrompter {
Ok(events_read == 0)
}

fn move_cursor_up() -> Result<(), AnyError> {
use std::io::Write;
write!(std::io::stderr(), "\x1B[1A")?;
fn move_cursor_up(stderr_lock: &mut StderrLock) -> Result<(), AnyError> {
write!(stderr_lock, "\x1B[1A")?;
Ok(())
}

fn read_stdin_line() -> Result<(), AnyError> {
fn read_stdin_line(stdin_lock: &mut StdinLock) -> Result<(), AnyError> {
let mut input = String::new();
let stdin = std::io::stdin();
stdin.read_line(&mut input)?;
stdin_lock.read_line(&mut input)?;
Ok(())
}
}

// Clear n-lines in terminal and move cursor to the beginning of the line.
fn clear_n_lines(n: usize) {
eprint!("\x1B[{n}A\x1B[0J");
fn clear_n_lines(stderr_lock: &mut StderrLock, n: usize) {
write!(stderr_lock, "\x1B[{n}A\x1B[0J").unwrap();
}

// Lock stdio streams, so no other output is written while the prompt is
// displayed.
let stdout_lock = std::io::stdout().lock();
let mut stderr_lock = std::io::stderr().lock();
let mut stdin_lock = std::io::stdin().lock();

// For security reasons we must consume everything in stdin so that previously
// buffered data cannot effect the prompt.
if let Err(err) = clear_stdin() {
// buffered data cannot affect the prompt.
if let Err(err) = clear_stdin(&mut stdin_lock, &mut stderr_lock) {
eprintln!("Error clearing stdin for permission prompt. {err:#}");
return PromptResponse::Deny; // don't grant permission if this fails
}

// Lock stdio streams, so no other output is written while the prompt is
// displayed.
let _stdout_guard = std::io::stdout().lock();
let _stderr_guard = std::io::stderr().lock();

let message = strip_ansi_codes_and_ascii_control(message);
let name = strip_ansi_codes_and_ascii_control(name);
let api_name = api_name.map(strip_ansi_codes_and_ascii_control);
Expand All @@ -238,13 247,12 @@ impl PermissionPrompter for TtyPrompter {
write!(&mut output, "└ {}", colors::bold("Allow?")).unwrap();
write!(&mut output, " {opts} > ").unwrap();

eprint!("{}", output);
stderr_lock.write_all(output.as_bytes()).unwrap();
}

let value = loop {
let mut input = String::new();
let stdin = std::io::stdin();
let result = stdin.read_line(&mut input);
let result = stdin_lock.read_line(&mut input);
if result.is_err() {
break PromptResponse::Deny;
};
Expand All @@ -254,34 262,48 @@ impl PermissionPrompter for TtyPrompter {
};
match ch {
'y' | 'Y' => {
clear_n_lines(if api_name.is_some() { 4 } else { 3 });
clear_n_lines(
&mut stderr_lock,
if api_name.is_some() { 4 } else { 3 },
);
let msg = format!("Granted {message}.");
eprintln!("✅ {}", colors::bold(&msg));
writeln!(stderr_lock, "✅ {}", colors::bold(&msg)).unwrap();
break PromptResponse::Allow;
}
'n' | 'N' => {
clear_n_lines(if api_name.is_some() { 4 } else { 3 });
clear_n_lines(
&mut stderr_lock,
if api_name.is_some() { 4 } else { 3 },
);
let msg = format!("Denied {message}.");
eprintln!("❌ {}", colors::bold(&msg));
writeln!(stderr_lock, "❌ {}", colors::bold(&msg)).unwrap();
break PromptResponse::Deny;
}
'A' if is_unary => {
clear_n_lines(if api_name.is_some() { 4 } else { 3 });
clear_n_lines(
&mut stderr_lock,
if api_name.is_some() { 4 } else { 3 },
);
let msg = format!("Granted all {name} access.");
eprintln!("✅ {}", colors::bold(&msg));
writeln!(stderr_lock, "✅ {}", colors::bold(&msg)).unwrap();
break PromptResponse::AllowAll;
}
_ => {
// If we don't get a recognized option try again.
clear_n_lines(1);
eprint!("└ {}", colors::bold("Unrecognized option. Allow?"));
eprint!(" {opts} > ");
clear_n_lines(&mut stderr_lock, 1);
write!(
stderr_lock,
"└ {} {opts} > ",
colors::bold("Unrecognized option. Allow?")
)
.unwrap();
}
};
};

drop(_stdout_guard);
drop(_stderr_guard);
drop(stdout_lock);
drop(stderr_lock);
drop(stdin_lock);

value
}
Expand Down