From fba6e3e1d837c0adc11d69ca4f4e0e39cca790fc Mon Sep 17 00:00:00 2001 From: Alessio Placitelli Date: Wed, 8 Jul 2020 12:01:16 +0200 Subject: [PATCH] Ensure Glean does not initialize with an empty application id Using an empty application id makes it impossible to upload pings to ingestion, since the application id is used to build the submission path. --- CHANGELOG.md | 1 + glean-core/src/error.rs | 4 ++++ glean-core/src/lib.rs | 5 ++++- glean-core/src/lib_unit_tests.rs | 11 +++++++++++ 4 files changed, 20 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 43ca3f7dda..100e68b2de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ * General * Remove locale from baseline ping. ([1609968](https://bugzilla.mozilla.org/show_bug.cgi?id=1609968), [#1016](https://github.com/mozilla/glean/pull/1016)) * Persist X-Debug-ID header on store ping. ([1605097](https://bugzilla.mozilla.org/show_bug.cgi?id=1605097), [#1042](https://github.com/mozilla/glean/pull/1042)) + * BUGFIX: raise an error if Glean is initialized with an empty string as the `application_id`. * Python * BUGFIX: correctly set the `app_build` metric to the newly provided `application_build_id` initialization option. diff --git a/glean-core/src/error.rs b/glean-core/src/error.rs index 8ce8d8f73a..b56cceea41 100644 --- a/glean-core/src/error.rs +++ b/glean-core/src/error.rs @@ -54,6 +54,9 @@ pub enum ErrorKind { /// Unknown error Utf8Error, + /// Glean initialization was attempted with an invalid configuration + InvalidConfig, + /// Glean not initialized NotInitialized, @@ -103,6 +106,7 @@ impl Display for Error { HistogramType(h) => write!(f, "HistogramType conversion from {} failed", h), OsString(s) => write!(f, "OsString conversion from {:?} failed", s), Utf8Error => write!(f, "Invalid UTF-8 byte sequence in string"), + InvalidConfig => write!(f, "Invalid Glean configuration provided"), NotInitialized => write!(f, "Global Glean object missing"), __NonExhaustive => write!(f, "Unknown error"), } diff --git a/glean-core/src/lib.rs b/glean-core/src/lib.rs index 53d67e3ec1..ec51336acd 100644 --- a/glean-core/src/lib.rs +++ b/glean-core/src/lib.rs @@ -42,7 +42,7 @@ mod util; pub use crate::common_metric_data::{CommonMetricData, Lifetime}; use crate::database::Database; -pub use crate::error::{Error, Result}; +pub use crate::error::{Error, ErrorKind, Result}; pub use crate::error_recording::{test_get_num_recorded_errors, ErrorType}; use crate::event_database::EventDatabase; use crate::internal_metrics::CoreMetrics; @@ -188,6 +188,9 @@ impl Glean { log::info!("Creating new Glean v{}", GLEAN_VERSION); let application_id = sanitize_application_id(&cfg.application_id); + if application_id.is_empty() { + return Err(ErrorKind::InvalidConfig.into()); + } // Creating the data store creates the necessary path as well. // If that fails we bail out and don't initialize further. diff --git a/glean-core/src/lib_unit_tests.rs b/glean-core/src/lib_unit_tests.rs index ffe9f45b53..0ad5ad17f5 100644 --- a/glean-core/src/lib_unit_tests.rs +++ b/glean-core/src/lib_unit_tests.rs @@ -778,3 +778,14 @@ fn test_setting_debug_view_tag() { assert_eq!(false, glean.set_debug_view_tag(invalid_tag)); assert_eq!(valid_tag, glean.debug_view_tag().unwrap()); } + +#[test] +#[should_panic] +fn test_empty_application_id() { + let dir = tempfile::tempdir().unwrap(); + let tmpname = dir.path().display().to_string(); + + let glean = Glean::with_options(&tmpname, "", true).unwrap(); + // Check that this is indeed the first run. + assert!(glean.is_first_run()); +}