diff --git a/Cargo.lock b/Cargo.lock index a12b06b3b2..a2991d85ec 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2440,7 +2440,6 @@ dependencies = [ "bstr", "forge_domain", "hex", - "infer", "pretty_assertions", "sha2 0.11.0", "tempfile", diff --git a/crates/forge_fs/Cargo.toml b/crates/forge_fs/Cargo.toml index 4232e439cc..c3ed670fac 100644 --- a/crates/forge_fs/Cargo.toml +++ b/crates/forge_fs/Cargo.toml @@ -9,7 +9,6 @@ tokio.workspace = true anyhow.workspace = true sha2.workspace = true hex.workspace = true -infer = "0.22.0" # For binary file detection thiserror = "2.0" bstr.workspace = true diff --git a/crates/forge_fs/src/is_binary.rs b/crates/forge_fs/src/is_binary.rs deleted file mode 100644 index 6dfe303cf5..0000000000 --- a/crates/forge_fs/src/is_binary.rs +++ /dev/null @@ -1,104 +0,0 @@ -use anyhow::Result; -use tokio::fs::File; -use tokio::io::AsyncReadExt; - -impl crate::ForgeFS { - /// Checks if a file is binary by examining its content. - /// This version takes a path and opens the file itself. - #[cfg(test)] - async fn is_binary_path>(path: T) -> Result<(bool, String)> { - use anyhow::Context; - - let path_ref = path.as_ref(); - let mut file = File::open(path_ref) - .await - .with_context(|| format!("Failed to open file {}", path_ref.display()))?; - - Self::is_binary(&mut file).await - } - - /// Checks if a file is binary by examining its content. - /// This version takes an already opened file handle, allowing for reuse - /// of the same file handle across multiple operations. - /// This is a crate-private implementation detail. - pub(crate) async fn is_binary(file: &mut File) -> Result<(bool, String)> { - // Read sample data - let mut sample = vec![0; 8192]; - let bytes_read = file.read(&mut sample).await?; - sample.truncate(bytes_read); - - // Handle empty files - if bytes_read == 0 { - return Ok((true, "Empty file".into())); - } - - // Get file type info - let is_text = match infer::get(&sample) { - Some(info) => matches!( - info.matcher_type(), - infer::MatcherType::Text | infer::MatcherType::Doc - ), - None => true, // Assume text if type can't be determined - }; - - let description = infer::get(&sample) - .map(|info| info.mime_type().to_string()) - .unwrap_or_else(|| "Text file (no specific format detected)".into()); - - Ok((is_text, description)) - } -} - -#[cfg(test)] -mod test { - use anyhow::Result; - use tokio::fs; - - async fn create_test_file(content: &[u8]) -> Result { - let file = tempfile::NamedTempFile::new()?; - fs::write(file.path(), content).await?; - Ok(file) - } - - #[tokio::test] - async fn test_is_binary_file() -> Result<()> { - // Test text file - let text_file = create_test_file(b"Hello, world!").await?; - let (is_text_or_doc, _) = crate::ForgeFS::is_binary_path(text_file.path()).await?; - assert!(is_text_or_doc, "Text file should be identified as text"); - - // Test binary data - let binary_content = vec![0, 1, 2, 3, 0, 0, 0, 0, 5, 6, 7, 8]; - let binary_file = create_test_file(&binary_content).await?; - let (is_text_or_doc, file_type) = - crate::ForgeFS::is_binary_path(binary_file.path()).await?; - - if !is_text_or_doc { - assert!( - file_type.contains("binary") || !file_type.contains("text"), - "Binary file type description should indicate binary" - ); - } - - // Test PNG file - let png_header = [ - 0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, 0x00, 0x00, 0x00, 0x0D, 0x49, 0x48, - 0x44, 0x52, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x08, 0x02, 0x00, 0x00, - 0x00, - ]; - let png_file = create_test_file(&png_header).await?; - let (is_text_or_doc, file_type) = crate::ForgeFS::is_binary_path(png_file.path()).await?; - assert!(!is_text_or_doc, "PNG file should be identified as binary"); - assert!( - file_type.contains("image/png"), - "PNG file type should be correctly identified" - ); - - // Test empty file - let empty_file = create_test_file(&[]).await?; - let (is_text_or_doc, _) = crate::ForgeFS::is_binary_path(empty_file.path()).await?; - assert!(is_text_or_doc, "Empty file should be considered text"); - - Ok(()) - } -} diff --git a/crates/forge_fs/src/lib.rs b/crates/forge_fs/src/lib.rs index e1d570ee9d..407043d93b 100644 --- a/crates/forge_fs/src/lib.rs +++ b/crates/forge_fs/src/lib.rs @@ -11,7 +11,6 @@ mod binary_detection; mod error; mod file_size; -mod is_binary; mod meta; mod read; mod read_range; diff --git a/crates/forge_fs/src/read_range.rs b/crates/forge_fs/src/read_range.rs index ed48f5de1b..0531e235dd 100644 --- a/crates/forge_fs/src/read_range.rs +++ b/crates/forge_fs/src/read_range.rs @@ -6,6 +6,7 @@ use bstr::ByteSlice; use forge_domain::FileInfo; use crate::error::Error; +use crate::is_binary; impl crate::ForgeFS { /// Reads a specific range of lines from a file. @@ -33,18 +34,18 @@ impl crate::ForgeFS { return Err(Error::StartGreaterThanEnd { start: start_line, end: end_line }.into()); } - // Open and check if file is binary - let mut file = tokio::fs::File::open(path_ref) - .await - .with_context(|| format!("Failed to open file {}", path_ref.display()))?; - if start_line == 0 || end_line == 0 { return Err(Error::IndexStartingWithZero { start: start_line, end: end_line }.into()); } - let (is_text, file_type) = Self::is_binary(&mut file).await?; - if !is_text { - return Err(Error::BinaryFileNotSupported(file_type).into()); + // Use the same BOM + zero-byte detector that fs_search and other + // callers use, so binary classification is consistent across all + // code paths. The previous infer-based check could classify a file + // as text here while fs_search skipped it as binary. + if is_binary(path_ref).await? { + return Err(Error::BinaryFileNotSupported( + "binary content detected".into(), + ).into()); } // Read file content @@ -332,4 +333,18 @@ mod test { Ok(()) } + + #[tokio::test] + async fn test_stray_zero_byte_classified_as_binary() -> Result<()> { + // Reproduces #3633: a file with a stray 0x00 in the first 512 bytes + // should be rejected as binary by read_range_utf8, matching what + // fs_search sees via the BOM-based detector. + let content = b"KEY=value\0trailing text\n"; + let file = tempfile::NamedTempFile::new()?; + fs::write(file.path(), content).await?; + + let result = crate::ForgeFS::read_range_utf8(file.path(), 1, 1).await; + assert!(result.is_err(), "file with stray NUL should be rejected as binary"); + Ok(()) + } }