github-mirrors/librespot
1
0
Fork 0
mirror of https://github.com/librespot-org/librespot.git synced 2024-12-18 17:11:53 +00:00
Code Issues Projects Releases Packages Wiki Activity

Fix Command line arguments incorrectly echoed in TRACE

Browse source 
Fix up for #886
Closes: #898

And...

* Don't silently ignore non-Unicode while parsing env vars.

* Iterating over `std::env::args` will panic! on invalid unicode. Let's not do that. `getopts` will catch missing args and exit if those args are required after our error message about the arg not being valid unicode.

* Gaurd against empty strings. There are a few places while parsing options strings that we don't immediately evaluate their validity let's at least makes sure that they are not empty if present.

* `args` is only used in `get_setup` it doesn't need to be in main.

* Nicer help header.

* Get rid of `use std::io::{stderr, Write};` and just use `rpassword::prompt_password_stderr`.

* Get rid of `get_credentials` it was clunky, ugly and only used once. There is no need for it to be a separate function.

* Handle an empty password prompt and password prompt parsing errors.

* + Other random misc clean ups.
This commit is contained in:
JasonLG1979 2021-12-05 13:00:33 -06:00
parent 7160dc1017
commit 8c480b7e39
1 changed files with 240 additions and 175 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

415
src/main.rs
View file

