Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(sourcebundles): Only accept UTF-8 files #816

Merged
merged 5 commits into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

**Fixes**

- sourcebundles: Only valid UTF-8 files can be written into sourcebundles ([#816](https://github.com/getsentry/symbolic/pull/816))
- Fix an issue when extracting the name of the debug file from a PE object ([#825](https://github.com/getsentry/symbolic/pull/825))

**Features**
Expand Down
31 changes: 30 additions & 1 deletion symbolic-debuginfo/src/sourcebundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ pub enum SourceBundleErrorKind {

/// Generic error when writing a source bundle, most likely IO.
WriteFailed,

/// The file is not valid UTF-8 or could not be read for another reason.
ReadFailed,
}

impl fmt::Display for SourceBundleErrorKind {
Expand All @@ -105,6 +108,7 @@ impl fmt::Display for SourceBundleErrorKind {
Self::BadManifest => write!(f, "failed to read/write source bundle manifest"),
Self::BadDebugFile => write!(f, "malformed debug info file"),
Self::WriteFailed => write!(f, "failed to write source bundle"),
Self::ReadFailed => write!(f, "file could not be read as UTF-8"),
}
}
}
Expand Down Expand Up @@ -1154,6 +1158,8 @@ where

/// Adds a file and its info to the bundle.
///
/// Only files containing valid UTF-8 are accepted.
///
/// Multiple files can be added at the same path. For the first duplicate, a counter will be
/// appended to the file name. Any subsequent duplicate increases that counter. For example:
///
Expand Down Expand Up @@ -1185,13 +1191,20 @@ where
S: AsRef<str>,
R: Read,
{
let mut buf = String::new();

if let Err(e) = file.read_to_string(&mut buf) {
Comment on lines +1194 to +1196
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not too happy about allocating/copying another string here, but meh I guess its not too bad

return Err(SourceBundleError::new(SourceBundleErrorKind::ReadFailed, e));
}

let full_path = self.file_path(path.as_ref());
let unique_path = self.unique_path(full_path);

self.writer
.start_file(unique_path.clone(), default_file_options())
.map_err(|e| SourceBundleError::new(SourceBundleErrorKind::WriteFailed, e))?;
std::io::copy(&mut file, &mut self.writer)
self.writer
.write_all(buf.as_bytes())
.map_err(|e| SourceBundleError::new(SourceBundleErrorKind::WriteFailed, e))?;

self.manifest.files.insert(unique_path, info);
Expand Down Expand Up @@ -1420,6 +1433,22 @@ mod tests {
Ok(())
}

#[test]
fn test_non_utf8() -> Result<(), SourceBundleError> {
let writer = Cursor::new(Vec::new());
let mut bundle = SourceBundleWriter::start(writer)?;

assert!(bundle
.add_file(
"bar.txt",
&[0, 159, 146, 150][..],
SourceFileInfo::default()
)
.is_err());

Ok(())
}

#[test]
fn test_duplicate_files() -> Result<(), SourceBundleError> {
let writer = Cursor::new(Vec::new());
Expand Down
Loading