From c0286b17cc23b5cf7e15d2cfa1059de85e159d4b Mon Sep 17 00:00:00 2001
From: sreedevk <sreedev@icloud.com>
Date: Sat, 19 Jul 2025 11:10:12 +0000
Subject: [PATCH] fix: hash collision edge case

---
 README.md              |  7 ++++++-
 rakelib/benchmark.rake |  1 +
 src/fileinfo.rs        | 47 +++++++++++++++++++++++++++++++++++++++---
 src/server.rs          |  8 +++----
 4 files changed, 55 insertions(+), 8 deletions(-)

diff --git a/README.md b/README.md
index dc33f58..45553ec 100644
--- a/README.md
+++ b/README.md
@@ -157,6 +157,10 @@ dust 'bench_artifacts'
     - [ ] --keep-last-modified
     - [ ] --keep-first-modified
 
+- [ ] fix: partial hash collision - a file full of null bytes ("\0") and an empty file. This is a known trade off in gxhash.
+    - [ ] include initial pages and final pages of the file 
+    - [ ] append the offset between the last initial page hashed and the first final page hashed in the content passed to the hasher.
+
 ## v0.3.1
 - [x] parallelization
     - [x] (scanning + processing sw + processing hw) & formatting & printing
@@ -164,7 +168,8 @@ dust 'bench_artifacts'
 - [x] simplify output to improve performance
 - [x] increase the number of pages hashed in partial hashing
 - [x] updated dependencies
-- [ ] POTENTIAL INTERMITTENT BUG: --progress causes characters to be rendered out of order
+- [x] fix: full file hash collision between a file full of null bytes ("\0") and an empty file. This is a known trade off in gxhash.
+    - [x] appending the file size at the end of content before hashing.
 
 ## v0.3.0
 - [x] parallelization
diff --git a/rakelib/benchmark.rake b/rakelib/benchmark.rake
index 6b4eb6d..a1c516a 100644
--- a/rakelib/benchmark.rake
+++ b/rakelib/benchmark.rake
@@ -13,6 +13,7 @@ namespace :benchmark do
     # Range (min … max):     1.8 ms …   9.7 ms    1474 runs
 
     root = "bench_artifacts"
+    FileUtils.rm_rf(root)
     Dir.mkdir(root)
 
     # files with same size
diff --git a/src/fileinfo.rs b/src/fileinfo.rs
index f13191f..f0d091c 100644
--- a/src/fileinfo.rs
+++ b/src/fileinfo.rs
@@ -25,12 +25,18 @@ pub struct FileInfo {
 
 impl FileInfo {
     pub fn hash(&self, seed: i64) -> Result<u128> {
+        if self.size == 0 {
+            return Ok(0u128);
+        };
+
         let file = fs::File::open(&self.path)?;
         let mapper = unsafe { Mmap::map(&file)? };
-
-        Ok(mapper
+        let content_hash = mapper
             .chunks(4096)
-            .fold(0u128, |acc, chunk: &[u8]| acc ^ gxhash128(chunk, seed)))
+            .fold(0u128, |acc, chunk: &[u8]| acc ^ gxhash128(chunk, seed));
+
+        // NOTE: avoids collision bw an empty file & a file full of null bytes.
+        Ok(content_hash ^ gxhash128(&self.size.to_ne_bytes(), seed))
     }
 
     pub fn initpages_hash(&self, seed: i64) -> Result<u128> {
@@ -61,3 +67,38 @@ impl FileInfo {
         *self_state == FileState::SwProcessed
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use tempfile::TempDir;
+    use std::fs::File;
+    use std::io::Write;
+    use anyhow::Result;
+
+    fn generate_null_bytes(size: usize) -> Vec<u8> {
+        (0..size).map(|_| 0).collect::<Vec<u8>>()
+    }
+
+    #[test]
+    fn hash_differentiates_between_a_file_of_null_bytes_vs_an_empty_file() -> Result<()> {
+        let root = TempDir::new()?;
+        let empty_file_name = root.path().join("empty_file.bin");
+
+        File::create_new(&empty_file_name)?;
+
+        let file_with_null_bytes_name = root.path().join("file_with_null_bytes.bin");
+        let mut file_with_null_bytes = File::create_new(&file_with_null_bytes_name)?;
+
+        file_with_null_bytes.write_all(&generate_null_bytes(1000 * 4096))?;
+
+        let empty_file_info = FileInfo::new(empty_file_name)?;
+        let file_with_empty_bytes_info = FileInfo::new(file_with_null_bytes_name)?;
+
+        let seed: i64 = 246910456374;
+
+        assert_ne!(empty_file_info.hash(seed)?, file_with_empty_bytes_info.hash(seed)?);
+
+        Ok(())
+    }
+}
diff --git a/src/server.rs b/src/server.rs
index 1d02f3e..c9e9b65 100644
--- a/src/server.rs
+++ b/src/server.rs
@@ -67,9 +67,9 @@ impl Server {
 
         self.threadpool.execute(move || {
             Scanner::new(app_args_clone_for_sc)
-                .unwrap()
+                .expect("unable to initialize scanner.")
                 .scan(file_queue_clone_sc, progbarbox_sc_clone)
-                .unwrap();
+                .expect("scanner failed.");
 
             sfin_sc_tr_cl.store(true, std::sync::atomic::Ordering::Relaxed);
         });
@@ -82,7 +82,7 @@ impl Server {
                 file_queue_clone_pr,
                 progbarbox_pr_clone_for_sw,
             )
-            .unwrap();
+            .expect("sizewise scanner failed.");
 
             swfin_pr_tr_sw.store(true, std::sync::atomic::Ordering::Relaxed);
         });
@@ -97,7 +97,7 @@ impl Server {
                 seed,
                 swfin_pr_tr_hw,
             )
-            .unwrap();
+            .expect("sizewise scanner failed.");
         });
 
         progbarbox.clear()?;