feat: change DebugSession::source_by_path() to return SourceCode enum (#758)

Co-authored-by: Sebastian Zivota <loewenheim@users.noreply.github.com>

Author: vaind

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+**Breaking changes**:
+
+- Change `DebugSession::source_by_path()` to return `SourceCode` enum with either file content or a URL to fetch it from. ([#758](https://github.com/getsentry/symbolic/pull/758))
+
 **Fixes**:
 
 - Make sure to parse `PortablePdb` streams in the correct order. ([#760](https://github.com/getsentry/symbolic/pull/760))

symbolic-debuginfo/src/base.rs

@@ -670,6 +670,17 @@ impl fmt::Debug for Function<'_> {
 /// A dynamically dispatched iterator over items with the given lifetime.
 pub type DynIterator<'a, T> = Box<dyn Iterator<Item = T> + 'a>;
 
+/// Represents a source file referenced by a debug information object file.
+#[non_exhaustive]
+#[derive(Debug, Clone)]
+pub enum SourceCode<'a> {
+    /// Verbatim source code/file contents.
+    Content(Cow<'a, str>),
+
+    /// Url (usually https) where the content can be fetched from.
+    Url(Cow<'a, str>),
+}
+
 /// A stateful session for interfacing with debug information.
 ///
 /// Debug sessions can be obtained via [`ObjectLike::debug_session`]. Since computing a session may
@@ -716,10 +727,9 @@ pub trait DebugSession<'session> {
     /// Returns an iterator over all source files referenced by this debug file.
     fn files(&'session self) -> Self::FileIterator;
 
-    /// Looks up a file's source contents by its full canonicalized path.
-    ///
-    /// The given path must be canonicalized.
-    fn source_by_path(&self, path: &str) -> Result<Option<Cow<'_, str>>, Self::Error>;
+    /// Looks up a file's source by its full canonicalized path.
+    /// Returns either source contents, if it was embedded, or a source link.
+    fn source_by_path(&self, path: &str) -> Result<Option<SourceCode<'_>>, Self::Error>;
 }
 
 /// An object containing debug information.

symbolic-debuginfo/src/breakpad.rs

@@ -1276,10 +1276,8 @@ impl<'data> BreakpadDebugSession<'data> {
         }
     }
 
-    /// Looks up a file's source contents by its full canonicalized path.
-    ///
-    /// The given path must be canonicalized.
-    pub fn source_by_path(&self, _path: &str) -> Result<Option<Cow<'_, str>>, BreakpadError> {
+    /// See [DebugSession::source_by_path] for more information.
+    pub fn source_by_path(&self, _path: &str) -> Result<Option<SourceCode<'_>>, BreakpadError> {
         Ok(None)
     }
 }
@@ -1297,7 +1295,7 @@ impl<'data, 'session> DebugSession<'session> for BreakpadDebugSession<'data> {
         self.files()
     }
 
-    fn source_by_path(&self, path: &str) -> Result<Option<Cow<'_, str>>, Self::Error> {
+    fn source_by_path(&self, path: &str) -> Result<Option<SourceCode<'_>>, Self::Error> {
         self.source_by_path(path)
     }
 }

symbolic-debuginfo/src/dwarf.rs

@@ -1331,10 +1331,8 @@ impl<'data> DwarfDebugSession<'data> {
         }
     }
 
-    /// Looks up a file's source contents by its full canonicalized path.
-    ///
-    /// The given path must be canonicalized.
-    pub fn source_by_path(&self, _path: &str) -> Result<Option<Cow<'_, str>>, DwarfError> {
+    /// See [DebugSession::source_by_path] for more information.
+    pub fn source_by_path(&self, _path: &str) -> Result<Option<SourceCode<'_>>, DwarfError> {
         Ok(None)
     }
 }
@@ -1352,7 +1350,7 @@ impl<'data, 'session> DebugSession<'session> for DwarfDebugSession<'data> {
         self.files()
     }
 
