Skip to content

Commit fc0786c

Browse files
wip: erroring when size too big
WIP fix for #2313
1 parent c08be9a commit fc0786c

File tree

1 file changed

+38
-13
lines changed

1 file changed

+38
-13
lines changed

src/utils/dif_upload.rs

+38-13
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ use symbolic::debuginfo::pe::PeObject;
2727
use symbolic::debuginfo::sourcebundle::{SourceBundleWriter, SourceFileDescriptor};
2828
use symbolic::debuginfo::{Archive, FileEntry, FileFormat, Object};
2929
use symbolic::il2cpp::ObjectLineMapping;
30+
use thiserror::Error;
3031
use walkdir::WalkDir;
3132
use which::which;
3233
use zip::result::ZipError;
@@ -767,7 +768,8 @@ fn collect_auxdif<'a>(
767768
};
768769

769770
// Skip this file if we don't want to process it.
770-
if !options.validate_dif(&dif) {
771+
if let Err(e) = options.validate_dif(&dif) {
772+
debug!("Skipping dif {name}: {e}");
771773
return None;
772774
}
773775

@@ -836,7 +838,9 @@ fn collect_object_dif<'a>(
836838
// If this is a PE file with an embedded Portable PDB, we extract and process the PPDB separately.
837839
if let Object::Pe(pe) = &object {
838840
if let Ok(Some(ppdb_dif)) = extract_embedded_ppdb(pe, name.as_str()) {
839-
if options.validate_dif(&ppdb_dif) {
841+
if let Err(e) = options.validate_dif(&ppdb_dif) {
842+
debug!("Skipping ppdb dif: {e}");
843+
} else {
840844
collected.push(ppdb_dif);
841845
}
842846
}
@@ -878,7 +882,9 @@ fn collect_object_dif<'a>(
878882
};
879883

880884
// Skip this file if we don't want to process it.
881-
if !options.validate_dif(&dif) {
885+
if let Err(e) = options.validate_dif(&dif) {
886+
debug!("Skipping dif {name}: {e}");
887+
// TODO: Check if we have a size error, and error if so.
882888
continue;
883889
}
884890

@@ -1394,6 +1400,24 @@ pub enum DifFormat {
13941400
Il2Cpp,
13951401
}
13961402

1403+
#[derive(Debug, Error)]
1404+
enum DifValidationError<'s> {
1405+
#[error("{dif_name} has an invalid format")]
1406+
InvalidFormat { dif_name: &'s str },
1407+
#[error("{dif_name} has invalid features")]
1408+
InvalidFeatures { dif_name: &'s str },
1409+
#[error("{dif_name} has an invalid debug id")]
1410+
InvalidDebugId { dif_name: &'s str },
1411+
#[error(
1412+
"Size of {dif_name} is {size} bytes, which exceeds the maximum size of {max_size} bytes"
1413+
)]
1414+
SizeTooLarge {
1415+
dif_name: &'s str,
1416+
size: usize,
1417+
max_size: u64,
1418+
},
1419+
}
1420+
13971421
/// Searches, processes and uploads debug information files (DIFs).
13981422
///
13991423
/// This struct is created with the `DifUpload::new` function. Then, set
@@ -1745,33 +1769,34 @@ impl<'a> DifUpload<'a> {
17451769
/// This takes all the filters configured in the [`DifUpload`] into account and returns
17461770
/// whether a file should be skipped or not. It also takes care of logging such a skip
17471771
/// if required.
1748-
fn validate_dif(&self, dif: &DifMatch) -> bool {
1772+
fn validate_dif<'d>(&self, dif: &'d DifMatch) -> Result<(), DifValidationError<'d>> {
17491773
// Skip if we didn't want this kind of DIF.
1774+
let dif_name = &dif.name;
17501775
if !self.valid_format(dif.format()) {
1751-
debug!("skipping {} because of format", dif.name);
1752-
return false;
1776+
return Err(DifValidationError::InvalidFormat { dif_name });
17531777
}
17541778

17551779
// Skip if this DIF does not have features we want.
17561780
if !self.valid_features(dif) {
1757-
debug!("skipping {} because of features", dif.name);
1758-
return false;
1781+
return Err(DifValidationError::InvalidFeatures { dif_name });
17591782
}
17601783

17611784
// Skip if this DIF has no DebugId or we are only looking for certain IDs.
17621785
let id = dif.debug_id.unwrap_or_default();
17631786
if id.is_nil() || !self.valid_id(id) {
1764-
debug!("skipping {} because of debugid", dif.name);
1765-
return false;
1787+
return Err(DifValidationError::InvalidDebugId { dif_name });
17661788
}
17671789

17681790
// Skip if file exceeds the maximum allowed file size.
17691791
if !self.valid_size(&dif.name, dif.data().len()) {
1770-
debug!("skipping {} because of size", dif.name);
1771-
return false;
1792+
return Err(DifValidationError::SizeTooLarge {
1793+
dif_name,
1794+
size: dif.data().len(),
1795+
max_size: self.max_file_size,
1796+
});
17721797
}
17731798

1774-
true
1799+
Ok(())
17751800
}
17761801

17771802
fn into_chunk_options(self, server_options: ChunkServerOptions) -> ChunkOptions<'a> {

0 commit comments

Comments
 (0)