@ -26,7 +26,6 @@ mod player_event_handler;
use player_event_handler::{emit_sink_event, run_program_on_events};
use std::env;
use std::io::{stderr, Write};
use std::ops::RangeInclusive;
use std::path::Path;
use std::pin::Pin;
@ -40,27 +39,16 @@ fn device_id(name: &str) -> String {
}
fn usage(program: &str, opts: &getopts::Options) -> String {
let brief = format!("Usage: {} [options]", program);
let repo_home = env!("CARGO_PKG_REPOSITORY");
let desc = env!("CARGO_PKG_DESCRIPTION");
let version = get_version_string();
let brief = format!(
"{}\n\n{}\n\n{}\n\nUsage: {} [<Options>]",
version, desc, repo_home, program
);
opts.usage(&brief)
}
fn arg_to_var(arg: &str) -> String {
// To avoid name collisions environment variables must be prepended
// with `LIBRESPOT_` so option/flag `foo-bar` becomes `LIBRESPOT_FOO_BAR`.
format!("LIBRESPOT_{}", arg.to_uppercase().replace("-", "_"))
}
fn env_var_present(arg: &str) -> bool {
env::var(arg_to_var(arg)).is_ok()
}
fn env_var_opt_str(option: &str) -> Option<String> {
match env::var(arg_to_var(option)) {
Ok(value) => Some(value),
Err(_) => None,
}
}
fn setup_logging(quiet: bool, verbose: bool) {
let mut builder = env_logger::Builder::new();
match env::var("RUST_LOG") {
@ -102,29 +90,6 @@ fn list_backends() {
}
}
pub fn get_credentials<F: FnOnce(&String) -> Option<String>>(
username: Option<String>,
password: Option<String>,
cached_credentials: Option<Credentials>,
prompt: F,
) -> Option<Credentials> {
if let Some(username) = username {
if let Some(password) = password {
return Some(Credentials::with_password(username, password));
}
match cached_credentials {
Some(credentials) if username == credentials.username => Some(credentials),
_ => {
let password = prompt(&username)?;
Some(Credentials::with_password(username, password))
}
}
} else {
cached_credentials
}
}
#[derive(Debug, Error)]
pub enum ParseFileSizeError {
#[error("empty argument")]
@ -218,9 +183,10 @@ struct Setup {
emit_sink_events: bool,
}
fn get_setup(args: &[String]) -> Setup {
const VALID_NORMALISATION_KNEE_RANGE: RangeInclusive<f64> = 0.0..=2.0;
fn get_setup() -> Setup {
const VALID_INITIAL_VOLUME_RANGE: RangeInclusive<u16> = 0..=100;
const VALID_VOLUME_RANGE: RangeInclusive<f64> = 0.0..=100.0;
const VALID_NORMALISATION_KNEE_RANGE: RangeInclusive<f64> = 0.0..=2.0;
const VALID_NORMALISATION_PREGAIN_RANGE: RangeInclusive<f64> = -10.0..=10.0;
const VALID_NORMALISATION_THRESHOLD_RANGE: RangeInclusive<f64> = -10.0..=0.0;
const VALID_NORMALISATION_ATTACK_RANGE: RangeInclusive<u64> = 1..=500;
@ -596,25 +562,72 @@ fn get_setup(args: &[String]) -> Setup {
"PORT",
);
let args: Vec<_> = std::env::args_os()
.filter_map(|s| match s.into_string() {
Ok(valid) => Some(valid),
Err(s) => {
eprintln!(
"Command line argument was not valid Unicode and will not be evaluated: {:?}",
s
);
None
}
})
.collect();
let matches = match opts.parse(&args[1..]) {
Ok(m) => m,
Err(f) => {
eprintln!(
"Error parsing command line options: {}\n{}",
f,
usage(&args[0], &opts)
);
Err(e) => {
eprintln!("Error parsing command line options: {}", e);
println!("\n{}", usage(&args[0], &opts));
exit(1);
}
};
let opt_present = |opt| matches.opt_present(opt) || env_var_present(opt);
let stripped_env_key = |k: &str| {
k.trim_start_matches("LIBRESPOT_")
.replace("_", "-")
.to_lowercase()
};
let env_vars: Vec<_> = env::vars_os().filter_map(|(k, v)| {
let mut env_var = None;
if let Ok(key) = k.into_string() {
if key.starts_with("LIBRESPOT_") {
let stripped_key = stripped_env_key(&key);
// Only match against long option/flag names.
// Something like LIBRESPOT_V for example is
// not valid because there are both -v and -V flags
// but env vars are assumed to be all uppercase.
let len = stripped_key.chars().count();
if len > 1 && matches.opt_defined(&stripped_key) {
match v.into_string() {
Ok(value) => {
env_var = Some((key, value));
},
Err(s) => {
eprintln!("Environment variable was not valid Unicode and will not be evaluated: {}={:?}", key, s);
}
}
}
}
}
env_var
})
.collect();
let opt_present =
|opt| matches.opt_present(opt) || env_vars.iter().any(|(k, _)| stripped_env_key(k) == opt);
let opt_str = |opt| {
if matches.opt_present(opt) {
matches.opt_str(opt)
} else {
env_var_opt_str(opt)
env_vars
.iter()
.find(|(k, _)| stripped_env_key(k) == opt)
.map(|(_, v)| v.to_string())
}
};
@ -632,55 +645,43 @@ fn get_setup(args: &[String]) -> Setup {
info!("{}", get_version_string());
let librespot_env_vars: Vec<String> = env::vars_os()
.filter_map(|(k, v)| {
let mut env_var = None;
if let Some(key) = k.to_str() {
if key.starts_with("LIBRESPOT_") {
if matches!(key, "LIBRESPOT_PASSWORD" | "LIBRESPOT_USERNAME") {
// Don't log creds.
env_var = Some(format!("\t\t{}=XXXXXXXX", key));
} else if let Some(value) = v.to_str() {
env_var = Some(format!("\t\t{}={}", key, value));
}
}
}
env_var
})
.collect();
if !librespot_env_vars.is_empty() {
if !env_vars.is_empty() {
trace!("Environment variable(s):");
for kv in librespot_env_vars {
trace!("{}", kv);
for (k, v) in &env_vars {
if matches!(k.as_str(), "LIBRESPOT_PASSWORD" | "LIBRESPOT_USERNAME") {
trace!("\t\t{}=\"XXXXXXXX\"", k);
} else if v.is_empty() {
trace!("\t\t{}=", k);
} else {
trace!("\t\t{}=\"{}\"", k, v);
}
}
}
let cmd_args = &args[1..];
let args_len = args.len();
let cmd_args_len = cmd_args.len();
if cmd_args_len > 0 {
if args_len > 1 {
trace!("Command line argument(s):");
for (index, key) in cmd_args.iter().enumerate() {
if key.starts_with('-') || key.starts_with("--") {
if matches!(key.as_str(), "--password" | "-p" | "--username" | "-u") {
// Don't log creds.
trace!("\t\t{} XXXXXXXX", key);
} else {
let mut value = "".to_string();
let next = index + 1;
if next < cmd_args_len {
let next_key = cmd_args[next].clone();
if !next_key.starts_with('-') && !next_key.starts_with("--") {
value = next_key;
}
}
for (index, key) in args.iter().enumerate() {
let opt = key.trim_start_matches('-');
trace!("\t\t{} {}", key, value);
if index > 0
&& &args[index - 1] != key
&& matches.opt_defined(opt)
&& matches.opt_present(opt)
{
if matches!(opt, PASSWORD | PASSWORD_SHORT | USERNAME | USERNAME_SHORT) {
// Don't log creds.
trace!("\t\t{} \"XXXXXXXX\"", key);
} else {
let value = matches.opt_str(opt).unwrap_or_else(|| "".to_string());
if value.is_empty() {
trace!("\t\t{}", key);
} else {
trace!("\t\t{} \"{}\"", key, value);
}
}
}
}
@ -707,7 +708,7 @@ fn get_setup(args: &[String]) -> Setup {
let backend = audio_backend::find(backend_name).unwrap_or_else(|| {
error!(
"Invalid `--{}` / `-{}`: {}",
"Invalid `--{}` / `-{}`: \"{}\"",
BACKEND,
BACKEND_SHORT,
opt_str(BACKEND).unwrap_or_default()
@ -720,7 +721,10 @@ fn get_setup(args: &[String]) -> Setup {
.as_deref()
.map(|format| {
AudioFormat::from_str(format).unwrap_or_else(|_| {
error!("Invalid `--{}` / `-{}`: {}", FORMAT, FORMAT_SHORT, format);
error!(
"Invalid `--{}` / `-{}`: \"{}\"",
FORMAT, FORMAT_SHORT, format
);
println!(
"Valid `--{}` / `-{}` values: F64, F32, S32, S24, S24_3, S16",
FORMAT, FORMAT_SHORT
@ -743,9 +747,17 @@ fn get_setup(args: &[String]) -> Setup {
feature = "rodio-backend",
feature = "portaudio-backend"
))]
if device == Some("?".into()) {
backend(device, format);
exit(0);
if let Some(ref value) = device {
if value == "?" {
backend(device, format);
exit(0);
} else if value.is_empty() {
error!(
"`--{}` / `-{}` can not be an empty string",
DEVICE, DEVICE_SHORT
);
exit(1);
}
}
#[cfg(not(any(
@ -774,7 +786,7 @@ fn get_setup(args: &[String]) -> Setup {
let mixer = mixer::find(mixer_type.as_deref()).unwrap_or_else(|| {
error!(
"Invalid `--{}` / `-{}`: {}",
"Invalid `--{}` / `-{}`: \"{}\"",
MIXER_TYPE,
MIXER_TYPE_SHORT,
opt_str(MIXER_TYPE).unwrap_or_default()
@ -807,7 +819,7 @@ fn get_setup(args: &[String]) -> Setup {
.map(|index| {
index.parse::<u32>().unwrap_or_else(|_| {
error!(
"Invalid `--{}` / `-{}`: {}",
"Invalid `--{}` / `-{}`: \"{}\"",
ALSA_MIXER_INDEX, ALSA_MIXER_INDEX_SHORT, index
);
println!("Default: {}", mixer_default_config.index);
@ -822,6 +834,15 @@ fn get_setup(args: &[String]) -> Setup {
#[cfg(feature = "alsa-backend")]
let control = opt_str(ALSA_MIXER_CONTROL).unwrap_or(mixer_default_config.control);
#[cfg(feature = "alsa-backend")]
if control.is_empty() {
error!(
"`--{}` / `-{}` can not be an empty string",
ALSA_MIXER_CONTROL, ALSA_MIXER_CONTROL_SHORT
);
exit(1);
}
#[cfg(not(feature = "alsa-backend"))]
let control = mixer_default_config.control;
@ -829,7 +850,7 @@ fn get_setup(args: &[String]) -> Setup {
.map(|range| {
let on_error = || {
error!(
"Invalid `--{}` / `-{}`: {}",
"Invalid `--{}` / `-{}`: \"{}\"",
VOLUME_RANGE, VOLUME_RANGE_SHORT, range
);
println!(
@ -871,7 +892,7 @@ fn get_setup(args: &[String]) -> Setup {
.map(|volume_ctrl| {
VolumeCtrl::from_str_with_range(volume_ctrl, volume_range).unwrap_or_else(|_| {
error!(
"Invalid `--{}` / `-{}`: {}",
"Invalid `--{}` / `-{}`: \"{}\"",
VOLUME_CTRL, VOLUME_CTRL_SHORT, volume_ctrl
);
println!(
@ -918,7 +939,7 @@ fn get_setup(args: &[String]) -> Setup {
.map(|e| {
e.unwrap_or_else(|e| {
error!(
"Invalid `--{}` / `-{}`: {}",
"Invalid `--{}` / `-{}`: \"{}\"",
CACHE_SIZE_LIMIT, CACHE_SIZE_LIMIT_SHORT, e
);
exit(1);
@ -945,20 +966,57 @@ fn get_setup(args: &[String]) -> Setup {
};
let credentials = {
let cached_credentials = cache.as_ref().and_then(Cache::credentials);
let cached_creds = cache.as_ref().and_then(Cache::credentials);
let password = |username: &String| -> Option<String> {
write!(stderr(), "Password for {}: ", username).ok()?;
stderr().flush().ok()?;
rpassword::read_password().ok()
};
if let Some(username) = opt_str(USERNAME) {
if username.is_empty() {
error!(
"`--{}` / `-{}` can not be an empty string",
USERNAME, USERNAME_SHORT
);
exit(1);
}
if let Some(password) = opt_str(PASSWORD) {
if password.is_empty() {
error!(
"`--{}` / `-{}` can not be an empty string",
PASSWORD, PASSWORD_SHORT
);
exit(1);
}
Some(Credentials::with_password(username, password))
} else {
match cached_creds {
Some(creds) if username == creds.username => Some(creds),
_ => {
let prompt = &format!("Password for {}: ", username);
get_credentials(
opt_str(USERNAME),
opt_str(PASSWORD),
cached_credentials,
password,
)
match rpassword::prompt_password_stderr(prompt) {
Ok(password) => {
if !password.is_empty() {
Some(Credentials::with_password(username, password))
} else {
trace!("Password was empty.");
if cached_creds.is_some() {
trace!("Using cached credentials.");
}
cached_creds
}
}
Err(e) => {
warn!("Cannot parse password: {}", e);
if cached_creds.is_some() {
trace!("Using cached credentials.");
}
cached_creds
}
}
}
}
}
} else {
cached_creds
}
};
let enable_discovery = !opt_present(DISABLE_DISCOVERY);
@ -980,12 +1038,14 @@ fn get_setup(args: &[String]) -> Setup {
.map(|port| {
let on_error = || {
error!(
"Invalid `--{}` / `-{}`: {}",
"Invalid `--{}` / `-{}`: \"{}\"",
ZEROCONF_PORT, ZEROCONF_PORT_SHORT, port
);
println!(
"Valid `--{}` / `-{}` values: 1 - 65535",
ZEROCONF_PORT, ZEROCONF_PORT_SHORT
"Valid `--{}` / `-{}` values: 1 - {}",
ZEROCONF_PORT,
ZEROCONF_PORT_SHORT,
u16::MAX
);
};
@ -1011,16 +1071,27 @@ fn get_setup(args: &[String]) -> Setup {
let name = opt_str(NAME).unwrap_or_else(|| connect_default_config.name.clone());
if name.is_empty() {
error!(
"`--{}` / `-{}` can not be an empty string",
NAME, NAME_SHORT
);
exit(1);
}
let initial_volume = opt_str(INITIAL_VOLUME)
.map(|initial_volume| {
let on_error = || {
error!(
"Invalid `--{}` / `-{}`: {}",
"Invalid `--{}` / `-{}`: \"{}\"",
INITIAL_VOLUME, INITIAL_VOLUME_SHORT, initial_volume
);
println!(
"Valid `--{}` / `-{}` values: 0 - 100",
INITIAL_VOLUME, INITIAL_VOLUME_SHORT
"Valid `--{}` / `-{}` values: {} - {}",
INITIAL_VOLUME,
INITIAL_VOLUME_SHORT,
VALID_INITIAL_VOLUME_RANGE.start(),
VALID_INITIAL_VOLUME_RANGE.end()
);
#[cfg(feature = "alsa-backend")]
println!(
@ -1039,7 +1110,7 @@ fn get_setup(args: &[String]) -> Setup {
exit(1);
});
if volume > 100 {
if !(VALID_INITIAL_VOLUME_RANGE).contains(&volume) {
on_error();
exit(1);
}
@ -1056,7 +1127,7 @@ fn get_setup(args: &[String]) -> Setup {
.as_deref()
.map(|device_type| {
DeviceType::from_str(device_type).unwrap_or_else(|_| {
error!("Invalid `--{}` / `-{}`: {}", DEVICE_TYPE, DEVICE_TYPE_SHORT, device_type);
error!("Invalid `--{}` / `-{}`: \"{}\"", DEVICE_TYPE, DEVICE_TYPE_SHORT, device_type);
println!("Valid `--{}` / `-{}` values: computer, tablet, smartphone, speaker, tv, avr, stb, audiodongle, \
gameconsole, castaudio, castvideo, automobile, smartwatch, chromebook, carthing, homething",
DEVICE_TYPE, DEVICE_TYPE_SHORT
@ -1079,55 +1150,50 @@ fn get_setup(args: &[String]) -> Setup {
}
};
let session_config = {
let device_id = device_id(&connect_config.name);
SessionConfig {
user_agent: version::VERSION_STRING.to_string(),
device_id,
proxy: opt_str(PROXY).or_else(|| std::env::var("http_proxy").ok()).map(
|s| {
match Url::parse(&s) {
Ok(url) => {
if url.host().is_none() || url.port_or_known_default().is_none() {
error!("Invalid proxy url, only URLs on the format \"http://host:port\" are allowed");
exit(1);
}
if url.scheme() != "http" {
error!("Only unsecure http:// proxies are supported");
exit(1);
}
url
},
Err(e) => {
error!("Invalid proxy URL: {}, only URLs in the format \"http://host:port\" are allowed", e);
let session_config = SessionConfig {
user_agent: version::VERSION_STRING.to_string(),
device_id: device_id(&connect_config.name),
proxy: opt_str(PROXY).or_else(|| std::env::var("http_proxy").ok()).map(
|s| {
match Url::parse(&s) {
Ok(url) => {
if url.host().is_none() || url.port_or_known_default().is_none() {
error!("Invalid proxy url, only URLs on the format \"http://host:port\" are allowed");
exit(1);
}
}
},
),
ap_port: opt_str(AP_PORT)
.map(|port| {
let on_error = || {
error!("Invalid `--{}` / `-{}`: {}", AP_PORT, AP_PORT_SHORT, port);
println!("Valid `--{}` / `-{}` values: 1 - 65535", AP_PORT, AP_PORT_SHORT);
};
let port = port.parse::<u16>().unwrap_or_else(|_| {
on_error();
exit(1);
});
if url.scheme() != "http" {
error!("Only unsecure http:// proxies are supported");
exit(1);
}
if port == 0 {
on_error();
url
},
Err(e) => {
error!("Invalid proxy URL: \"{}\", only URLs in the format \"http://host:port\" are allowed", e);
exit(1);
}
}
},
),
ap_port: opt_str(AP_PORT).map(|port| {
let on_error = || {
error!("Invalid `--{}` / `-{}`: \"{}\"", AP_PORT, AP_PORT_SHORT, port);
println!("Valid `--{}` / `-{}` values: 1 - {}", AP_PORT, AP_PORT_SHORT, u16::MAX);
};
port
}),
}
let port = port.parse::<u16>().unwrap_or_else(|_| {
on_error();
exit(1);
});
if port == 0 {
on_error();
exit(1);
}
port
}),
};
let player_config = {
@ -1138,7 +1204,7 @@ fn get_setup(args: &[String]) -> Setup {
.map(|bitrate| {
Bitrate::from_str(bitrate).unwrap_or_else(|_| {
error!(
"Invalid `--{}` / `-{}`: {}",
"Invalid `--{}` / `-{}`: \"{}\"",
BITRATE, BITRATE_SHORT, bitrate
);
println!(
@ -1200,7 +1266,7 @@ fn get_setup(args: &[String]) -> Setup {
let method = NormalisationMethod::from_str(method).unwrap_or_else(|_| {
error!(
"Invalid `--{}` / `-{}`: {}",
"Invalid `--{}` / `-{}`: \"{}\"",
NORMALISATION_METHOD, NORMALISATION_METHOD_SHORT, method
);
println!(
@ -1227,7 +1293,7 @@ fn get_setup(args: &[String]) -> Setup {
.map(|gain_type| {
NormalisationType::from_str(gain_type).unwrap_or_else(|_| {
error!(
"Invalid `--{}` / `-{}`: {}",
"Invalid `--{}` / `-{}`: \"{}\"",
NORMALISATION_GAIN_TYPE, NORMALISATION_GAIN_TYPE_SHORT, gain_type
);
println!(
@ -1244,7 +1310,7 @@ fn get_setup(args: &[String]) -> Setup {
.map(|pregain| {
let on_error = || {
error!(
"Invalid `--{}` / `-{}`: {}",
"Invalid `--{}` / `-{}`: \"{}\"",
NORMALISATION_PREGAIN, NORMALISATION_PREGAIN_SHORT, pregain
);
println!(
@ -1275,7 +1341,7 @@ fn get_setup(args: &[String]) -> Setup {
.map(|threshold| {
let on_error = || {
error!(
"Invalid `--{}` / `-{}`: {}",
"Invalid `--{}` / `-{}`: \"{}\"",
NORMALISATION_THRESHOLD, NORMALISATION_THRESHOLD_SHORT, threshold
);
println!(
@ -1309,7 +1375,7 @@ fn get_setup(args: &[String]) -> Setup {
.map(|attack| {
let on_error = || {
error!(
"Invalid `--{}` / `-{}`: {}",
"Invalid `--{}` / `-{}`: \"{}\"",
NORMALISATION_ATTACK, NORMALISATION_ATTACK_SHORT, attack
);
println!(
@ -1343,7 +1409,7 @@ fn get_setup(args: &[String]) -> Setup {
.map(|release| {
let on_error = || {
error!(
"Invalid `--{}` / `-{}`: {}",
"Invalid `--{}` / `-{}`: \"{}\"",
NORMALISATION_RELEASE, NORMALISATION_RELEASE_SHORT, release
);
println!(
@ -1377,7 +1443,7 @@ fn get_setup(args: &[String]) -> Setup {
.map(|knee| {
let on_error = || {
error!(
"Invalid `--{}` / `-{}`: {}",
"Invalid `--{}` / `-{}`: \"{}\"",
NORMALISATION_KNEE, NORMALISATION_KNEE_SHORT, knee
);
println!(
@ -1418,7 +1484,7 @@ fn get_setup(args: &[String]) -> Setup {
Some(dither::find_ditherer(ditherer_name).unwrap_or_else(|| {
error!(
"Invalid `--{}` / `-{}`: {}",
"Invalid `--{}` / `-{}`: \"{}\"",
DITHER,
DITHER_SHORT,
opt_str(DITHER).unwrap_or_default()
@ -1488,8 +1554,7 @@ async fn main() {
env::set_var(RUST_BACKTRACE, "full")
}
let args: Vec<String> = std::env::args().collect();
let setup = get_setup(&args);
let setup = get_setup();
let mut last_credentials = None;
let mut spirc: Option<Spirc> = None;