From b29234c93b3c3f35b16df0b42b7a478e9516e5c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Saparelli?= Date: Mon, 13 Mar 2023 16:40:36 +1300 Subject: [PATCH] Clarify git credential code and fix infinite loop bug (#898) Fixes #897 --- crates/bin/src/args.rs | 61 ++-------------------------- crates/bin/src/git_credentials.rs | 67 +++++++++++++++++++++++++++++++ crates/bin/src/lib.rs | 1 + 3 files changed, 71 insertions(+), 58 deletions(-) create mode 100644 crates/bin/src/git_credentials.rs diff --git a/crates/bin/src/args.rs b/crates/bin/src/args.rs index 0122d87c..d234711b 100644 --- a/crates/bin/src/args.rs +++ b/crates/bin/src/args.rs @@ -1,7 +1,7 @@ use std::{ env, ffi::OsString, - fmt, fs, iter, + fmt, num::{NonZeroU64, ParseIntError}, path::PathBuf, str::FromStr, @@ -14,7 +14,7 @@ use binstalk::{ }; use clap::{error::ErrorKind, CommandFactory, Parser, ValueEnum}; use compact_str::CompactString; -use dirs::home_dir; + use log::LevelFilter; use semver::VersionReq; use strum::EnumCount; @@ -473,7 +473,7 @@ You cannot use --{option} and specify multiple packages at the same time. Do one if let Ok(github_token) = env::var("GH_TOKEN") { opts.github_token = Some(github_token.into()); } else if !opts.no_discover_github_token { - if let Some(github_token) = try_extract_from_git_credentials() { + if let Some(github_token) = crate::git_credentials::try_from_home() { opts.github_token = Some(github_token); } else if let Ok(github_token) = gh_token::get() { opts.github_token = Some(github_token.into()); @@ -484,41 +484,6 @@ You cannot use --{option} and specify multiple packages at the same time. Do one opts } -fn try_extract_from_git_credentials() -> Option { - home_dir() - .map(|mut home| { - home.push(".git-credentials"); - home - }) - .into_iter() - .chain(iter::from_fn(|| { - let home = env::var_os("XDG_CONFIG_HOME")?; - (!home.is_empty()).then(|| { - let mut path = PathBuf::from(home); - path.push("git/credentials"); - path - }) - })) - .find_map(try_extract_from_git_credentials_from) -} - -fn try_extract_from_git_credentials_from(path: PathBuf) -> Option { - fs::read_to_string(path) - .ok()? - .lines() - .find_map(extract_github_token_from_git_credentials_line) - .map(CompactString::from) -} - -fn extract_github_token_from_git_credentials_line(line: &str) -> Option<&str> { - let cred = line - .trim() - .strip_prefix("https://")? - .strip_suffix("@github.com")?; - - Some(cred.split_once(':')?.1) -} - #[cfg(test)] mod test { use super::*; @@ -527,24 +492,4 @@ mod test { fn verify_cli() { Args::command().debug_assert() } - - const GIT_CREDENTIALS_TEST_CASES: &[(&str, Option<&str>)] = &[ - // Success - ("https://NobodyXu:gho_asdc@github.com", Some("gho_asdc")), - ( - "https://NobodyXu:gho_asdc12dz@github.com", - Some("gho_asdc12dz"), - ), - // Failure - ("http://NobodyXu:gho_asdc@github.com", None), - ("https://NobodyXu:gho_asdc@gitlab.com", None), - ("https://NobodyXugho_asdc@github.com", None), - ]; - - #[test] - fn test_extract_github_token_from_git_credentials_line() { - GIT_CREDENTIALS_TEST_CASES.iter().for_each(|(line, res)| { - assert_eq!(extract_github_token_from_git_credentials_line(line), *res); - }) - } } diff --git a/crates/bin/src/git_credentials.rs b/crates/bin/src/git_credentials.rs new file mode 100644 index 00000000..7accd935 --- /dev/null +++ b/crates/bin/src/git_credentials.rs @@ -0,0 +1,67 @@ +use std::{ + env, fs, + path::{Path, PathBuf}, +}; + +use compact_str::CompactString; +use dirs::home_dir; + +pub fn try_from_home() -> Option { + if let Some(mut home) = home_dir() { + home.push(".git-credentials"); + if let Some(cred) = from_file(home) { + return Some(cred); + } + } + + if let Some(home) = env::var_os("XDG_CONFIG_HOME") { + let home = Path::new(&home).join("git/credentials"); + if let Some(cred) = from_file(home) { + return Some(cred); + } + } + + None +} + +fn from_file(path: PathBuf) -> Option { + fs::read_to_string(path) + .ok()? + .lines() + .find_map(from_line) + .map(CompactString::from) +} + +fn from_line(line: &str) -> Option<&str> { + let cred = line + .trim() + .strip_prefix("https://")? + .strip_suffix("@github.com")?; + + Some(cred.split_once(':')?.1) +} + +#[cfg(test)] +mod test { + use super::*; + + const GIT_CREDENTIALS_TEST_CASES: &[(&str, Option<&str>)] = &[ + // Success + ("https://NobodyXu:gho_asdc@github.com", Some("gho_asdc")), + ( + "https://NobodyXu:gho_asdc12dz@github.com", + Some("gho_asdc12dz"), + ), + // Failure + ("http://NobodyXu:gho_asdc@github.com", None), + ("https://NobodyXu:gho_asdc@gitlab.com", None), + ("https://NobodyXugho_asdc@github.com", None), + ]; + + #[test] + fn test_extract_from_line() { + GIT_CREDENTIALS_TEST_CASES.iter().for_each(|(line, res)| { + assert_eq!(from_line(line), *res); + }) + } +} diff --git a/crates/bin/src/lib.rs b/crates/bin/src/lib.rs index c1c70c59..57299fe9 100644 --- a/crates/bin/src/lib.rs +++ b/crates/bin/src/lib.rs @@ -1,6 +1,7 @@ pub mod args; pub mod bin_util; pub mod entry; +pub mod git_credentials; pub mod install_path; pub mod logging; pub mod manifests;