More efficient file handling

Author: mbrubeck

src/basetype/check.rs

@@ -1,16 +1,37 @@
 use crate::{read_bytes, Mime};
 use fnv::FnvHashMap;
-use std::path::Path;
+use std::fs::File;
 
 pub(crate) struct BaseType;
 
 impl crate::Checker for BaseType {
-    fn match_bytes(&self, file: &[u8], mimetype: &str) -> bool {
-        match_bytes(file, mimetype)
-    }
-
-    fn match_filepath(&self, filepath: &Path, mimetype: &str) -> bool {
-        match_filepath(filepath, mimetype)
+    fn match_bytes(&self, bytes: &[u8], mimetype: &str) -> bool {
+        if mimetype == "application/octet-stream" || mimetype == "all/allfiles" {
+            // Both of these are the case if we have a bytestream at all
+            return true;
+        }
+        if mimetype == "text/plain" {
+            is_text_plain_from_u8(bytes)
+        } else {
+            // ...how did we get bytes for this?
+            false
+        }
+    }
+
+    fn match_file(&self, file: &File, mimetype: &str) -> bool {
+        // Being bad with error handling here,
+        // but if you can't open it it's probably not a file.
+        let Ok(meta) = file.metadata() else {
+            return false;
+        };
+
+        match mimetype {
+            "all/all" => true,
+            "all/allfiles" | "application/octet-stream" => meta.is_file(),
+            "inode/directory" => meta.is_dir(),
+            "text/plain" => is_text_plain_from_file(file),
+            _ => false,
+        }
     }
 
     fn get_supported(&self) -> Vec<Mime> {
@@ -27,46 +48,14 @@ impl crate::Checker for BaseType {
 }
 
 /// If there are any null bytes, return False. Otherwise return True.
-fn is_text_plain_from_u8(b: &[u8]) -> bool {
-    memchr::memchr(0, b).is_none()
+fn is_text_plain_from_u8(bytes: &[u8]) -> bool {
+    memchr::memchr(0, bytes).is_none()
 }
 
 // TODO: Hoist the main logic here somewhere else. This'll get redundant fast!
-fn is_text_plain_from_filepath(filepath: &Path) -> bool {
-    let Ok(bytes) = read_bytes(filepath, 512) else {
+fn is_text_plain_from_file(file: &File) -> bool {
+    let Ok(bytes) = read_bytes(file, 512) else {
         return false;
     };
     is_text_plain_from_u8(&bytes)
 }
-
-#[allow(unused_variables)]
-pub fn match_bytes(b: &[u8], mimetype: &str) -> bool {
-    if mimetype == "application/octet-stream" || mimetype == "all/allfiles" {
-        // Both of these are the case if we have a bytestream at all
-        return true;
-    }
-    if mimetype == "text/plain" {
-        is_text_plain_from_u8(b)
-    } else {
-        // ...how did we get bytes for this?
-        false
-    }
-}
-
-pub fn match_filepath(filepath: &Path, mimetype: &str) -> bool {
-    use std::fs;
-
-    // Being bad with error handling here,
-    // but if you can't open it it's probably not a file.
-    let Ok(meta) = fs::metadata(filepath) else {
-        return false;
-    };
-
-    match mimetype {
-        "all/all" => true,
-        "all/allfiles" | "application/octet-stream" => meta.is_file(),
-        "inode/directory" => meta.is_dir(),
-        "text/plain" => is_text_plain_from_filepath(filepath),
-        _ => false,
-    }
-}

src/fdo_magic/builtin/check.rs

@@ -2,17 +2,41 @@ use super::ALL_RULES;
 use crate::{fdo_magic::check::from_u8_walker, read_bytes, Mime};
 use fnv::FnvHashMap;
 use petgraph::prelude::*;
-use std::path::Path;
+use std::fs::File;
 
 pub(crate) struct FdoMagic;
 
 impl crate::Checker for FdoMagic {
-    fn match_bytes(&self, file: &[u8], mimetype: &str) -> bool {
-        match_bytes(file, mimetype)
+    fn match_bytes(&self, bytes: &[u8], mimetype: &str) -> bool {
+        // Get magic ruleset
+        let Some(graph) = ALL_RULES.get(mimetype) else {
+            return false;
+        };
+
+        // Check all rulesets
+        graph
+            .externals(Incoming)
+            .any(|node| from_u8_walker(bytes, graph, node, true))
     }
 
-    fn match_filepath(&self, filepath: &Path, mimetype: &str) -> bool {
-        match_filepath(filepath, mimetype)
+    fn match_file(&self, file: &File, mimetype: &str) -> bool {
+        // Get magic ruleset
+        let Some(magic_rules) = ALL_RULES.get(mimetype) else {
+            return false;
+        };
+
+        // Get # of bytes to read
+        let scanlen = magic_rules
+            .node_weights()
+            .map(|rule| rule.scan_len())
+            .max()
+            .unwrap_or(0);
+
+        let Ok(bytes) = read_bytes(file, scanlen) else {
+            return false;
+        };
+
+        self.match_bytes(&bytes, mimetype)
     }
 
     fn get_supported(&self) -> Vec<Mime> {
@@ -27,40 +51,3 @@ impl crate::Checker for FdoMagic {
         super::init::get_aliaslist()
     }
 }
-
-/// Test against all rules
-#[allow(unused_variables)]
-pub fn match_bytes(file: &[u8], mimetype: &str) -> bool {
-    // Get magic ruleset
-    let Some(graph) = ALL_RULES.get(mimetype) else {
-        return false;
-    };
-
-    // Check all rulesets
-    graph
-        .externals(Incoming)
-        .any(|node| from_u8_walker(file, graph, node, true))
-}
-
-/// This only exists for the case of a direct match_filepath call
-/// and even then we could probably get rid of this...
-#[allow(unused_variables)]
-pub fn match_filepath(filepath: &Path, mimetype: &str) -> bool {
-    // Get magic ruleset
-    let Some(magic_rules) = ALL_RULES.get(mimetype) else {
-        return false;
-    };
-
-    // Get # of bytes to read
-    let scanlen = magic_rules
-        .node_weights()
-        .map(|rule| rule.scan_len())
-        .max()
-        .unwrap_or(0);
-
-    let Ok(bytes) = read_bytes(filepath, scanlen) else {
-        return false;
-    };
-
-    match_bytes(&bytes, mimetype)
-}

src/fdo_magic/check.rs

@@ -1,15 +1,15 @@
 use petgraph::prelude::*;
 use std::iter::zip;
 
-fn from_u8_singlerule(file: &[u8], rule: &super::MagicRule) -> bool {
+fn from_u8_singlerule(bytes: &[u8], rule: &super::MagicRule) -> bool {
     // Check if we're even in bounds
     let bound_min = rule.start_off as usize;
     let bound_max = rule.start_off as usize + rule.val.len() + rule.region_len as usize;
 
-    if (file.len()) < bound_max {
+    if (bytes.len()) < bound_max {
         return false;
     }
-    let testarea = &file[bound_min..bound_max];
+    let testarea = &bytes[bound_min..bound_max];
 
     testarea.windows(rule.val.len()).any(|window| {
         // Apply mask to value
@@ -27,7 +27,7 @@ fn from_u8_singlerule(file: &[u8], rule: &super::MagicRule) -> bool {
 /// Test every given rule by walking graph
 /// TODO: Not loving the code duplication here.
 pub fn from_u8_walker(
-    file: &[u8],
+    bytes: &[u8],
     graph: &DiGraph<super::MagicRule, u32>,
     node: NodeIndex,
     isroot: bool,
@@ -38,7 +38,7 @@ pub fn from_u8_walker(
         let rule = &graph[node];
 
         // Check root
-        if !from_u8_singlerule(file, rule) {
+        if !from_u8_singlerule(bytes, rule) {
             return false;
         }
 
@@ -54,10 +54,10 @@ pub fn from_u8_walker(
     for y in n {
         let rule = &graph[y];
 
-        if from_u8_singlerule(file, rule) {
+        if from_u8_singlerule(bytes, rule) {
             // Check next indent level if needed
             if graph.neighbors_directed(y, Outgoing).count() != 0 {
-                return from_u8_walker(file, graph, y, false);
+                return from_u8_walker(bytes, graph, y, false);
             // Next indent level is lower, so this must be it
             } else {
                 return true;