tor-browser

commit d2d007a2f0ee39d9ada4dedc536373036b6174b6
parent 5ac7dbbc40d934bea8796e946befe8ac422d4283
Author: Alex Franchuk <afranchuk@mozilla.com>
Date:   Wed,  7 Jan 2026 15:24:58 +0000

Bug 2008797 - Avoid propagating crashreporter client settings deserialization errors r=gsvelto

Differential Revision: https://phabricator.services.mozilla.com/D278046

Diffstat:
M
toolkit/crashreporter/client/app/src/logic.rs
 | 
26
+++++++++++++++++++-------
M
toolkit/crashreporter/client/app/src/settings.rs
 | 
7
++++---
M
toolkit/crashreporter/client/app/src/test.rs
 | 
37
+++++++++++++++++++++++++++++++++++++

3 files changed, 60 insertions(+), 10 deletions(-)
diff --git a/toolkit/crashreporter/client/app/src/logic.rs b/toolkit/crashreporter/client/app/src/logic.rs
@@ -50,15 +50,27 @@ impl ReportCrash {
     pub fn new(config: Arc<Config>, extra: serde_json::Value) -> anyhow::Result<Self> {
         let settings_file = config.data_dir().join("crashreporter_settings.json");
         let settings: Settings = match std::fs::File::open(&settings_file) {
-            Err(e) if e.kind() != std::io::ErrorKind::NotFound => {
-                anyhow::bail!(
-                    "failed to open settings file ({}): {e}",
-                    settings_file.display()
-                );
+            Err(e) => {
+                if e.kind() != std::io::ErrorKind::NotFound {
+                    log::warn!(
+                        "failed to open settings file ({}): {e}",
+                        settings_file.display()
+                    );
+                }
+                Default::default()
             }
-            Err(_) => Default::default(),
-            Ok(f) => Settings::from_reader(f)?,
+            Ok(f) => match Settings::from_reader(f) {
+                Err(e) => {
+                    log::warn!(
+                        "failed to read settings file ({}): {e}",
+                        settings_file.display()
+                    );
+                    Default::default()
+                }
+                Ok(s) => s,
+            },
         };
+        log::debug!("loaded settings: {settings:?}");
 
         Ok(ReportCrash {
             config,
diff --git a/toolkit/crashreporter/client/app/src/settings.rs b/toolkit/crashreporter/client/app/src/settings.rs
@@ -7,12 +7,13 @@
 #[derive(Debug, serde::Serialize, serde::Deserialize)]
 pub struct Settings {
     /// Whether a crash report should be sent.
+    #[serde(default = "default_true")]
     pub submit_report: bool,
     /// Whether the URL that was open should be included in a sent report.
+    #[serde(default = "default_true")]
     pub include_url: bool,
     /// Whether hardware tests (such as memory tests) are enabled
-    // This is a new field, so might be missing in previously stored settings
-    #[serde(default = "missing_test_hardware")]
+    #[serde(default = "default_true")]
     pub test_hardware: bool,
 }
 
@@ -43,6 +44,6 @@ impl Settings {
     }
 }
 
-fn missing_test_hardware() -> bool {
+fn default_true() -> bool {
     true
 }
diff --git a/toolkit/crashreporter/client/app/src/test.rs b/toolkit/crashreporter/client/app/src/test.rs
@@ -1114,6 +1114,43 @@ fn persistent_settings() {
 }
 
 #[test]
+fn partial_settings_default() {
+    let mut test = GuiTest::new();
+    test.files.add_dir("data_dir").add_file(
+        "data_dir/crashreporter_settings.json",
+        "{\"include_url\":false}",
+    );
+    test.run(|interact| {
+        interact.element("quit", |_style, b: &model::Button| b.click.fire(&()));
+    });
+    test.assert_files()
+        .saved_settings(Settings {
+            submit_report: true,
+            include_url: false,
+            test_hardware: true,
+        })
+        .submitted();
+}
+
+#[test]
+fn corrupt_settings_default() {
+    let mut test = GuiTest::new();
+    test.files
+        .add_dir("data_dir")
+        .add_file("data_dir/crashreporter_settings.json", "not valid json");
+    test.run(|interact| {
+        interact.element("quit", |_style, b: &model::Button| b.click.fire(&()));
+    });
+    test.assert_files()
+        .saved_settings(Settings {
+            submit_report: true,
+            include_url: true,
+            test_hardware: true,
+        })
+        .submitted();
+}
+
+#[test]
 fn send_memtest_output() {
     let mut test = GuiTest::new();
     test.config.run_memtest = true;