-    fn source_by_path(&self, path: &str) -> Result<Option<Cow<'_, str>>, Self::Error> {
+    fn source_by_path(&self, path: &str) -> Result<Option<SourceCode<'_>>, Self::Error> {
         self.source_by_path(path)
     }
 }

symbolic-debuginfo/src/object.rs

@@ -1,6 +1,5 @@
 //! Generic wrappers over various object file formats.
 
-use std::borrow::Cow;
 use std::error::Error;
 use std::fmt;
 
@@ -483,10 +482,9 @@ impl<'d> ObjectDebugSession<'d> {
         }
     }
 
-    /// Looks up a file's source contents by its full canonicalized path.
-    ///
-    /// The given path must be canonicalized.
-    pub fn source_by_path(&self, path: &str) -> Result<Option<Cow<'_, str>>, ObjectError> {
+    /// Looks up a file's source by its full canonicalized path.
+    /// Returns either source contents, if it was embedded, or a source link.
+    pub fn source_by_path(&self, path: &str) -> Result<Option<SourceCode<'_>>, ObjectError> {
         match *self {
             ObjectDebugSession::Breakpad(ref s) => {
                 s.source_by_path(path).map_err(ObjectError::transparent)
@@ -520,7 +518,7 @@ impl<'session> DebugSession<'session> for ObjectDebugSession<'_> {
         self.files()
     }
 
-    fn source_by_path(&self, path: &str) -> Result<Option<Cow<'_, str>>, Self::Error> {
+    fn source_by_path(&self, path: &str) -> Result<Option<SourceCode<'_>>, Self::Error> {
         self.source_by_path(path)
     }
 }

symbolic-debuginfo/src/pdb.rs

@@ -637,10 +637,8 @@ impl<'d> PdbDebugSession<'d> {
         }
     }
 
-    /// Looks up a file's source contents by its full canonicalized path.
-    ///
-    /// The given path must be canonicalized.
-    pub fn source_by_path(&self, _path: &str) -> Result<Option<Cow<'_, str>>, PdbError> {
+    /// See [DebugSession::source_by_path] for more information.
+    pub fn source_by_path(&self, _path: &str) -> Result<Option<SourceCode<'_>>, PdbError> {
         Ok(None)
     }
 }
@@ -658,7 +656,7 @@ impl<'session> DebugSession<'session> for PdbDebugSession<'_> {
         self.files()
     }
 
-    fn source_by_path(&self, path: &str) -> Result<Option<Cow<'_, str>>, Self::Error> {
+    fn source_by_path(&self, path: &str) -> Result<Option<SourceCode<'_>>, Self::Error> {
         self.source_by_path(path)
     }
 }

symbolic-debuginfo/src/ppdb.rs

@@ -4,9 +4,10 @@ use std::collections::HashMap;
 use std::fmt;
 use std::iter;
 
+use lazycell::LazyCell;
 use symbolic_common::{Arch, CodeId, DebugId};
 use symbolic_ppdb::EmbeddedSource;
-use symbolic_ppdb::{FormatError, PortablePdb};
+use symbolic_ppdb::{Document, FormatError, PortablePdb};
 
 use crate::base::*;
 
@@ -141,24 +142,44 @@ impl fmt::Debug for PortablePdbObject<'_> {
 /// A debug session for a Portable PDB object.
 pub struct PortablePdbDebugSession<'data> {
     ppdb: PortablePdb<'data>,
