Bug 1390547 - Escape MozCrashReason when writing out a crash report r=ted
authorGabriele Svelto <gsvelto@mozilla.com>
Thu, 11 Oct 2018 11:15:02 +0000
changeset 496427 b791c9a47d33088319ad53b32d0756ec014dfdb7
parent 496426 06f4b6597f74d01d0dce767a83ed537612cb7173
child 496428 b7fc0fb0f0e26404469cdddeef541b8154835ec0
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersted
bugs1390547
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1390547 - Escape MozCrashReason when writing out a crash report r=ted Differential Revision: https://phabricator.services.mozilla.com/D7256
toolkit/crashreporter/nsExceptionHandler.cpp
toolkit/crashreporter/test/unit/test_crash_rust_panic_multiline.js
toolkit/crashreporter/test/unit/xpcshell.ini
toolkit/crashreporter/test/unit_ipc/test_content_rust_panic_multiline.js
toolkit/crashreporter/test/unit_ipc/xpcshell.ini
--- a/toolkit/crashreporter/nsExceptionHandler.cpp
+++ b/toolkit/crashreporter/nsExceptionHandler.cpp
@@ -882,16 +882,52 @@ LaunchCrashHandlerService(XP_CHAR* aProg
     Unused << HANDLE_EINTR(sys_waitpid(pid, &status, __WALL));
   }
 
   return true;
 }
 
 #endif
 
