Skip to content

Commit

Permalink
Merge pull request #1043 from Dexterp37/fix_empty_appid
Browse files Browse the repository at this point in the history
Bug 1649925 - Ensure Glean does not initialize with an empty application id
  • Loading branch information
Dexterp37 authored Jul 8, 2020
2 parents 002f0f4 + fba6e3e commit 202fc44
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
4 changes: 4 additions & 0 deletions glean-core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ pub enum ErrorKind {
/// Unknown error
Utf8Error,

/// Glean initialization was attempted with an invalid configuration
InvalidConfig,

/// Glean not initialized
NotInitialized,

Expand Down Expand Up @@ -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"),
}
Expand Down
5 changes: 4 additions & 1 deletion glean-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down
11 changes: 11 additions & 0 deletions glean-core/src/lib_unit_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

0 comments on commit 202fc44

Please sign in to comment.