-    sources: HashMap<String, EmbeddedSource<'data>>,
+    sources: LazyCell<HashMap<String, PPDBSource<'data>>>,
+}
+
+#[derive(Debug, Clone)]
+enum PPDBSource<'data> {
+    Embedded(EmbeddedSource<'data>),
+    Link(Document),
 }
 
 impl<'data> PortablePdbDebugSession<'data> {
     fn new(ppdb: &'_ PortablePdb<'data>) -> Result<Self, FormatError> {
-        let mut sources: HashMap<String, EmbeddedSource<'data>> = HashMap::new();
-        for source in ppdb.get_embedded_sources()? {
-            match source {
-                Ok(source) => sources.insert(source.get_path().into(), source),
-                Err(e) => return Err(e),
-            };
-        }
         Ok(PortablePdbDebugSession {
             ppdb: ppdb.clone(),
-            sources,
+            sources: LazyCell::new(),
         })
     }
 
+    fn init_sources(&self) -> HashMap<String, PPDBSource<'data>> {
+        let count = self.ppdb.get_documents_count().unwrap_or(0);
+        let mut result = HashMap::with_capacity(count);
+
+        if let Ok(iter) = self.ppdb.get_embedded_sources() {
+            for source in iter.flatten() {
+                result.insert(source.get_path().to_string(), PPDBSource::Embedded(source));
+            }
+        };
+
+        for i in 1..count + 1 {
+            if let Ok(doc) = self.ppdb.get_document(i) {
+                if !result.contains_key(&doc.name) {
+                    result.insert(doc.name.clone(), PPDBSource::Link(doc));
+                }
+            }
+        }
+
+        result
+    }
+
     /// Returns an iterator over all functions in this debug file.
     pub fn functions(&self) -> PortablePdbFunctionIterator<'_> {
         iter::empty()
@@ -169,15 +190,17 @@ impl<'data> PortablePdbDebugSession<'data> {
         PortablePdbFileIterator::new(&self.ppdb)
     }
 
-    /// Looks up a file's source contents by its full canonicalized path.
-    ///
-    /// The given path must be canonicalized.
-    pub fn source_by_path(&self, path: &str) -> Result<Option<Cow<'_, str>>, FormatError> {
-        match self.sources.get(path) {
+    /// See [DebugSession::source_by_path] for more information.
+    pub fn source_by_path(&self, path: &str) -> Result<Option<SourceCode<'_>>, FormatError> {
+        let sources = self.sources.borrow_with(|| self.init_sources());
+        match sources.get(path) {
             None => Ok(None),
-            Some(source) => source
+            Some(PPDBSource::Embedded(source)) => source
                 .get_contents()
-                .map(|bytes| Some(from_utf8_cow_lossy(&bytes))),
+                .map(|bytes| Some(SourceCode::Content(from_utf8_cow_lossy(&bytes)))),
+            Some(PPDBSource::Link(document)) => {
+                Ok(self.ppdb.get_source_link(document).map(SourceCode::Url))
+            }
         }
     }
 }
@@ -195,7 +218,7 @@ impl<'data, 'session> DebugSession<'session> for PortablePdbDebugSession<'data>
         self.files()
     }
 
-    fn source_by_path(&self, path: &str) -> Result<Option<Cow<'_, str>>, Self::Error> {
+    fn source_by_path(&self, path: &str) -> Result<Option<SourceCode<'_>>, Self::Error> {
         self.source_by_path(path)
     }
 }

symbolic-debuginfo/src/sourcebundle.rs

@@ -650,17 +650,15 @@ impl<'data> SourceBundleDebugSession<'data> {
         Ok(Some(source_content))
     }
 
-    /// Looks up a file's source contents by its full canonicalized path.
-    ///
-    /// The given path must be canonicalized.
-    pub fn source_by_path(&self, path: &str) -> Result<Option<Cow<'_, str>>, SourceBundleError> {
+    /// See [DebugSession::source_by_path] for more information.
+    pub fn source_by_path(&self, path: &str) -> Result<Option<SourceCode<'_>>, SourceBundleError> {
         let zip_path = match self.zip_path_by_source_path(path) {
             Some(zip_path) => zip_path,
             None => return Ok(None),
         };
 
-        self.source_by_zip_path(zip_path)
-            .map(|opt| opt.map(Cow::Owned))
+        let content = self.source_by_zip_path(zip_path)?;
+        Ok(content.map(|opt| SourceCode::Content(Cow::Owned(opt))))
     }
 }
 
@@ -677,7 +675,7 @@ impl<'data, 'session> DebugSession<'session> for SourceBundleDebugSession<'data>
         self.files()
     }
 
