diff options
28 files changed, 179 insertions, 114 deletions
diff --git a/crates/shirabe-class-map-generator/src/php_file_parser.rs b/crates/shirabe-class-map-generator/src/php_file_parser.rs index 0cb9180f..450bea90 100644 --- a/crates/shirabe-class-map-generator/src/php_file_parser.rs +++ b/crates/shirabe-class-map-generator/src/php_file_parser.rs @@ -28,8 +28,12 @@ impl PhpFileParser { "File at \"{}\" is not readable, check its permissions", path ); - } else if trim(file_get_contents(path).unwrap_or_default().as_str(), None) - .is_empty() + // TODO(bytes) + } else if trim( + &String::from_utf8_lossy(&file_get_contents(path).unwrap_or_default()), + None, + ) + .is_empty() { // The input file was really empty and thus contains no classes return Ok(vec![]); @@ -178,7 +182,7 @@ impl PhpFileParser { } if is_file(path) { - return file_get_contents(path).is_some(); + return file_get_contents(path).is_ok(); } // assume false otherwise diff --git a/crates/shirabe-php-shim/src/fs.rs b/crates/shirabe-php-shim/src/fs.rs index 26ea1fdf..41383490 100644 --- a/crates/shirabe-php-shim/src/fs.rs +++ b/crates/shirabe-php-shim/src/fs.rs @@ -1,4 +1,3 @@ -use crate::PhpMixed; use crate::PhpResource; use crate::StreamBacking; use crate::StreamState; @@ -871,7 +870,7 @@ pub fn file_put_contents3(filename: &str, data: &str, flags: i64) -> Option<i64> Some(data.len() as i64) } -pub fn file_get_contents(path: impl AsRef<std::path::Path>) -> Option<String> { +pub fn file_get_contents(path: impl AsRef<std::path::Path>) -> std::io::Result<Vec<u8>> { let path = path.as_ref(); // PHP supports the file:// stream wrapper; strip it to read the local file. let path = path @@ -879,34 +878,23 @@ pub fn file_get_contents(path: impl AsRef<std::path::Path>) -> Option<String> { .and_then(|s| s.strip_prefix("file://")) .map_or(path, std::path::Path::new); std::fs::read(path) - .ok() - .map(|bytes| String::from_utf8_lossy(&bytes).into_owned()) } -/// `$use_include_path` and the stream `$context` have no effect; the read always goes to the -/// local filesystem. -pub fn file_get_contents5( - path: &str, - _use_include_path: bool, - _context: PhpMixed, - offset: i64, - length: Option<i64>, -) -> Option<String> { +pub fn file_get_contents_with_max_length( + path: impl AsRef<std::path::Path>, + length: usize, +) -> std::io::Result<Vec<u8>> { + let path = path.as_ref(); // PHP supports the file:// stream wrapper; strip it to read the local file. - let path = path.strip_prefix("file://").unwrap_or(path); - let bytes = std::fs::read(path).ok()?; - let len = bytes.len() as i64; - let start = if offset < 0 { - (len + offset).max(0) - } else { - offset.min(len) - } as usize; - let slice = &bytes[start..]; - let slice = match length { - Some(l) if l >= 0 => &slice[..(l as usize).min(slice.len())], - _ => slice, - }; - Some(String::from_utf8_lossy(slice).into_owned()) + let path = path + .to_str() + .and_then(|s| s.strip_prefix("file://")) + .map_or(path, std::path::Path::new); + let mut buf = Vec::new(); + std::fs::File::open(path)? + .take(length as u64) + .read_to_end(&mut buf)?; + Ok(buf) } pub fn getcwd() -> Option<String> { diff --git a/crates/shirabe-php-shim/src/string.rs b/crates/shirabe-php-shim/src/string.rs index 024a3943..3a61a59d 100644 --- a/crates/shirabe-php-shim/src/string.rs +++ b/crates/shirabe-php-shim/src/string.rs @@ -299,11 +299,8 @@ pub fn trim(s: &str, chars: Option<&str>) -> String { } // Byte-based, matching PHP's substr. A negative start/length counts from the end. -// The result is reinterpreted as UTF-8 (lossily), which only matters when a slice -// boundary falls inside a multibyte sequence. -pub fn substr(s: &str, start: i64, length: Option<i64>) -> String { - let bytes = s.as_bytes(); - let len = bytes.len() as i64; +pub fn substr_bytes(s: &[u8], start: i64, length: Option<i64>) -> Vec<u8> { + let len = s.len() as i64; let start = if start < 0 { (len + start).max(0) } else { @@ -314,7 +311,13 @@ pub fn substr(s: &str, start: i64, length: Option<i64>) -> String { Some(l) if l < 0 => (len + l).max(start), Some(l) => (start + l).min(len), }; - String::from_utf8_lossy(&bytes[start as usize..end as usize]).into_owned() + s[start as usize..end as usize].to_vec() +} + +// The result is reinterpreted as UTF-8 (lossily), which only matters when a slice +// boundary falls inside a multibyte sequence. +pub fn substr(s: &str, start: i64, length: Option<i64>) -> String { + String::from_utf8_lossy(&substr_bytes(s.as_bytes(), start, length)).into_owned() } pub fn implode(glue: &str, pieces: &[String]) -> String { diff --git a/crates/shirabe/src/autoload/autoload_generator.rs b/crates/shirabe/src/autoload/autoload_generator.rs index 55124467..3abf3453 100644 --- a/crates/shirabe/src/autoload/autoload_generator.rs +++ b/crates/shirabe/src/autoload/autoload_generator.rs @@ -556,8 +556,11 @@ return array( // carry over existing autoload.php's suffix if possible and none is configured if suffix.is_none() && Filesystem::is_readable(&format!("{}/autoload.php", vendor_path)) { - let content = - file_get_contents(format!("{}/autoload.php", vendor_path)).unwrap_or_default(); + // TODO(bytes): preg_match matches over a &str. + let content = String::from_utf8_lossy( + &file_get_contents(format!("{}/autoload.php", vendor_path)).unwrap_or_default(), + ) + .into_owned(); if let Some(matches) = preg_match(php_regex!("{ComposerAutoloaderInit([^:\\s]+)::}"), &content) { diff --git a/crates/shirabe/src/cache.rs b/crates/shirabe/src/cache.rs index a8e4fb26..269a31ef 100644 --- a/crates/shirabe/src/cache.rs +++ b/crates/shirabe/src/cache.rs @@ -136,7 +136,11 @@ impl Cache { crate::io::DEBUG, ); - return file_get_contents(&full_path); + // TODO(bytes): the payload is handed back to callers as a String, so a cache + // entry that is not valid UTF-8 is corrupted by from_utf8_lossy. + return file_get_contents(&full_path) + .map(|c| String::from_utf8_lossy(&c).into_owned()) + .ok(); } } diff --git a/crates/shirabe/src/command/bump_command.rs b/crates/shirabe/src/command/bump_command.rs index e10f7b15..9639cbe9 100644 --- a/crates/shirabe/src/command/bump_command.rs +++ b/crates/shirabe/src/command/bump_command.rs @@ -73,8 +73,8 @@ impl BumpCommand { let composer_json = JsonFile::new(composer_json_path.clone(), None, None)?; let contents = match file_get_contents(composer_json.get_path()) { - Some(c) => c, - None => { + Ok(c) => c, + Err(_) => { io.write_error3( &format!("<error>{} is not readable.</error>", composer_json_path), true, @@ -86,7 +86,7 @@ impl BumpCommand { if !is_writable(&composer_json_path) && Silencer::call(|| { - file_put_contents(&composer_json_path, contents.as_bytes()) + file_put_contents(&composer_json_path, &contents) .map(|_| ()) .ok_or_else(|| anyhow::anyhow!("file_put_contents failed")) }) @@ -305,8 +305,9 @@ impl BumpCommand { updates: &indexmap::IndexMap<&str, indexmap::IndexMap<String, String>>, ) -> anyhow::Result<bool> { let contents = match file_get_contents(json.get_path()) { - Some(c) => c, - None => { + // TODO(bytes): JsonManipulator takes the JSON as a String. + Ok(c) => String::from_utf8_lossy(&c).into_owned(), + Err(_) => { return Err(shirabe_php_shim::RuntimeException::new(format!( "Unable to read {} contents.", json.get_path() diff --git a/crates/shirabe/src/command/init_command.rs b/crates/shirabe/src/command/init_command.rs index 8d892bd3..67530008 100644 --- a/crates/shirabe/src/command/init_command.rs +++ b/crates/shirabe/src/command/init_command.rs @@ -215,7 +215,9 @@ impl InitCommand { fn add_vendor_ignore(&self, ignore_file: &str, vendor: &str) { let mut contents = String::new(); if file_exists(ignore_file) { - contents = file_get_contents(ignore_file).unwrap_or_default(); + // TODO(bytes): the ignore file is edited as a String. + contents = String::from_utf8_lossy(&file_get_contents(ignore_file).unwrap_or_default()) + .into_owned(); if strpos(&contents, "\n") != Some(0) { contents.push('\n'); diff --git a/crates/shirabe/src/command/package_discovery_trait.rs b/crates/shirabe/src/command/package_discovery_trait.rs index 4a915fbe..4816e9d4 100644 --- a/crates/shirabe/src/command/package_discovery_trait.rs +++ b/crates/shirabe/src/command/package_discovery_trait.rs @@ -112,7 +112,9 @@ pub trait PackageDiscoveryTrait: BaseCommand { // @phpstan-ignore-next-line as RequireCommand does not have the option above so this code is reachable there let file = Factory::get_composer_file().unwrap_or_default(); if is_file(&file) && Filesystem::is_readable(&file) { - let contents = file_get_contents(&file).unwrap_or_default(); + // TODO(bytes): json_decode_assoc takes the JSON as a &str. + let contents = + String::from_utf8_lossy(&file_get_contents(&file).unwrap_or_default()).into_owned(); let composer = json_decode_assoc(&contents).unwrap_or(PhpMixed::Null); if is_array(&composer) && let Some(arr) = composer.as_array() diff --git a/crates/shirabe/src/command/require_command.rs b/crates/shirabe/src/command/require_command.rs index 4f90a532..2dfb140d 100644 --- a/crates/shirabe/src/command/require_command.rs +++ b/crates/shirabe/src/command/require_command.rs @@ -51,11 +51,11 @@ pub struct RequireCommand { first_require: std::cell::Cell<bool>, json: std::cell::RefCell<Option<std::rc::Rc<std::cell::RefCell<JsonFile>>>>, file: std::cell::RefCell<String>, - composer_backup: std::cell::RefCell<String>, + composer_backup: std::cell::RefCell<Vec<u8>>, /// file name lock: std::cell::RefCell<String>, /// contents before modification if the lock file exists - lock_backup: std::cell::RefCell<Option<String>>, + lock_backup: std::cell::RefCell<Option<Vec<u8>>>, dependency_resolution_completed: std::rc::Rc<std::cell::Cell<bool>>, repos: std::cell::RefCell<Option<crate::repository::RepositoryInterfaceHandle>>, repository_sets: @@ -78,7 +78,7 @@ impl RequireCommand { first_require: std::cell::Cell::new(false), json: std::cell::RefCell::new(None), file: std::cell::RefCell::new(String::new()), - composer_backup: std::cell::RefCell::new(String::new()), + composer_backup: std::cell::RefCell::new(Vec::new()), lock: std::cell::RefCell::new(String::new()), lock_backup: std::cell::RefCell::new(None), dependency_resolution_completed: std::rc::Rc::new(std::cell::Cell::new(false)), @@ -655,7 +655,11 @@ impl RequireCommand { remove_key: &str, sort_packages: bool, ) -> bool { - let contents = file_get_contents(json.borrow().get_path()).unwrap_or_default(); + // TODO(bytes): JsonManipulator takes the JSON as a String. + let contents = String::from_utf8_lossy( + &file_get_contents(json.borrow().get_path()).unwrap_or_default(), + ) + .into_owned(); let mut manipulator = match JsonManipulator::new(contents) { Ok(m) => m, @@ -712,12 +716,9 @@ impl RequireCommand { extra ); self.get_io().write_error3(&msg, true, io_interface::NORMAL); - file_put_contents( - json.borrow().get_path(), - self.composer_backup.borrow().as_bytes(), - ); + file_put_contents(json.borrow().get_path(), &self.composer_backup.borrow()); if let Some(ref lock_backup) = *self.lock_backup.borrow() { - file_put_contents(&lock, lock_backup.as_bytes()); + file_put_contents(&lock, lock_backup); } } } @@ -837,7 +838,7 @@ impl Command for RequireCommand { file_get_contents(json.borrow().get_path()).unwrap_or_default(); let lock = self.lock.borrow().clone(); *self.lock_backup.borrow_mut() = if file_exists(&lock) { - file_get_contents(&lock) + file_get_contents(&lock).ok() } else { None }; @@ -859,7 +860,7 @@ impl Command for RequireCommand { let backup_contents = self.composer_backup.borrow().clone(); if !is_writable(&file) && Silencer::call(|| { - shirabe_php_shim::file_put_contents(&file_path, backup_contents.as_bytes()); + shirabe_php_shim::file_put_contents(&file_path, &backup_contents); Ok::<bool, anyhow::Error>(false) }) .ok() diff --git a/crates/shirabe/src/config/json_config_source.rs b/crates/shirabe/src/config/json_config_source.rs index 59146630..5e927398 100644 --- a/crates/shirabe/src/config/json_config_source.rs +++ b/crates/shirabe/src/config/json_config_source.rs @@ -48,7 +48,11 @@ impl JsonConfigSource { .into()); } - contents = file_get_contents(self.file.borrow().get_path()).unwrap_or_default(); + // TODO(bytes): JsonManipulator takes the JSON as a String. + contents = String::from_utf8_lossy( + &file_get_contents(self.file.borrow().get_path()).unwrap_or_default(), + ) + .into_owned(); } else if self.auth_config { contents = "{\n}\n".to_string(); } else { diff --git a/crates/shirabe/src/console/application.rs b/crates/shirabe/src/console/application.rs index 24aee249..233ff751 100644 --- a/crates/shirabe/src/console/application.rs +++ b/crates/shirabe/src/console/application.rs @@ -2263,7 +2263,7 @@ impl ApplicationHandle { bin2hex(&random_bytes(5)) ); if !(file_put_contents(&tempfile, file!().as_bytes()).is_some_and(|n| n > 0) - && file_get_contents(&tempfile).as_deref() == Some(file!()) + && file_get_contents(&tempfile).as_deref().ok() == Some(file!().as_bytes()) && unlink(&tempfile).is_ok() && !file_exists(&tempfile)) { @@ -2280,9 +2280,11 @@ impl ApplicationHandle { // add non-standard scripts as own commands let file = Factory::get_composer_file().unwrap_or_default(); if may_need_script_command && is_file(&file) && Filesystem::is_readable(&file) { - let composer_json: PhpMixed = - json_decode_assoc(&file_get_contents(&file).unwrap_or_default()) - .unwrap_or(PhpMixed::Null); + // TODO(bytes): json_decode_assoc takes the JSON as a &str. + let composer_json: PhpMixed = json_decode_assoc(&String::from_utf8_lossy( + &file_get_contents(&file).unwrap_or_default(), + )) + .unwrap_or(PhpMixed::Null); if let Some(arr) = composer_json.as_array() && let Some(scripts) = arr.get("scripts").and_then(|v| v.as_array()) { diff --git a/crates/shirabe/src/downloader/zip_downloader.rs b/crates/shirabe/src/downloader/zip_downloader.rs index cdb9307c..c0470162 100644 --- a/crates/shirabe/src/downloader/zip_downloader.rs +++ b/crates/shirabe/src/downloader/zip_downloader.rs @@ -12,7 +12,7 @@ use shirabe_php_shim::Catch as _; use shirabe_php_shim::{ CmpOp, ErrorException, PhpMixed, RuntimeException, UnexpectedValueException, ZipArchive, bin2hex, class_exists, file_exists, file_get_contents, filesize, hash_file, impl_php_class, - is_file, json_encode, php_regex, preg_match, random_int, str_replace, strlen, substr, + is_file, json_encode, php_regex, preg_match, random_int, str_replace, strlen, substr_bytes, version_compare, }; use shirabe_symfony_process::ExecutableFinder; @@ -233,16 +233,19 @@ impl ZipDownloader { )); self.inner.io.borrow().write_error(&format!( "First 100 bytes (hex): {}", - bin2hex( - substr(&file_get_contents(file).unwrap_or_default(), 0, Some(100)) - .as_bytes() - ) + bin2hex(&substr_bytes( + &file_get_contents(file).unwrap_or_default(), + 0, + Some(100) + )) )); self.inner.io.borrow().write_error(&format!( "Last 100 bytes (hex): {}", - bin2hex( - substr(&file_get_contents(file).unwrap_or_default(), -100, None).as_bytes() - ) + bin2hex(&substr_bytes( + &file_get_contents(file).unwrap_or_default(), + -100, + None + )) )); if strlen(&package.get_dist_url().unwrap_or_default()) > 0 { self.inner.io.borrow().write_error(&format!( diff --git a/crates/shirabe/src/factory.rs b/crates/shirabe/src/factory.rs index afdedd81..6889a20d 100644 --- a/crates/shirabe/src/factory.rs +++ b/crates/shirabe/src/factory.rs @@ -766,7 +766,11 @@ impl Factory { Some(io.clone()), )?, im, - &file_get_contents(composer_file_path).unwrap_or_default(), + // TODO(bytes): Locker takes the composer.json contents as a &str + // to hash them. + &String::from_utf8_lossy( + &file_get_contents(composer_file_path).unwrap_or_default(), + ), process, ); composer_full diff --git a/crates/shirabe/src/installer/binary_installer.rs b/crates/shirabe/src/installer/binary_installer.rs index 554620df..1f70a5ca 100644 --- a/crates/shirabe/src/installer/binary_installer.rs +++ b/crates/shirabe/src/installer/binary_installer.rs @@ -9,9 +9,9 @@ use crate::util::Platform; use crate::util::ProcessExecutor; use crate::util::Silencer; use shirabe_php_shim::{ - PhpMixed, basename, basename_with_suffix, chmod, dirname, fclose, fgets, file_exists, - file_get_contents5, file_put_contents, fopen, is_dir, is_file, is_link, php_regex, preg_match, - realpath, rmdir, substr, trim, umask, + basename, basename_with_suffix, chmod, dirname, fclose, fgets, file_exists, + file_get_contents_with_max_length, file_put_contents, fopen, is_dir, is_file, is_link, + php_regex, preg_match, realpath, rmdir, substr, trim, umask, }; /// Seam over the BinaryInstaller methods reached through LibraryInstaller, so tests can inject a @@ -311,8 +311,12 @@ impl BinaryInstaller { let bin_dir = ProcessExecutor::escape(&dirname(&bin_path)); let bin_file = basename(&bin_path); - let bin_contents = - file_get_contents5(bin, false, PhpMixed::Null, 0, Some(500)).unwrap_or_default(); + // TODO(bytes): preg_match matches over a &str, and the shebang it captures is spliced + // into the generated proxy as a String. + let bin_contents = String::from_utf8_lossy( + &file_get_contents_with_max_length(bin, 500).unwrap_or_default(), + ) + .into_owned(); // For php files, we generate a PHP proxy instead of a shell one, // which allows calling the proxy with a custom php process if let Some(m) = preg_match( diff --git a/crates/shirabe/src/json/json_file.rs b/crates/shirabe/src/json/json_file.rs index c4a06d77..703b9fbb 100644 --- a/crates/shirabe/src/json/json_file.rs +++ b/crates/shirabe/src/json/json_file.rs @@ -165,7 +165,11 @@ impl JsonFile { io_interface::NORMAL, ); } - Ok(file_get_contents(&self.path)) + // TODO(bytes): the JSON text travels as a String, as it does on the + // HttpDownloader branch above. + Ok(file_get_contents(&self.path) + .map(|c| String::from_utf8_lossy(&c).into_owned()) + .ok()) } })() { Ok(j) => j, @@ -282,10 +286,10 @@ impl JsonFile { content: &str, ) -> anyhow::Result<Option<i64>> { // PHP: @file_get_contents($path) - let current_content = Silencer::call(|| Ok(file_get_contents(path))) + let current_content = Silencer::call(|| Ok(file_get_contents(path).ok())) .ok() .flatten(); - if current_content.is_none() || current_content.as_deref() != Some(content) { + if current_content.is_none() || current_content.as_deref() != Some(content.as_bytes()) { return Ok(file_put_contents(path, content.as_bytes())); } @@ -307,7 +311,9 @@ impl JsonFile { )) .into()); } - let content = file_get_contents(&self.path).unwrap_or_default(); + // TODO(bytes): json_decode_obj and validate_syntax take the JSON as a &str. + let content = String::from_utf8_lossy(&file_get_contents(&self.path).unwrap_or_default()) + .into_owned(); let data = json_decode_obj(&content)?; if matches!(data, PhpMixed::Null) && content != "null" { diff --git a/crates/shirabe/src/package/locker.rs b/crates/shirabe/src/package/locker.rs index b98b8dae..3af06ce3 100644 --- a/crates/shirabe/src/package/locker.rs +++ b/crates/shirabe/src/package/locker.rs @@ -631,10 +631,10 @@ impl Locker { Box<dyn FnOnce(IndexMap<String, PhpMixed>) -> IndexMap<String, PhpMixed>>, >, ) -> anyhow::Result<()> { - let contents = file_get_contents(composer_json.get_path()); - let contents = match contents { - Some(s) => s, - None => { + let contents = match file_get_contents(composer_json.get_path()) { + // TODO(bytes): get_content_hash parses the contents as a &str. + Ok(s) => String::from_utf8_lossy(&s).into_owned(), + Err(_) => { return Err(RuntimeException::new(format!( "Unable to read {} contents to update the lock file hash.", composer_json.get_path() diff --git a/crates/shirabe/src/plugin/plugin_manager.rs b/crates/shirabe/src/plugin/plugin_manager.rs index 04c53d08..bc434262 100644 --- a/crates/shirabe/src/plugin/plugin_manager.rs +++ b/crates/shirabe/src/plugin/plugin_manager.rs @@ -434,10 +434,13 @@ impl PluginManager { let path = class_loader.find_file(&class).unwrap_or_else(|| { panic!("plugin class `{class}` is already defined but has no autoloadable file") }); - // TODO(bytes): file_get_contents is lossy UTF-8; the eval'd plugin source - // should be carried as bytes. - let code = file_get_contents(&path) - .unwrap_or_else(|| panic!("unable to read the plugin class file `{path}`")); + // TODO(bytes): the eval'd plugin source is carried as a String, so + // from_utf8_lossy corrupts a source file that is not valid UTF-8. + let code = + String::from_utf8_lossy(&file_get_contents(&path).unwrap_or_else(|_| { + panic!("unable to read the plugin class file `{path}`") + })) + .into_owned(); let class_counter = CLASS_COUNTER.load(std::sync::atomic::Ordering::Relaxed); let separator_pos = strrpos(&class, "\\"); let mut class_name = class.clone(); diff --git a/crates/shirabe/src/repository/path_repository.rs b/crates/shirabe/src/repository/path_repository.rs index 17f22d40..25a5ad57 100644 --- a/crates/shirabe/src/repository/path_repository.rs +++ b/crates/shirabe/src/repository/path_repository.rs @@ -184,7 +184,11 @@ impl PathRepository { continue; } - let json = file_get_contents(&composer_file_path).unwrap_or_default(); + // TODO(bytes): JsonFile::parse_json takes the JSON as a &str. + let json = String::from_utf8_lossy( + &file_get_contents(&composer_file_path).unwrap_or_default(), + ) + .into_owned(); let parsed = JsonFile::parse_json(Some(&json), Some(&composer_file_path))?; let mut package: IndexMap<String, PhpMixed> = match parsed { PhpMixed::Array(m) => m.into_iter().collect(), diff --git a/crates/shirabe/src/util/config_validator.rs b/crates/shirabe/src/util/config_validator.rs index f8018fe5..93d3b4bb 100644 --- a/crates/shirabe/src/util/config_validator.rs +++ b/crates/shirabe/src/util/config_validator.rs @@ -72,7 +72,11 @@ impl ConfigValidator { } if manifest.is_some() { - let contents = shirabe_php_shim::file_get_contents(file).unwrap_or_default(); + // TODO(bytes): detect_duplicate_keys scans the JSON as a &str. + let contents = String::from_utf8_lossy( + &shirabe_php_shim::file_get_contents(file).unwrap_or_default(), + ) + .into_owned(); if let Some((key, line)) = detect_duplicate_keys(&contents) { warnings.push(format!("Key {key} is a duplicate in {file} at line {line}")); } diff --git a/crates/shirabe/src/util/filesystem.rs b/crates/shirabe/src/util/filesystem.rs index 05146fd3..8408a8a9 100644 --- a/crates/shirabe/src/util/filesystem.rs +++ b/crates/shirabe/src/util/filesystem.rs @@ -804,7 +804,7 @@ impl Filesystem { } if is_file(path) { - return Silencer::call(|| Ok(file_get_contents(path).is_some())).unwrap_or(false); + return Silencer::call(|| Ok(file_get_contents(path).is_ok())).unwrap_or(false); } if is_dir(path) { @@ -1018,7 +1018,7 @@ impl Filesystem { pub fn file_put_contents_if_modified(&self, path: &str, content: &str) -> anyhow::Result<i64> { let current_content = Silencer::call(|| Ok(file_get_contents(path).unwrap_or_default())).unwrap_or_default(); - if current_content.is_empty() || current_content != content { + if current_content.is_empty() || current_content != content.as_bytes() { return Ok(file_put_contents(path, content.as_bytes()).unwrap_or(0)); } diff --git a/crates/shirabe/src/util/http_downloader.rs b/crates/shirabe/src/util/http_downloader.rs index 8d34dcdf..f9f69c9b 100644 --- a/crates/shirabe/src/util/http_downloader.rs +++ b/crates/shirabe/src/util/http_downloader.rs @@ -466,7 +466,7 @@ impl HttpDownloader { let _ = stream_context_create(&ctx_options, None); let test_connectivity = file_get_contents("https://8.8.8.8"); Silencer::restore(); - if test_connectivity.is_some() { + if test_connectivity.is_ok() { return Some(vec![ "<error>The following exception probably indicates you have misconfigured DNS resolver(s)</error>".to_string(), ]); diff --git a/crates/shirabe/src/util/perforce.rs b/crates/shirabe/src/util/perforce.rs index 295851c6..3804eed7 100644 --- a/crates/shirabe/src/util/perforce.rs +++ b/crates/shirabe/src/util/perforce.rs @@ -371,8 +371,9 @@ impl Perforce { p4_create_client_command, None, None, + // TODO(bytes): Process carries its stdin as a PhpMixed::String. file_get_contents(self.get_p4_client_spec()) - .map(PhpMixed::String) + .map(|s| PhpMixed::String(String::from_utf8_lossy(&s).into_owned())) .unwrap_or(PhpMixed::Null), None, )?; diff --git a/crates/shirabe/src/util/platform.rs b/crates/shirabe/src/util/platform.rs index ad530db3..bb3dd739 100644 --- a/crates/shirabe/src/util/platform.rs +++ b/crates/shirabe/src/util/platform.rs @@ -164,13 +164,14 @@ impl Platform { return false; } - let file_contents = Silencer::call(|| Ok(file_get_contents("/proc/version"))) + let file_contents = Silencer::call(|| Ok(file_get_contents("/proc/version").ok())) .ok() .flatten() .unwrap_or_default(); if !ini_get("open_basedir").is_some_and(|s| PhpMixed::String(s).to_bool()) && is_readable("/proc/version") - && stripos(&file_contents, "microsoft").is_some() + // TODO(bytes) + && stripos(&String::from_utf8_lossy(&file_contents), "microsoft").is_some() && !Self::is_docker() // Docker and Podman running inside WSL should not be seen as WSL { @@ -224,11 +225,12 @@ impl Platform { Err(_) => break, }; let data = match data { - Some(d) => d, - None => continue, + Ok(d) => d, + Err(_) => continue, }; // detect default mount points created by Docker/containerd - if data.contains("/var/lib/docker/") || data.contains("/io.containerd.snapshotter") { + let contains = |needle: &[u8]| data.windows(needle.len()).any(|w| w == needle); + if contains(b"/var/lib/docker/") || contains(b"/io.containerd.snapshotter") { *cached = Some(true); return true; } diff --git a/crates/shirabe/src/util/remote_filesystem.rs b/crates/shirabe/src/util/remote_filesystem.rs index b584d722..85a48808 100644 --- a/crates/shirabe/src/util/remote_filesystem.rs +++ b/crates/shirabe/src/util/remote_filesystem.rs @@ -17,10 +17,10 @@ use shirabe_php_shim::Catch as _; use shirabe_php_shim::{ PhpMixed, RuntimeException, STREAM_NOTIFY_FAILURE, STREAM_NOTIFY_FILE_SIZE_IS, STREAM_NOTIFY_PROGRESS, array_replace_recursive, base64_encode, explode, extension_loaded, - file_get_contents, file_get_contents5, file_put_contents, filter_var_boolean, gethostbyname, - http_clear_last_response_headers, http_get_last_response_headers, ini_get, json_decode_assoc, - parse_url, php_regex, preg_is_match, preg_match, preg_quote, preg_replace, strpos, strtolower, - strtr, substr, trim, zlib_decode, + file_get_contents, file_get_contents_with_max_length, file_put_contents, filter_var_boolean, + gethostbyname, http_clear_last_response_headers, http_get_last_response_headers, ini_get, + json_decode_assoc, parse_url, php_regex, preg_is_match, preg_match, preg_quote, preg_replace, + strpos, strtolower, strtr, substr, trim, zlib_decode, }; /// Result of `RemoteFilesystem::get` — string content, `true` (for copy), or `false`. @@ -715,7 +715,7 @@ impl RemoteFilesystem { response_headers: &mut Vec<String>, max_file_size: Option<i64>, ) -> anyhow::Result<Option<String>> { - let mut result: Option<String> = None; + let mut result: Option<Vec<u8>> = None; // PHP reads the magic `$http_response_header` variable instead before 8.4, which is where // http_get_last_response_headers() and its companion appeared. @@ -725,12 +725,13 @@ impl RemoteFilesystem { // PHP has no scheme branch here: `file_get_contents` reads `file://` URLs and plain // (scheme-less) local paths through the same stream wrapper it uses for the network // schemes. Only the local subset is modeled so far. - let outer: Result<Option<String>, anyhow::Error> = + let outer: Result<Option<Vec<u8>>, anyhow::Error> = if self.scheme == "file" || self.scheme.is_empty() { Ok(match max_file_size { - Some(max) => file_get_contents5(file_url, false, PhpMixed::Null, 0, Some(max)), + Some(max) => file_get_contents_with_max_length(file_url, max as usize), None => file_get_contents(file_url), - }) + } + .ok()) } else { // TODO(http): wrap PHP's `file_get_contents` with stream context and error capture // for http(s) and other network schemes; depends on the unmodeled PHP stream-context @@ -742,13 +743,15 @@ impl RemoteFilesystem { Err(e) => caught_e = Some(e), } + // Platform::strlen counts bytes whichever branch it takes, so the length is read off the + // buffer directly. if let Some(ref r) = result && let Some(max) = max_file_size - && Platform::strlen(r) >= max + && r.len() as i64 >= max { return Err(MaxFileSizeExceededException::new(format!( "Maximum allowed download size reached. Downloaded {} of allowed {} bytes", - Platform::strlen(r), + r.len(), max )) .into()); @@ -761,7 +764,9 @@ impl RemoteFilesystem { return Err(e); } - Ok(result) + // TODO(bytes): the body is handed back as a String because RemoteFilesystem::get and + // GetResult carry it as one; from_utf8_lossy corrupts binary payloads. + Ok(result.map(|r| String::from_utf8_lossy(&r).into_owned())) } fn callback_get( diff --git a/crates/shirabe/tests/dependency_resolver/pool_builder_test.rs b/crates/shirabe/tests/dependency_resolver/pool_builder_test.rs index a9b59596..50c97366 100644 --- a/crates/shirabe/tests/dependency_resolver/pool_builder_test.rs +++ b/crates/shirabe/tests/dependency_resolver/pool_builder_test.rs @@ -54,7 +54,9 @@ fn load_package( } fn read_test_file(file: &str, fixtures_dir: &str) -> IndexMap<String, String> { - let contents = shirabe_php_shim::file_get_contents(file).unwrap(); + // TODO(bytes) + let contents = + String::from_utf8_lossy(&shirabe_php_shim::file_get_contents(file).unwrap()).into_owned(); let tokens = preg_split_delim_capture(php_regex!(r"#(?:^|\n*)--([A-Z-]+)--\n#"), &contents); // PHP section_info is a map of name => required flag. diff --git a/crates/shirabe/tests/dependency_resolver/pool_optimizer_test.rs b/crates/shirabe/tests/dependency_resolver/pool_optimizer_test.rs index b84d152e..928119f3 100644 --- a/crates/shirabe/tests/dependency_resolver/pool_optimizer_test.rs +++ b/crates/shirabe/tests/dependency_resolver/pool_optimizer_test.rs @@ -58,7 +58,9 @@ fn reduce_packages_info_for_comparison(packages: &[BasePackageHandle]) -> Vec<St } fn read_test_file(file: &str, fixtures_dir: &str) -> IndexMap<String, String> { - let contents = shirabe_php_shim::file_get_contents(file).unwrap(); + // TODO(bytes) + let contents = + String::from_utf8_lossy(&shirabe_php_shim::file_get_contents(file).unwrap()).into_owned(); let tokens = preg_split_delim_capture(php_regex!(r"#(?:^|\n*)--([A-Z-]+)--\n#"), &contents); let section_info: Vec<&str> = vec!["TEST", "REQUEST", "POOL-BEFORE", "POOL-AFTER"]; diff --git a/crates/shirabe/tests/repository/path_repository_test.rs b/crates/shirabe/tests/repository/path_repository_test.rs index c892ddc7..9d56ec10 100644 --- a/crates/shirabe/tests/repository/path_repository_test.rs +++ b/crates/shirabe/tests/repository/path_repository_test.rs @@ -295,7 +295,11 @@ fn test_reference_config() { "sha1", &format!( "{}{}", - file_get_contents(format!("{}/composer.json", dist_url)).unwrap_or_default(), + // TODO(bytes) + String::from_utf8_lossy( + &file_get_contents(format!("{}/composer.json", dist_url)) + .unwrap_or_default() + ), serialize(&PhpMixed::Array(options.clone())) ) )) diff --git a/crates/shirabe/tests/util/remote_filesystem_test.rs b/crates/shirabe/tests/util/remote_filesystem_test.rs index d294c5d2..4421f044 100644 --- a/crates/shirabe/tests/util/remote_filesystem_test.rs +++ b/crates/shirabe/tests/util/remote_filesystem_test.rs @@ -267,7 +267,14 @@ fn test_copy() { GetResult::True )); assert!(std::path::Path::new(&file).exists()); - assert!(strpos(&file_get_contents(&file).unwrap_or_default(), "testCopy").is_some()); + // TODO(bytes) + assert!( + strpos( + &String::from_utf8_lossy(&file_get_contents(&file).unwrap_or_default()), + "testCopy" + ) + .is_some() + ); unlink(&file); } |