+void
+WriteEscapedMozCrashReason(PlatformWriter& aWriter)
+{
+  const char *reason;
+  size_t len;
+  char *rust_panic_reason;
+  bool rust_panic = get_rust_panic_reason(&rust_panic_reason, &len);
+
+  if (rust_panic) {
+    reason = rust_panic_reason;
+  } else if (gMozCrashReason != nullptr) {
+    reason = gMozCrashReason;
+    len = strlen(reason);
+  } else {
+    return; // No crash reason, bail out
+  }
+
+  WriteString(aWriter, AnnotationToString(Annotation::MozCrashReason));
+  WriteLiteral(aWriter, "=");
+
+  // The crash reason might be non-null-terminated in the case of a rust panic,
+  // it has also not being escaped so escape it one character at a time and
+  // write out the resulting string.
+  for (size_t i = 0; i < len; i++) {
+    if (reason[i] == '\\') {
+      WriteLiteral(aWriter, "\\\\");
+    } else if (reason[i] == '\n') {
+      WriteLiteral(aWriter, "\\n");
+    } else {
+      aWriter.WriteBuffer(reason + i, 1);
+    }
+  }
+
+  WriteLiteral(aWriter, "\n");
+}
+
 // Callback invoked from breakpad's exception handler, this writes out the
 // last annotations after a crash occurs and launches the crash reporter client.
 //
 // This function is not declared static even though it's not used outside of
 // this file because of an issue in Fennec which prevents breakpad's exception
 // handler from invoking it. See bug 1424304.
 bool
 MinidumpCallback(
@@ -1083,28 +1119,18 @@ MinidumpCallback(
     if (apiData.Valid()) {
       DllBlocklist_WriteNotes(apiData.Handle());
       DllBlocklist_WriteNotes(eventFile.Handle());
     }
 #endif
     WriteGlobalMemoryStatus(&apiData, &eventFile);
 #endif // XP_WIN
 
-    char* rust_panic_reason;
-    size_t rust_panic_len;
-    if (get_rust_panic_reason(&rust_panic_reason, &rust_panic_len)) {
-      // rust_panic_reason is not null-terminated.
-      WriteAnnotation(apiData, Annotation::MozCrashReason, rust_panic_reason,
-                      rust_panic_len);
-      WriteAnnotation(eventFile, Annotation::MozCrashReason, rust_panic_reason,
-                      rust_panic_len);
-    } else if (gMozCrashReason) {
-      WriteAnnotation(apiData, Annotation::MozCrashReason, gMozCrashReason);
-      WriteAnnotation(eventFile, Annotation::MozCrashReason, gMozCrashReason);
-    }
+    WriteEscapedMozCrashReason(apiData);
+    WriteEscapedMozCrashReason(eventFile);
 
     if (oomAllocationSizeBuffer[0]) {
       WriteAnnotation(apiData, Annotation::OOMAllocationSize,
                       oomAllocationSizeBuffer);
       WriteAnnotation(eventFile, Annotation::OOMAllocationSize,
                       oomAllocationSizeBuffer);
     }
 
@@ -1288,25 +1314,17 @@ PrepareChildExceptionTimeAnnotations(voi
     XP_STOA(gOOMAllocationSize, oomAllocationSizeBuffer, 10);
   }
 
   if (oomAllocationSizeBuffer[0]) {
     WriteAnnotation(apiData, Annotation::OOMAllocationSize,
                     oomAllocationSizeBuffer);
   }
 
-  char* rust_panic_reason;
-  size_t rust_panic_len;
-  if (get_rust_panic_reason(&rust_panic_reason, &rust_panic_len)) {
-    // rust_panic_reason is not null-terminated.
-    WriteAnnotation(apiData, Annotation::MozCrashReason, rust_panic_reason,
-                    rust_panic_len);
-  } else if (gMozCrashReason) {
-    WriteAnnotation(apiData, Annotation::MozCrashReason, gMozCrashReason);
-  }
+  WriteEscapedMozCrashReason(apiData);
 
   std::function<void(const char*)> getThreadAnnotationCB =
     [&] (const char * aValue) -> void {
     if (aValue) {
       WriteAnnotation(apiData, Annotation::ThreadIdNameMapping, aValue);
     }
   };
   GetFlatThreadAnnotation(getThreadAnnotationCB, true);
@@ -3051,17 +3069,17 @@ static bool
 IsDataEscaped(char* aData)
 {
   if (strchr(aData, '\n')) {
     // There should not be any newlines
     return false;
   }
   char* pos = aData;
   while ((pos = strchr(pos, '\\'))) {
-    if (*(pos + 1) != '\\') {
+    if (*(pos + 1) != '\\' && *(pos + 1) != 'n') {
       return false;
     }
     // Add 2 to account for the second pos
     pos += 2;
   }
   return true;
 }
 
copy from toolkit/crashreporter/test/unit/test_crash_rust_panic.js
copy to toolkit/crashreporter/test/unit/test_crash_rust_panic_multiline.js
--- a/toolkit/crashreporter/test/unit/test_crash_rust_panic.js
+++ b/toolkit/crashreporter/test/unit/test_crash_rust_panic_multiline.js
@@ -1,11 +1,11 @@
 function run_test() {
   // Try crashing with a Rust panic
   do_crash(function() {
-             Cc["@mozilla.org/xpcom/debug;1"].getService(Ci.nsIDebug2).rustPanic("OH NO");
+             Cc["@mozilla.org/xpcom/debug;1"].getService(Ci.nsIDebug2).rustPanic("OH NO\nOH NOES!");
            },
            function(mdump, extra) {
-             Assert.equal(extra.MozCrashReason, "OH NO");
+             Assert.equal(extra.MozCrashReason, "OH NO\nOH NOES!");
            },
           // process will exit with a zero exit status
           true);
 }
--- a/toolkit/crashreporter/test/unit/xpcshell.ini
+++ b/toolkit/crashreporter/test/unit/xpcshell.ini
@@ -3,16 +3,17 @@ head = head_crashreporter.js
 skip-if = toolkit == 'android'
 support-files =
   crasher_subprocess_head.js
   crasher_subprocess_tail.js
 
 [test_crash_moz_crash.js]
 [test_crash_purevirtual.js]
 [test_crash_rust_panic.js]
+[test_crash_rust_panic_multiline.js]
 [test_crash_after_js_oom_reported.js]
 [test_crash_after_js_oom_recovered.js]
 [test_crash_after_js_oom_reported_2.js]
 [test_crash_after_js_large_allocation_failure.js]
 [test_crash_after_js_large_allocation_failure_reporting.js]
 [test_crash_oom.js]
 [test_oom_annotation_windows.js]
 skip-if = os != 'win'
copy from toolkit/crashreporter/test/unit_ipc/test_content_rust_panic.js
copy to toolkit/crashreporter/test/unit_ipc/test_content_rust_panic_multiline.js
--- a/toolkit/crashreporter/test/unit_ipc/test_content_rust_panic.js
+++ b/toolkit/crashreporter/test/unit_ipc/test_content_rust_panic_multiline.js
@@ -7,15 +7,15 @@ function run_test() {
     return;
   }
 
   // Try crashing with a Rust panic
   do_triggered_content_crash(
     function() {
       Cc["@mozilla.org/xpcom/debug;1"]
         .getService(Ci.nsIDebug2)
-        .rustPanic("OH NO");
+        .rustPanic("OH NO\nOH NOES!");
     },
     function(mdump, extra) {
-      Assert.equal(extra.MozCrashReason, "OH NO");
+      Assert.equal(extra.MozCrashReason, "OH NO\nOH NOES!");
     }
   );
 }
--- a/toolkit/crashreporter/test/unit_ipc/xpcshell.ini
+++ b/toolkit/crashreporter/test/unit_ipc/xpcshell.ini
@@ -7,9 +7,10 @@ support-files =
   !/toolkit/crashreporter/test/unit/head_crashreporter.js
 
 [test_content_annotation.js]
 [test_content_exception_time_annotation.js]
 [test_content_oom_annotation_windows.js]
 skip-if = os != 'win'
 [test_content_memory_list.js]
 skip-if = os != 'win'
-[test_content_rust_panic.js]
\ No newline at end of file
+[test_content_rust_panic.js]
+[test_content_rust_panic_multiline.js]