From 709e81565c76ed4ec131433f963fdbb6b94249cc Mon Sep 17 00:00:00 2001 From: Ana Hobden Date: Mon, 27 Mar 2023 11:56:44 -0700 Subject: [PATCH] Fixup diagnostic_endpoint setting to be more flexible (#374) * Fixup diagnostic_endpoint setting to be more flexible * Fixup doctests --- src/action/mod.rs | 2 +- src/diagnostics.rs | 40 +++++++++++++++++++++++++++++++++++---- src/planner/linux.rs | 2 +- src/planner/macos.rs | 2 +- src/planner/mod.rs | 7 ++++++- src/planner/steam_deck.rs | 2 +- src/settings.rs | 12 ++++++------ 7 files changed, 52 insertions(+), 15 deletions(-) diff --git a/src/action/mod.rs b/src/action/mod.rs index 17703af..c21d555 100644 --- a/src/action/mod.rs +++ b/src/action/mod.rs @@ -163,7 +163,7 @@ impl Planner for MyPlanner { .into_keys() .collect::>(), self.common.ssl_cert_file.clone(), - )) + )?) } } diff --git a/src/diagnostics.rs b/src/diagnostics.rs index 6788305..5d71220 100644 --- a/src/diagnostics.rs +++ b/src/diagnostics.rs @@ -65,18 +65,22 @@ pub struct DiagnosticData { impl DiagnosticData { pub fn new( - endpoint: Option, + endpoint: Option, planner: String, configured_settings: Vec, ssl_cert_file: Option, - ) -> Self { + ) -> Result { + let endpoint = match endpoint { + Some(endpoint) => diagnostic_endpoint_parser(&endpoint)?, + None => None, + }; let (os_name, os_version) = match OsRelease::new() { Ok(os_release) => (os_release.name, os_release.version), Err(_) => ("unknown".into(), "unknown".into()), }; let is_ci = is_ci::cached() || std::env::var("NIX_INSTALLER_CI").unwrap_or_else(|_| "0".into()) == "1"; - Self { + Ok(Self { endpoint, version: env!("CARGO_PKG_VERSION").into(), planner, @@ -87,7 +91,7 @@ impl DiagnosticData { is_ci, ssl_cert_file, failure_chain: None, - } + }) } pub fn failure(mut self, err: &NixInstallerError) -> Self { @@ -215,6 +219,13 @@ pub enum DiagnosticError { #[source] reqwest::Error, ), + /// Parsing URL + #[error("Parsing URL")] + Parse( + #[source] + #[from] + url::ParseError, + ), #[error("Write path `{0}`")] Write(std::path::PathBuf, #[source] std::io::Error), #[error("Serializing receipt")] @@ -237,3 +248,24 @@ impl ErrorDiagnostic for DiagnosticError { return static_str.to_string(); } } + +pub fn diagnostic_endpoint_parser(input: &str) -> Result, DiagnosticError> { + match Url::parse(input) { + Ok(v) => match v.scheme() { + "https" | "http" | "file" => Ok(Some(v)), + _ => Err(DiagnosticError::UnknownUrlScheme), + }, + Err(url_error) if url_error == url::ParseError::RelativeUrlWithoutBase => { + match Url::parse(&format!("file://{input}")) { + Ok(v) => Ok(Some(v)), + Err(file_error) => Err(file_error)?, + } + }, + Err(url_error) => Err(url_error)?, + } +} + +pub fn diagnostic_endpoint_validator(input: &str) -> Result { + let _ = diagnostic_endpoint_parser(input)?; + Ok(input.to_string()) +} diff --git a/src/planner/linux.rs b/src/planner/linux.rs index d7c8ca2..18f386a 100644 --- a/src/planner/linux.rs +++ b/src/planner/linux.rs @@ -135,7 +135,7 @@ impl Planner for Linux { .into_keys() .collect::>(), self.settings.ssl_cert_file.clone(), - )) + )?) } } diff --git a/src/planner/macos.rs b/src/planner/macos.rs index 93fe66f..c351095 100644 --- a/src/planner/macos.rs +++ b/src/planner/macos.rs @@ -211,7 +211,7 @@ impl Planner for Macos { .into_keys() .collect::>(), self.settings.ssl_cert_file.clone(), - )) + )?) } } diff --git a/src/planner/mod.rs b/src/planner/mod.rs index 8417bda..4f50f3a 100644 --- a/src/planner/mod.rs +++ b/src/planner/mod.rs @@ -79,7 +79,7 @@ impl Planner for MyPlanner { .into_keys() .collect::>(), self.common.ssl_cert_file.clone(), - )) + )?) } } @@ -317,6 +317,9 @@ pub enum PlannerError { NixExists, #[error("WSL1 is not supported, please upgrade to WSL2: https://learn.microsoft.com/en-us/windows/wsl/install#upgrade-version-from-wsl-1-to-wsl-2")] Wsl1, + #[cfg(feature = "diagnostics")] + #[error(transparent)] + Diagnostic(#[from] crate::diagnostics::DiagnosticError), } impl HasExpectedErrors for PlannerError { @@ -334,6 +337,8 @@ impl HasExpectedErrors for PlannerError { this @ PlannerError::NixOs => Some(Box::new(this)), this @ PlannerError::NixExists => Some(Box::new(this)), this @ PlannerError::Wsl1 => Some(Box::new(this)), + #[cfg(feature = "diagnostics")] + PlannerError::Diagnostic(diagnostic_error) => Some(Box::new(diagnostic_error)), } } } diff --git a/src/planner/steam_deck.rs b/src/planner/steam_deck.rs index 95d3ad0..528cc0b 100644 --- a/src/planner/steam_deck.rs +++ b/src/planner/steam_deck.rs @@ -287,7 +287,7 @@ impl Planner for SteamDeck { .into_keys() .collect::>(), self.settings.ssl_cert_file.clone(), - )) + )?) } } diff --git a/src/settings.rs b/src/settings.rs index 6a66274..834a95f 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -214,14 +214,16 @@ pub struct CommonSettings { /// "status": "Success" /// } /// - /// To disable diagnostic reporting, unset the default with `--diagnostic-endpoint=` + /// To disable diagnostic reporting, unset the default with `--diagnostic-endpoint ""`, or `NIX_INSTALLER_DIAGNOSTIC_ENDPOINT=""` #[clap( long, env = "NIX_INSTALLER_DIAGNOSTIC_ENDPOINT", global = true, - default_value = "https://install.determinate.systems/nix/diagnostic" + value_parser = crate::diagnostics::diagnostic_endpoint_validator, + num_args = 0..=1, // Required to allow `--diagnostic-endpoint` or `NIX_INSTALLER_DIAGNOSTIC_ENDPOINT=""` + default_missing_value = "https://install.determinate.systems/nix/diagnostic" )] - pub diagnostic_endpoint: Option, + pub diagnostic_endpoint: Option, } impl CommonSettings { @@ -285,9 +287,7 @@ impl CommonSettings { force: false, ssl_cert_file: Default::default(), #[cfg(feature = "diagnostics")] - diagnostic_endpoint: Some( - "https://install.determinate.systems/nix/diagnostic".try_into()?, - ), + diagnostic_endpoint: Some("https://install.determinate.systems/nix/diagnostic".into()), }) }