Bug 1610629 - Use unix_path::Path/PathBuf in mozdevice for Android paths r=whimboo,webdriver-reviewers,jgraham

Differential Revision: https://phabricator.services.mozilla.com/D120377
This commit is contained in:
Barret Rennie 2021-07-28 00:02:40 +00:00
Родитель c9ac59dfa8
Коммит de403d5666
5 изменённых файлов: 158 добавлений и 113 удалений

16
Cargo.lock сгенерированный
Просмотреть файл

@ -3113,6 +3113,7 @@ dependencies = [
"once_cell",
"regex",
"tempfile",
"unix_path",
"uuid",
"walkdir",
]
@ -5321,6 +5322,21 @@ version = "0.2.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "826e7639553986605ec5979c7dd957c7895e93eabed50ab2ffa7f6128a75097c"
[[package]]
name = "unix_path"
version = "1.0.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "af8e291873ae77c4c8d9c9b34d0bee68a35b048fb39c263a5155e0e353783eaf"
dependencies = [
"unix_str",
]
[[package]]
name = "unix_str"
version = "1.0.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "2ace0b4755d0a2959962769239d56267f8a024fef2d9b32666b3dcd0946b0906"
[[package]]
name = "url"
version = "2.1.0"

Просмотреть файл

@ -1,11 +1,10 @@
use crate::capabilities::AndroidOptions;
use mozdevice::{AndroidStorage, Device, Host};
use mozdevice::{AndroidStorage, Device, Host, UnixPathBuf};
use mozprofile::profile::Profile;
use serde::Serialize;
use serde_yaml::{Mapping, Value};
use std::fmt;
use std::io;
use std::path::PathBuf;
use std::time;
use webdriver::error::{ErrorStatus, WebDriverError};
@ -97,11 +96,11 @@ impl AndroidProcess {
#[derive(Debug)]
pub struct AndroidHandler {
pub config: PathBuf,
pub config: UnixPathBuf,
pub options: AndroidOptions,
pub process: AndroidProcess,
pub profile: PathBuf,
pub test_root: PathBuf,
pub profile: UnixPathBuf,
pub test_root: UnixPathBuf,
// For port forwarding host => target
pub host_port: u16,
@ -170,19 +169,19 @@ impl AndroidHandler {
let test_root = match device.storage {
AndroidStorage::App => {
device.run_as_package = Some(options.package.to_owned());
let mut buf = PathBuf::from("/data/data");
let mut buf = UnixPathBuf::from("/data/data");
buf.push(&options.package);
buf.push("test_root");
buf
}
AndroidStorage::Internal => PathBuf::from("/data/local/tmp/test_root"),
AndroidStorage::Internal => UnixPathBuf::from("/data/local/tmp/test_root"),
AndroidStorage::Sdcard => {
// We need to push the profile to a location on the device that can also
// be read and write by the application, and works for unrooted devices.
// The only location that meets this criteria is under:
// $EXTERNAL_STORAGE/Android/data/%options.package%/files
let response = device.execute_host_shell_command("echo $EXTERNAL_STORAGE")?;
let mut buf = PathBuf::from(response.trim_end_matches('\n'));
let mut buf = UnixPathBuf::from(response.trim_end_matches('\n'));
buf.push("Android/data");
buf.push(&options.package);
buf.push("files/test_root");
@ -213,7 +212,7 @@ impl AndroidHandler {
return Err(AndroidError::PackageNotFound(options.package.clone()));
}
let config = PathBuf::from(format!(
let config = UnixPathBuf::from(format!(
"/data/local/tmp/{}-geckoview-config.yaml",
&options.package
));
@ -413,8 +412,7 @@ mod test {
use crate::android::AndroidHandler;
use crate::capabilities::AndroidOptions;
use mozdevice::{AndroidStorage, AndroidStorageInput};
use std::path::PathBuf;
use mozdevice::{AndroidStorage, AndroidStorageInput, UnixPathBuf};
fn run_handler_storage_test(package: &str, storage: AndroidStorageInput) {
let options = AndroidOptions::new(package.to_owned(), storage);
@ -423,7 +421,7 @@ mod test {
assert_eq!(handler.options, options);
assert_eq!(handler.process.package, package);
let expected_config_path = PathBuf::from(format!(
let expected_config_path = UnixPathBuf::from(format!(
"/data/local/tmp/{}-geckoview-config.yaml",
&package
));
@ -440,12 +438,12 @@ mod test {
let test_root = match handler.process.device.storage {
AndroidStorage::App => {
let mut buf = PathBuf::from("/data/data");
let mut buf = UnixPathBuf::from("/data/data");
buf.push(&package);
buf.push("test_root");
buf
}
AndroidStorage::Internal => PathBuf::from("/data/local/tmp/test_root"),
AndroidStorage::Internal => UnixPathBuf::from("/data/local/tmp/test_root"),
AndroidStorage::Sdcard => {
let response = handler
.process
@ -453,7 +451,7 @@ mod test {
.execute_host_shell_command("echo $EXTERNAL_STORAGE")
.unwrap();
let mut buf = PathBuf::from(response.trim_end_matches('\n'));
let mut buf = UnixPathBuf::from(response.trim_end_matches('\n'));
buf.push("Android/data/");
buf.push(&package);
buf.push("files/test_root");

Просмотреть файл

@ -14,5 +14,6 @@ once_cell = "1.4.0"
regex = { version = "1", default-features = false, features = ["perf", "std"] }
tempfile = "3"
walkdir = "2"
unix_path = "1.0"
uuid = { version = "0.8", features = ["serde", "v4"] }

Просмотреть файл

@ -25,9 +25,10 @@ use std::io::{self, Read, Write};
use std::iter::FromIterator;
use std::net::TcpStream;
use std::num::{ParseIntError, TryFromIntError};
use std::path::{Path, PathBuf};
use std::path::{Component, Path};
use std::str::{FromStr, Utf8Error};
use std::time::{Duration, SystemTime};
pub use unix_path::{Path as UnixPath, PathBuf as UnixPathBuf};
use uuid::Uuid;
use walkdir::WalkDir;
@ -410,7 +411,7 @@ pub struct Device {
pub storage: AndroidStorage,
/// Cache intermediate tempfile name used in pushing via run_as.
pub tempfile: PathBuf,
pub tempfile: UnixPathBuf,
}
impl Device {
@ -424,7 +425,7 @@ impl Device {
storage: AndroidStorage::App,
su_c_root: false,
su_0_root: false,
tempfile: PathBuf::from("/data/local/tmp"),
tempfile: UnixPathBuf::from("/data/local/tmp"),
};
device
.tempfile
@ -467,7 +468,7 @@ impl Device {
.map(|v| v.contains("Success"))
}
pub fn create_dir(&self, path: &Path) -> Result<()> {
pub fn create_dir(&self, path: &UnixPath) -> Result<()> {
debug!("Creating {}", path.display());
let enable_run_as = self.enable_run_as_for_path(&path);
@ -476,7 +477,7 @@ impl Device {
Ok(())
}
pub fn chmod(&self, path: &Path, mask: &str, recursive: bool) -> Result<()> {
pub fn chmod(&self, path: &UnixPath, mask: &str, recursive: bool) -> Result<()> {
let enable_run_as = self.enable_run_as_for_path(&path);
let recursive = match recursive {
@ -518,10 +519,10 @@ impl Device {
Ok(response.replace("\r\n", "\n"))
}
pub fn enable_run_as_for_path(&self, path: &Path) -> bool {
pub fn enable_run_as_for_path(&self, path: &UnixPath) -> bool {
match &self.run_as_package {
Some(package) => {
let mut p = PathBuf::from("/data/data/");
let mut p = UnixPathBuf::from("/data/data/");
p.push(package);
path.starts_with(p)
}
@ -691,12 +692,12 @@ impl Device {
.and(Ok(()))
}
pub fn path_exists(&self, path: &Path, enable_run_as: bool) -> Result<bool> {
pub fn path_exists(&self, path: &UnixPath, enable_run_as: bool) -> Result<bool> {
self.execute_host_shell_command_as(format!("ls {}", path.display()).as_str(), enable_run_as)
.map(|path| !path.contains("No such file or directory"))
}
pub fn push(&self, buffer: &mut dyn Read, dest: &Path, mode: u32) -> Result<()> {
pub fn push(&self, buffer: &mut dyn Read, dest: &UnixPath, mode: u32) -> Result<()> {
// Implement the ADB protocol to send a file to the device.
// The protocol consists of the following steps:
// * Send "host:transport" command with device serial
@ -708,7 +709,7 @@ impl Device {
let enable_run_as = self.enable_run_as_for_path(&dest.to_path_buf());
let dest1 = match enable_run_as {
true => self.tempfile.as_path(),
false => Path::new(dest),
false => UnixPath::new(dest),
};
// If the destination directory does not exist, adb will
@ -723,8 +724,8 @@ impl Device {
// exist so we can create them and adjust their permissions
// prior to performing the push.
let mut current = dest.parent();
let mut leaf: Option<&Path> = None;
let mut root: Option<&Path> = None;
let mut leaf: Option<&UnixPath> = None;
let mut root: Option<&UnixPath> = None;
while let Some(path) = current {
if self.path_exists(path, enable_run_as)? {
@ -828,7 +829,7 @@ impl Device {
}
}
pub fn push_dir(&self, source: &Path, dest_dir: &Path, mode: u32) -> Result<()> {
pub fn push_dir(&self, source: &Path, dest_dir: &UnixPath, mode: u32) -> Result<()> {
debug!("Pushing {} to {}", source.display(), dest_dir.display());
let walker = WalkDir::new(source).follow_links(false).into_iter();
@ -843,19 +844,18 @@ impl Device {
let mut file = File::open(path)?;
let mut dest = dest_dir.to_path_buf();
dest.push(
path.strip_prefix(source)
.map_err(|e| io::Error::new(io::ErrorKind::Other, e.to_string()))?,
);
let tail = path
.strip_prefix(source)
.map_err(|e| io::Error::new(io::ErrorKind::Other, e.to_string()))?;
let dest = append_components(dest_dir, tail)?;
self.push(&mut file, &dest, mode)?;
}
Ok(())
}
pub fn remove(&self, path: &Path) -> Result<()> {
pub fn remove(&self, path: &UnixPath) -> Result<()> {
debug!("Deleting {}", path.display());
self.execute_host_shell_command_as(
@ -866,3 +866,26 @@ impl Device {
Ok(())
}
}
pub(crate) fn append_components(base: &UnixPath, tail: &Path) -> std::result::Result<UnixPathBuf, io::Error> {
let mut buf = base.to_path_buf();
for component in tail.components() {
if let Component::Normal(ref segment) = component {
let utf8 = segment.to_str().ok_or_else(|| {
io::Error::new(
io::ErrorKind::Other,
"Could not represent path segment as UTF-8",
)
})?;
buf.push(utf8);
} else {
return Err(io::Error::new(
io::ErrorKind::Other,
"Unexpected path component".to_owned(),
));
}
}
Ok(buf)
}

Просмотреть файл

@ -13,9 +13,10 @@ use crate::*;
//
// $ cargo test -- --ignored --test-threads=1
use crate::{AndroidStorage, AndroidStorageInput};
use crate::{AndroidStorage, AndroidStorageInput, append_components};
use std::collections::BTreeSet;
use std::panic;
use std::path::PathBuf;
use tempfile::{tempdir, TempDir};
#[test]
@ -75,7 +76,7 @@ fn encode_message_with_invalid_string() {
fn run_device_test<F>(test: F)
where
F: FnOnce(&Device, &TempDir, &Path) + panic::UnwindSafe,
F: FnOnce(&Device, &TempDir, &UnixPath) + panic::UnwindSafe,
{
let host = Host {
..Default::default()
@ -85,7 +86,7 @@ where
.expect("device_or_default");
let tmp_dir = tempdir().expect("create temp dir");
let remote_path = Path::new("/data/local/tmp/mozdevice/");
let remote_path = UnixPath::new("/data/local/tmp/mozdevice/");
let _ = device.remove(remote_path);
@ -220,7 +221,7 @@ fn host_device_or_default_storage_as_sdcard() {
#[test]
#[ignore]
fn device_shell_command() {
run_device_test(|device: &Device, _: &TempDir, _: &Path| {
run_device_test(|device: &Device, _: &TempDir, _: &UnixPath| {
assert_eq!(
"Linux\n",
device
@ -233,7 +234,7 @@ fn device_shell_command() {
#[test]
#[ignore]
fn device_forward_port_hardcoded() {
run_device_test(|device: &Device, _: &TempDir, _: &Path| {
run_device_test(|device: &Device, _: &TempDir, _: &UnixPath| {
assert_eq!(
3035,
device
@ -248,7 +249,7 @@ fn device_forward_port_hardcoded() {
// #[ignore]
// TODO: "adb server response to `forward tcp:0 ...` was not a u16: \"000559464\"")
// fn device_forward_port_system_allocated() {
// run_device_test(|device: &Device, _: &TempDir, _: &Path| {
// run_device_test(|device: &Device, _: &TempDir, _: &UnixPath| {
// let local_port = device.forward_port(0, 3037).expect("local_port");
// assert_ne!(local_port, 0);
// // TODO: check with forward --list
@ -258,7 +259,7 @@ fn device_forward_port_hardcoded() {
#[test]
#[ignore]
fn device_kill_forward_port_no_forwarded_port() {
run_device_test(|device: &Device, _: &TempDir, _: &Path| {
run_device_test(|device: &Device, _: &TempDir, _: &UnixPath| {
device
.kill_forward_port(3038)
.expect_err("adb error: listener 'tcp:3038' ");
@ -268,7 +269,7 @@ fn device_kill_forward_port_no_forwarded_port() {
#[test]
#[ignore]
fn device_kill_forward_port_twice() {
run_device_test(|device: &Device, _: &TempDir, _: &Path| {
run_device_test(|device: &Device, _: &TempDir, _: &UnixPath| {
let local_port = device
.forward_port(3039, 3040)
.expect("forwarded local port");
@ -286,7 +287,7 @@ fn device_kill_forward_port_twice() {
#[test]
#[ignore]
fn device_kill_forward_all_ports_no_forwarded_port() {
run_device_test(|device: &Device, _: &TempDir, _: &Path| {
run_device_test(|device: &Device, _: &TempDir, _: &UnixPath| {
device
.kill_forward_all_ports()
.expect("to not fail for no forwarded ports");
@ -296,7 +297,7 @@ fn device_kill_forward_all_ports_no_forwarded_port() {
#[test]
#[ignore]
fn device_kill_forward_all_ports_twice() {
run_device_test(|device: &Device, _: &TempDir, _: &Path| {
run_device_test(|device: &Device, _: &TempDir, _: &UnixPath| {
let local_port1 = device
.forward_port(3039, 3040)
.expect("forwarded local port");
@ -318,7 +319,7 @@ fn device_kill_forward_all_ports_twice() {
#[test]
#[ignore]
fn device_reverse_port_hardcoded() {
run_device_test(|device: &Device, _: &TempDir, _: &Path| {
run_device_test(|device: &Device, _: &TempDir, _: &UnixPath| {
assert_eq!(4035, device.reverse_port(4035, 4036).expect("remote_port"));
// TODO: check with reverse --list
});
@ -328,7 +329,7 @@ fn device_reverse_port_hardcoded() {
// #[ignore]
// TODO: No adb response: ParseInt(ParseIntError { kind: Empty })
// fn device_reverse_port_system_allocated() {
// run_device_test(|device: &Device, _: &TempDir, _: &Path| {
// run_device_test(|device: &Device, _: &TempDir, _: &UnixPath| {
// let reverse_port = device.reverse_port(0, 4037).expect("remote port");
// assert_ne!(reverse_port, 0);
// // TODO: check with reverse --list
@ -338,7 +339,7 @@ fn device_reverse_port_hardcoded() {
#[test]
#[ignore]
fn device_kill_reverse_port_no_reverse_port() {
run_device_test(|device: &Device, _: &TempDir, _: &Path| {
run_device_test(|device: &Device, _: &TempDir, _: &UnixPath| {
device
.kill_reverse_port(4038)
.expect_err("listener 'tcp:4038' not found");
@ -349,7 +350,7 @@ fn device_kill_reverse_port_no_reverse_port() {
// #[ignore]
// TODO: "adb error: adb server response did not contain expected hexstring length: \"\""
// fn device_kill_reverse_port_twice() {
// run_device_test(|device: &Device, _: &TempDir, _: &Path| {
// run_device_test(|device: &Device, _: &TempDir, _: &UnixPath| {
// let remote_port = device
// .reverse_port(4039, 4040)
// .expect("reversed local port");
@ -367,7 +368,7 @@ fn device_kill_reverse_port_no_reverse_port() {
#[test]
#[ignore]
fn device_kill_reverse_all_ports_no_reversed_port() {
run_device_test(|device: &Device, _: &TempDir, _: &Path| {
run_device_test(|device: &Device, _: &TempDir, _: &UnixPath| {
device
.kill_reverse_all_ports()
.expect("to not fail for no reversed ports");
@ -377,7 +378,7 @@ fn device_kill_reverse_all_ports_no_reversed_port() {
#[test]
#[ignore]
fn device_kill_reverse_all_ports_twice() {
run_device_test(|device: &Device, _: &TempDir, _: &Path| {
run_device_test(|device: &Device, _: &TempDir, _: &UnixPath| {
let local_port1 = device
.forward_port(4039, 4040)
.expect("forwarded local port");
@ -399,87 +400,93 @@ fn device_kill_reverse_all_ports_twice() {
#[test]
#[ignore]
fn device_push() {
run_device_test(|device: &Device, _: &TempDir, remote_root_path: &Path| {
fn adjust_mode(mode: u32) -> u32 {
// Adjust the mode by copying the user permissions to
// group and other as indicated in
// [send_impl](https://android.googlesource.com/platform/system/core/+/master/adb/daemon/file_sync_service.cpp#516).
// This ensures that group and other can both access a
// file if the user can access it.
let mut m = mode & 0o777;
m |= (m >> 3) & 0o070;
m |= (m >> 3) & 0o007;
m
}
fn get_permissions(mode: u32) -> String {
// Convert the mode integer into the string representation
// of the mode returned by `ls`. This assumes the object is
// a file and not a directory.
let mut perms = vec!["-", "r", "w", "x", "r", "w", "x", "r", "w", "x"];
let mut bit_pos = 0;
while bit_pos < 9 {
if (1 << bit_pos) & mode == 0 {
perms[9 - bit_pos] = "-"
}
bit_pos += 1;
run_device_test(
|device: &Device, _: &TempDir, remote_root_path: &UnixPath| {
fn adjust_mode(mode: u32) -> u32 {
// Adjust the mode by copying the user permissions to
// group and other as indicated in
// [send_impl](https://android.googlesource.com/platform/system/core/+/master/adb/daemon/file_sync_service.cpp#516).
// This ensures that group and other can both access a
// file if the user can access it.
let mut m = mode & 0o777;
m |= (m >> 3) & 0o070;
m |= (m >> 3) & 0o007;
m
}
perms.concat()
}
let content = "test";
let remote_path = remote_root_path.join("foo.bar");
let modes = vec![0o421, 0o644, 0o666, 0o777];
for mode in modes {
let adjusted_mode = adjust_mode(mode);
let adjusted_perms = get_permissions(adjusted_mode);
device
.push(
&mut io::BufReader::new(content.as_bytes()),
&remote_path,
mode,
)
.expect("file has been pushed");
fn get_permissions(mode: u32) -> String {
// Convert the mode integer into the string representation
// of the mode returned by `ls`. This assumes the object is
// a file and not a directory.
let mut perms = vec!["-", "r", "w", "x", "r", "w", "x", "r", "w", "x"];
let mut bit_pos = 0;
while bit_pos < 9 {
if (1 << bit_pos) & mode == 0 {
perms[9 - bit_pos] = "-"
}
bit_pos += 1;
}
perms.concat()
}
let content = "test";
let remote_path = remote_root_path.join("foo.bar");
let modes = vec![0o421, 0o644, 0o666, 0o777];
for mode in modes {
let adjusted_mode = adjust_mode(mode);
let adjusted_perms = get_permissions(adjusted_mode);
device
.push(
&mut io::BufReader::new(content.as_bytes()),
&remote_path,
mode,
)
.expect("file has been pushed");
let output = device
.execute_host_shell_command(&format!("ls -l {}", remote_path.display()))
.expect("host shell command for 'ls' to succeed");
assert!(output.contains(remote_path.to_str().unwrap()));
assert!(output.starts_with(&adjusted_perms));
}
let output = device
.execute_host_shell_command(&format!("ls -l {}", remote_path.display()))
.expect("host shell command for 'ls' to succeed");
.execute_host_shell_command(&format!("ls -ld {}", remote_root_path.display()))
.expect("host shell command for 'ls parent' to succeed");
assert!(output.contains(remote_path.to_str().unwrap()));
assert!(output.starts_with(&adjusted_perms));
}
assert!(output.contains(remote_root_path.to_str().unwrap()));
assert!(output.starts_with("drwxrwxrwx"));
let output = device
.execute_host_shell_command(&format!("ls -ld {}", remote_root_path.display()))
.expect("host shell command for 'ls parent' to succeed");
let file_content = device
.execute_host_shell_command(&format!("cat {}", remote_path.display()))
.expect("host shell command for 'cat' to succeed");
assert!(output.contains(remote_root_path.to_str().unwrap()));
assert!(output.starts_with("drwxrwxrwx"));
let file_content = device
.execute_host_shell_command(&format!("cat {}", remote_path.display()))
.expect("host shell command for 'cat' to succeed");
assert_eq!(file_content, content);
});
assert_eq!(file_content, content);
},
);
}
#[test]
#[ignore]
fn device_push_dir() {
run_device_test(
|device: &Device, tmp_dir: &TempDir, remote_root_path: &Path| {
|device: &Device, tmp_dir: &TempDir, remote_root_path: &UnixPath| {
let content = "test";
let files = [
Path::new("foo1.bar"),
Path::new("foo2.bar"),
Path::new("bar/foo3.bar"),
Path::new("bar/more/baz/moar/foo3.bar"),
PathBuf::from("foo1.bar"),
PathBuf::from("foo2.bar"),
PathBuf::from("bar").join("foo3.bar"),
PathBuf::from("bar")
.join("more")
.join("baz")
.join("moar")
.join("foo3.bar"),
];
for file in files.iter() {
let path = tmp_dir.path().join(file);
let path = tmp_dir.path().join(&file);
let _ = std::fs::create_dir_all(path.parent().unwrap());
let f = File::create(path).expect("to create file");
@ -492,7 +499,7 @@ fn device_push_dir() {
.expect("to push_dir");
for file in files.iter() {
let path = remote_root_path.join(file);
let path = append_components(remote_root_path, file).unwrap();
let output = device
.execute_host_shell_command(&format!("ls {}", path.display()))
.expect("host shell command for 'ls' to succeed");