fix(fs): carry file_get_contents results as bytes

file_get_contents() and file_get_contents_with_max_length() return
Vec<u8> instead of a from_utf8_lossy'd String.
Call sites whose consumer takes a &str still convert lossily and are
marked TODO(bytes).

file_get_contents_with_max_length() now reads at most the requested
number of bytes instead of the whole file.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

6 files changed, 33 insertions, 21 deletions

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(