Skip to content

Commit

Permalink
Replace atty dependency with std::io::IsTerminal. (#4790)
Browse files Browse the repository at this point in the history
The `atty` library has [a GitHub security
advisory](GHSA-g98v-hv3f-hcfr) noting that
it is unmaintained and can be unsound under some conditions. Its
functionality can be replaced with `std::io::IsTerminal`; this PR does
that.

I also added a helper function which replaces the two identical uses;
this gives a single place to change the policy if desired, rather than
perhaps accidentally editing only one site.
  • Loading branch information
kpreid authored Jan 12, 2024
1 parent e654f0d commit c027450
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 27 deletions.
21 changes: 0 additions & 21 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ array-init = "2.1"
arrow2 = "0.17"
arrow2_convert = "0.5.0"
async-executor = "1.0"
atty = "0.2"
backtrace = "0.3"
bincode = "1.3"
bitflags = { version = "2.4", features = ["bytemuck"] }
Expand Down
1 change: 0 additions & 1 deletion crates/re_sdk/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ re_sdk_comms = { workspace = true, features = ["client"] }
re_types_core.workspace = true

ahash.workspace = true
atty.workspace = true
crossbeam.workspace = true
document-features.workspace = true
once_cell.workspace = true
Expand Down
13 changes: 9 additions & 4 deletions crates/re_sdk/src/recording_stream.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::fmt;
use std::io::IsTerminal;
use std::sync::{atomic::AtomicI64, Arc};

use ahash::HashMap;
Expand Down Expand Up @@ -37,6 +38,12 @@ fn forced_sink_path() -> Option<String> {
std::env::var(ENV_FORCE_SAVE).ok()
}

/// Should we stream data to stdout when requested, or should we refrain because it's actually a
/// terminal?
fn is_stdout_listening() -> bool {
!std::io::stdout().is_terminal()
}

/// Errors that can occur when creating/manipulating a [`RecordingStream`].
#[derive(thiserror::Error, Debug)]
pub enum RecordingStreamError {
Expand Down Expand Up @@ -368,8 +375,7 @@ impl RecordingStreamBuilder {
/// ```
#[cfg(not(target_arch = "wasm32"))]
pub fn stdout(self) -> RecordingStreamResult<RecordingStream> {
let is_stdout_listening = !atty::is(atty::Stream::Stdout);
if !is_stdout_listening {
if !is_stdout_listening() {
return self.buffered();
}

Expand Down Expand Up @@ -1409,8 +1415,7 @@ impl RecordingStream {
return Ok(());
}

let is_stdout_listening = !atty::is(atty::Stream::Stdout);
if !is_stdout_listening {
if !is_stdout_listening() {
self.set_sink(Box::new(crate::log_sink::BufferedSink::new()));
return Ok(());
}
Expand Down

0 comments on commit c027450

Please sign in to comment.