Skip to content

Commit

Permalink
Ensure Glean does not initialize with an empty application id
Browse files Browse the repository at this point in the history
Using an empty application id makes it impossible to upload pings
to ingestion, since the application id is used to build the submission
path.
  • Loading branch information
Dexterp37 committed Jul 8, 2020
1 parent 002f0f4 commit fba6e3e
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 fba6e3e

Please sign in to comment.