From 3270271e220c6c9bfad7678a920afc035b252aff Mon Sep 17 00:00:00 2001
From: Betty Da <bda@sentry.io>
Date: Wed, 16 Jun 2021 17:29:39 -0400
Subject: [PATCH 1/2] fix(symsorter): Don't emit files with empty debug
 identifiers

Files that symsorter was unable to generate a valid, nonempty debug
identifier (i.e. 000...000) for were being written to disk as if
they were successfully parsed and sorted. Chances are if the debug
identifier is empty, then the debug info is unusable.
---
 crates/symsorter/src/app.rs   |  7 +++----
 crates/symsorter/src/utils.rs | 31 +++++++++++++++----------------
 2 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/crates/symsorter/src/app.rs b/crates/symsorter/src/app.rs
index fb09827c3..4f5453066 100644
--- a/crates/symsorter/src/app.rs
+++ b/crates/symsorter/src/app.rs
@@ -123,9 +123,7 @@ fn process_file(
 
     for obj in archive.objects() {
         let obj = maybe_ignore_error!(obj.map_err(|e| anyhow!(e)));
-        let new_filename = root.join(maybe_ignore_error!(
-            get_target_filename(&obj).ok_or_else(|| anyhow!("unsupported file"))
-        ));
+        let new_filename = root.join(maybe_ignore_error!(get_target_filename(&obj)));
 
         fs::create_dir_all(new_filename.parent().unwrap())?;
 
@@ -160,7 +158,8 @@ fn process_file(
         } else {
             io::copy(&mut obj.data(), &mut out)?;
         }
-        rv.push((get_unified_id(&obj), obj.kind()));
+        let unified_id = maybe_ignore_error!(get_unified_id(&obj));
+        rv.push((unified_id, obj.kind()));
     }
 
     Ok(rv)
diff --git a/crates/symsorter/src/utils.rs b/crates/symsorter/src/utils.rs
index c9bacac9b..264fa0974 100644
--- a/crates/symsorter/src/utils.rs
+++ b/crates/symsorter/src/utils.rs
@@ -1,7 +1,7 @@
 use std::io::Cursor;
 use std::path::{Path, PathBuf};
 
-use anyhow::{anyhow, Result};
+use anyhow::{anyhow, bail, Result};
 use lazy_static::lazy_static;
 use regex::Regex;
 use symbolic::common::ByteView;
@@ -26,31 +26,30 @@ pub fn is_bundle_id(input: &str) -> bool {
 }
 
 /// Gets the unified ID from an object.
-pub fn get_unified_id(obj: &Object) -> String {
+pub fn get_unified_id(obj: &Object) -> Result<String> {
     if obj.file_format() == FileFormat::Pe || obj.code_id().is_none() {
-        obj.debug_id().breakpad().to_string().to_lowercase()
+        let debug_id = obj.debug_id();
+        if debug_id.is_nil() {
+            Err(anyhow!("failed to generate debug identifier"))
+        } else {
+            Ok(debug_id.breakpad().to_string().to_lowercase())
+        }
     } else {
-        obj.code_id().as_ref().unwrap().as_str().to_string()
+        Ok(obj.code_id().as_ref().unwrap().as_str().to_string())
     }
 }
 
 /// Returns the intended target filename for an object.
-pub fn get_target_filename(obj: &Object) -> Option<PathBuf> {
-    let id = get_unified_id(obj);
+pub fn get_target_filename(obj: &Object) -> Result<PathBuf> {
+    let id = get_unified_id(obj)?;
     // match the unified format here.
     let suffix = match obj.kind() {
         ObjectKind::Debug => "debuginfo",
-        ObjectKind::Sources => {
-            if obj.file_format() == FileFormat::SourceBundle {
-                "sourcebundle"
-            } else {
-                return None;
-            }
-        }
+        ObjectKind::Sources if obj.file_format() == FileFormat::SourceBundle => "sourcebundle",
         ObjectKind::Relocatable | ObjectKind::Library | ObjectKind::Executable => "executable",
-        _ => return None,
+        _ => bail!("unsupported file"),
     };
-    Some(format!("{}/{}/{}", &id[..2], &id[2..], suffix).into())
+    Ok(format!("{}/{}/{}", &id[..2], &id[2..], suffix).into())
 }
 
 /// Creates a source bundle from a path.
@@ -59,7 +58,7 @@ pub fn create_source_bundle(path: &Path, unified_id: &str) -> Result<Option<Byte
     let archive = Archive::parse(&bv).map_err(|e| anyhow!(e))?;
     for obj in archive.objects() {
         let obj = obj.map_err(|e| anyhow!(e))?;
-        if get_unified_id(&obj) == unified_id {
+        if matches!(get_unified_id(&obj), Ok(generated_id) if unified_id == generated_id) {
             let mut out = Vec::<u8>::new();
             let writer =
                 SourceBundleWriter::start(Cursor::new(&mut out)).map_err(|e| anyhow!(e))?;

From 535da7c8793efb193ffc54c1e627f86f9cb34146 Mon Sep 17 00:00:00 2001
From: Jan Michael Auer <mail@jauer.org>
Date: Thu, 17 Jun 2021 08:33:21 +0200
Subject: [PATCH 2/2] meta: Changelog

---
 CHANGELOG.md | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 44b8e48a9..37fc7e0d6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,10 @@
 
 - Bump symbolic to support versioning of CFI Caches. ([#467](https://github.com/getsentry/symbolicator/pull/467))
 
+### Tools
+
+- `symsorter` no longer emits files with empty debug identifiers. ([#469](https://github.com/getsentry/symbolicator/pull/469))
+
 ## 0.3.4
 
 ### Features