Skip to content

Commit

Permalink
Make create_apk_elf_path() infallible
Browse files Browse the repository at this point in the history
It doesn't really make much sense for the create_apk_elf_path() helper
to be fallible. Conceptually at least, the algorithm is super straight
forward and there is no reason to report an error just because no APK
path is present. Yes, it's a logical error of sorts, but it's a logical
error everywhere else as well, not just in this function.
Adjust the logic to be a bit more robust (or at least easier to grasp)
and to not emit any errors.

Signed-off-by: Daniel Müller <[email protected]>
  • Loading branch information
d-e-s-o committed Feb 27, 2025
1 parent 19f4dad commit eb98810
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 26 deletions.
61 changes: 36 additions & 25 deletions src/apk.rs
Original file line number Diff line number Diff line change
@@ -1,49 +1,60 @@
use std::ffi::OsStr;
use std::path::Component;
use std::path::Path;
use std::path::PathBuf;

use crate::Error;
use crate::Result;


pub(crate) fn create_apk_elf_path(apk: &Path, elf: &Path) -> Result<PathBuf> {
let mut extension = apk
.extension()
.unwrap_or_else(|| OsStr::new("apk"))
.to_os_string();
pub(crate) fn create_apk_elf_path(apk: &Path, elf: &Path) -> PathBuf {
let mut apk = apk.to_path_buf();
// Append '!' to indicate separation from archive internal contents
// that follow. This is an Android convention.
let () = extension.push("!");

let mut apk = apk.to_path_buf();
if !apk.set_extension(extension) {
return Err(Error::with_invalid_data(format!(
"path {} is not valid",
apk.display()
)))
}

let () = apk.as_mut_os_string().push("!");
let elf = {
let mut it = elf.components();
if let Some(first) = it.next() {
match first {
Component::Prefix(_) | Component::RootDir => {
// We removed the root directory/prefix.
it.as_path()
}
_ => elf,
}
} else {
elf
}
};
let path = apk.join(elf);
Ok(path)

path
}


#[cfg(test)]
mod tests {
use super::*;

use crate::ErrorKind;


/// Check that we can create a path to an ELF inside an APK as expected.
#[test]
fn elf_apk_path_creation() {
let apk = Path::new("/root/test.apk");
let elf = Path::new("subdir/libc.so");
let path = create_apk_elf_path(apk, elf).unwrap();
let path = create_apk_elf_path(apk, elf);
assert_eq!(path, Path::new("/root/test.apk!/subdir/libc.so"));

let err = create_apk_elf_path(Path::new(""), elf).unwrap_err();
assert_eq!(err.kind(), ErrorKind::InvalidData);
let apk = Path::new("/root/test");
let elf = Path::new("subdir/libc.so");
let path = create_apk_elf_path(apk, elf);
assert_eq!(path, Path::new("/root/test!/subdir/libc.so"));

let apk = Path::new("/root/test");
let elf = Path::new("/subdir/libc.so");
let path = create_apk_elf_path(apk, elf);
assert_eq!(path, Path::new("/root/test!/subdir/libc.so"));

let path = create_apk_elf_path(Path::new(""), elf);
assert_eq!(path, Path::new("!/subdir/libc.so"));

let path = create_apk_elf_path(apk, Path::new(""));
assert_eq!(path, Path::new("/root/test!/"));
}
}
2 changes: 1 addition & 1 deletion src/symbolize/symbolizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ fn default_apk_dispatcher(
) -> Result<Box<dyn Resolve>> {
// Create an Android-style binary-in-APK path for
// reporting purposes.
let apk_elf_path = create_apk_elf_path(info.apk_path, info.member_path)?;
let apk_elf_path = create_apk_elf_path(info.apk_path, info.member_path);
let parser = Rc::new(ElfParser::from_mmap(info.member_mmap, Some(apk_elf_path)));
let resolver = ElfResolver::from_parser(parser, debug_dirs)?;
let resolver = Box::new(resolver);
Expand Down

0 comments on commit eb98810

Please sign in to comment.