-    fn source_by_path(&self, path: &str) -> Result<Option<Cow<'_, str>>, Self::Error> {
+    fn source_by_path(&self, path: &str) -> Result<Option<SourceCode<'_>>, Self::Error> {
         self.source_by_path(path)
     }
 }
@@ -1152,11 +1150,12 @@ mod tests {
             .flatten()
             .flat_map(|f| {
                 let path = f.abs_path_str();
-                session
-                    .source_by_path(&path)
-                    .ok()
-                    .flatten()
-                    .map(|c| (path, c.into_owned()))
+                session.source_by_path(&path).ok().flatten().map(|source| {
+                    let SourceCode::Content(text) = source else {
+                         unreachable!();
+                     };
+                    (path, text.into_owned())
+                })
             })
             .collect();
 

symbolic-debuginfo/tests/test_objects.rs

@@ -2,7 +2,7 @@ use std::{env, ffi::CString, fmt, io::BufWriter};
 
 use symbolic_common::ByteView;
 use symbolic_debuginfo::{
-    elf::ElfObject, pe::PeObject, FileEntry, Function, LineInfo, Object, SymbolMap,
+    elf::ElfObject, pe::PeObject, FileEntry, Function, LineInfo, Object, SourceCode, SymbolMap,
 };
 use symbolic_testutils::fixture;
 
@@ -776,10 +776,51 @@ fn test_ppdb_source_by_path() -> Result<(), Error> {
                 "C:\\dev\\sentry-dotnet\\samples\\Sentry.Samples.Console.Basic\\Program.cs",
             )
             .unwrap();
-        let source_text = source.unwrap();
-        assert_eq!(source_text.len(), 204);
+        match source.unwrap() {
+            SourceCode::Content(text) => assert_eq!(text.len(), 204),
+            _ => panic!(),
+        }
+    }
+
+    Ok(())
+}
+
+#[test]
+fn test_ppdb_source_links() -> Result<(), Error> {
+    let view = ByteView::open(fixture("ppdb-sourcelink-sample/ppdb-sourcelink-sample.pdb"))?;
+    let object = Object::parse(&view)?;
+    let session = object.debug_session()?;
+
+    let known_embedded_sources = vec![
+        ".NETStandard,Version=v2.0.AssemblyAttributes.cs",
+        "ppdb-sourcelink-sample.AssemblyInfo.cs",
+    ];
+
+    // Testing this is simple because there's just one prefix rule in this PPDB.
+    let src_prefix = "C:\\dev\\symbolic\\";
+    let url_prefix = "https://raw.githubusercontent.com/getsentry/symbolic/9f7ceefc29da4c45bc802751916dbb3ea72bf08f/";
+
+    for file in session.files() {
+        let file = file.unwrap();
+
+        match session.source_by_path(&file.path_str()).unwrap().unwrap() {
+            SourceCode::Content(text) => {
+                assert!(known_embedded_sources.contains(&file.name_str().as_ref()));
+                assert!(!text.is_empty());
+            }
+            SourceCode::Url(url) => {
+                // testing this is simple because there's just one prefix rule in this PPDB.
+                let expected = file
+                    .path_str()
+                    .replace(src_prefix, url_prefix)
+                    .replace('\\', "/");
+                assert_eq!(url, expected);
+            }
+            _ => panic!(),
+        }
     }
 
+    assert!(session.source_by_path("c:/non/existent/path.cs")?.is_none());
     Ok(())
 }
 

symbolic-ppdb/Cargo.toml

@@ -26,6 +26,7 @@ watto = { version = "0.1.0", features = ["writer", "strings"] }
 thiserror = "1.0.31"
 uuid = "1.0.0"
 flate2 = { version ="1.0.13", default-features = false, features = [ "rust_backend" ] }
+serde_json = { version = "1.0.40" }
 
 [dev-dependencies]
 symbolic-debuginfo = { path = "../symbolic-debuginfo" }