fix: detect OOXML docs with reordered ZIP entries

src/matchers/doc.rs

@@ -2,8 +2,27 @@ use core::convert::TryInto;
 
 use super::compare_bytes;
 
+// ZIP local file header fields used by the structured OOXML scan. Unlike the
+// legacy detector (which searches for the next `PK\x03\x04` in a byte window),
+// this path advances by parsing each header's lengths and compressed size.
+const ZIP_LOCAL_FILE_HEADER: &[u8] = b"PK\x03\x04";
+const ZIP_LOCAL_FILE_HEADER_LEN: usize = 30;
+const ZIP_FLAGS_OFFSET: usize = 6;
+const ZIP_COMPRESSED_SIZE_OFFSET: usize = 18;
+const ZIP_FILE_NAME_LENGTH_OFFSET: usize = 26;
+const ZIP_EXTRA_FIELD_LENGTH_OFFSET: usize = 28;
+// When set, the real sizes follow the file data in a data descriptor, so the
+// compressed-size field in the local header cannot be trusted for seeking.
+const ZIP_DATA_DESCRIPTOR_FLAG: u16 = 0x0008;
+// ZIP64 stores the real size in the extra field; `u32::MAX` is only a marker.
+const ZIP64_SIZE: u32 = u32::MAX;
+// `get_from_path` reads at most 8 KiB. Deriving the iteration bound from the
+// smallest possible local header covers every header parseable in that window
+// while keeping direct buffer matching bounded as well.
+const MAX_OOXML_ENTRIES: usize = 8192 / ZIP_LOCAL_FILE_HEADER_LEN;
+
 #[allow(clippy::upper_case_acronyms)]
-#[derive(Debug, Eq, PartialEq)]
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
 enum DocType {
     DOC,
     DOCX,
@@ -14,6 +33,13 @@ enum DocType {
     OOXML,
 }
 
+// One ZIP local file header: the entry name used for classification, and the
+// absolute offset of the next header when it can be computed safely.
+struct LocalFileHeader<'a> {
+    file_name: &'a [u8],
+    next_offset: Option<usize>,
+}
+
 /// Returns whether a buffer is Microsoft Word Document (DOC) data.
 #[must_use]
 pub fn is_doc(buf: &[u8]) -> bool {
@@ -51,6 +77,77 @@ pub fn is_pptx(buf: &[u8]) -> bool {
 }
 
 fn msooxml(buf: &[u8]) -> Option<DocType> {
+    // Preserve every positive result from the original positional heuristic.
+    // The structured scan below only adds conservative matches for layouts the
+    // legacy detector could not classify (e.g. metadata entries before the
+    // office namespace, or a main part that is not among the first few files).
+    let legacy_type = legacy_msooxml(buf);
+    if let Some(doc_type) = legacy_type {
+        // Concrete types win immediately; a bare `OOXML` result is kept so the
+        // structured scan can still try to refine it.
+        if doc_type != DocType::OOXML {
+            return Some(doc_type);
+        }
+    }
+
+    // Walk ZIP local headers by parsing each entry instead of scanning for the
+    // next `PK` signature. This is order-independent and ignores `PK\x03\x04`
+    // sequences that happen to appear inside file data or extra fields.
+    let mut header_offset = 0;
+    let mut saw_content_types = false;
+    let mut saw_package_relationships = false;
+    let mut main_part_type = None;
+    let mut ambiguous_type = false;
+
+    for _ in 0..MAX_OOXML_ENTRIES {
+        let header = match local_file_header(buf, header_offset) {
+            Some(header) => header,
+            None => break,
+        };
+
+        // Package-level OPC files required by every OOXML document.
+        if header.file_name == b"[Content_Types].xml" {
+            saw_content_types = true;
+        } else if header.file_name == b"_rels/.rels" {
+            saw_package_relationships = true;
+        }
+
+        // Record the first canonical main part; conflicting main parts make the
+        // archive ambiguous (e.g. both word/ and xl/ present).
+        if let Some(doc_type) = check_msooxml_main_part(header.file_name) {
+            match main_part_type {
+                None => main_part_type = Some(doc_type),
+                Some(current) if current == doc_type => {}
+                Some(_) => ambiguous_type = true,
+            }
+        }
+
+        // Stop when the next header cannot be located safely (short buffer,
+        // data descriptor, or ZIP64 sizes).
+        header_offset = match header.next_offset {
+            Some(next_offset) => next_offset,
+            None => break,
+        };
+    }
+
+    // The package-level files and canonical main part are independent of
+    // physical ZIP entry order and together provide a conservative OOXML
+    // signal for archives that the legacy detector could not classify.
+    if !ambiguous_type && saw_content_types && saw_package_relationships && main_part_type.is_some()
+    {
+        return main_part_type;
+    }
+
+    // Fall back to a generic OOXML hit when we saw package markers (or legacy
+    // already did) but could not pin down a single concrete office type.
+    if legacy_type.is_some() || saw_content_types || saw_package_relationships {
+        Some(DocType::OOXML)
+    } else {
+        None
+    }
+}
+
+fn legacy_msooxml(buf: &[u8]) -> Option<DocType> {
     let signature = [b'P', b'K', 0x03, 0x04];
 
     // start by checking for ZIP local file header signature
@@ -162,3 +259,60 @@ fn search(buf: &[u8], start: usize, range: usize) -> Option<usize> {
         .windows(signature.len())
         .position(|window| window == signature)
 }
+
+// Exact main-part names from the OOXML / OPC packages. Prefer these over a
+// bare `word/` / `ppt/` / `xl/` prefix so ordinary ZIPs that happen to contain
+// similarly named paths are not classified as Office documents.
+fn check_msooxml_main_part(file_name: &[u8]) -> Option<DocType> {
+    match file_name {
+        b"word/document.xml" => Some(DocType::DOCX),
+        b"ppt/presentation.xml" => Some(DocType::PPTX),
+        b"xl/workbook.xml" => Some(DocType::XLSX),
+        _ => None,
+    }
+}
+
+// Parse one ZIP local file header at `header_offset`.
+//
+// Layout: signature (4) | ... | flags (2 @ +6) | ... | compressed size (4 @ +18)
+//       | ... | file name length (2 @ +26) | extra length (2 @ +28)
+//       | file name | extra field | file data
+fn local_file_header(buf: &[u8], header_offset: usize) -> Option<LocalFileHeader<'_>> {
+    if !compare_bytes(buf, ZIP_LOCAL_FILE_HEADER, header_offset) {
+        return None;
+    }
+
+    let flags = read_u16(buf, header_offset.checked_add(ZIP_FLAGS_OFFSET)?)?;
+    let compressed_size = read_u32(buf, header_offset.checked_add(ZIP_COMPRESSED_SIZE_OFFSET)?)?;
+    let name_length_offset = header_offset.checked_add(ZIP_FILE_NAME_LENGTH_OFFSET)?;
+    let name_length = usize::from(read_u16(buf, name_length_offset)?);
+    let extra_length_offset = header_offset.checked_add(ZIP_EXTRA_FIELD_LENGTH_OFFSET)?;
+    let extra_length = usize::from(read_u16(buf, extra_length_offset)?);
+    let name_start = header_offset.checked_add(ZIP_LOCAL_FILE_HEADER_LEN)?;
+    let name_end = name_start.checked_add(name_length)?;
+    let file_name = buf.get(name_start..name_end)?;
+    let data_start = name_end.checked_add(extra_length)?;
+
+    // Only compute the next entry offset when the local header's compressed
+    // size is authoritative. Otherwise refuse to seek rather than guess.
+    let next_offset = if flags & ZIP_DATA_DESCRIPTOR_FLAG != 0 || compressed_size == ZIP64_SIZE {
+        None
+    } else {
+        data_start.checked_add(compressed_size as usize)
+    };
+
+    Some(LocalFileHeader {
+        file_name,
+        next_offset,
+    })
+}
+
+fn read_u16(buf: &[u8], offset: usize) -> Option<u16> {
+    let end = offset.checked_add(2)?;
+    Some(u16::from_le_bytes(buf.get(offset..end)?.try_into().ok()?))
+}
+
+fn read_u32(buf: &[u8], offset: usize) -> Option<u32> {
+    let end = offset.checked_add(4)?;
+    Some(u32::from_le_bytes(buf.get(offset..end)?.try_into().ok()?))
+}

tests/doc.rs

@@ -61,3 +61,194 @@ test_format!(
     pptx,
     "sample.pptx"
 );
+
+fn append_zip_entry(
+    buf: &mut Vec<u8>,
+    file_name: &str,
+    extra_field: &[u8],
+    contents: &[u8],
+    flags: u16,
+) {
+    let mut header = [0_u8; 30];
+    header[..4].copy_from_slice(b"PK\x03\x04");
+    header[4..6].copy_from_slice(&20_u16.to_le_bytes());
+    header[6..8].copy_from_slice(&flags.to_le_bytes());
+    header[18..22].copy_from_slice(
+        &u32::try_from(contents.len())
+            .expect("test contents fit in a ZIP local header")
+            .to_le_bytes(),
+    );
+    header[22..26].copy_from_slice(
+        &u32::try_from(contents.len())
+            .expect("test contents fit in a ZIP local header")
+            .to_le_bytes(),
+    );
+    header[26..28].copy_from_slice(
+        &u16::try_from(file_name.len())
+            .expect("test file name fits in a ZIP local header")
+            .to_le_bytes(),
+    );
+    header[28..30].copy_from_slice(
+        &u16::try_from(extra_field.len())
+            .expect("test extra field fits in a ZIP local header")
+            .to_le_bytes(),
+    );
+    buf.extend_from_slice(&header);
+    buf.extend_from_slice(file_name.as_bytes());
+    buf.extend_from_slice(extra_field);
+    buf.extend_from_slice(contents);
+}
+
+fn append_empty_zip_entry(buf: &mut Vec<u8>, file_name: &str) {
+    append_zip_entry(buf, file_name, &[], &[], 0);
+}
+
+fn zip_with_entries(entries: &[&str]) -> Vec<u8> {
+    let mut buf = Vec::new();
+    for entry in entries {
+        append_empty_zip_entry(&mut buf, entry);
+    }
+    buf
+}
+
+#[test]
+fn detects_ooxml_type_after_metadata_entries() {
+    for (main_part, expected_extension) in [
+        ("word/document.xml", "docx"),
+        ("xl/workbook.xml", "xlsx"),
+        ("ppt/presentation.xml", "pptx"),
+    ] {
+        let buf = zip_with_entries(&[
+            "[Content_Types].xml",
+            "_rels/.rels",
+            "docProps/core.xml",
+            "docProps/app.xml",
+            main_part,
+        ]);
+
+        let kind = infer::get(&buf).expect("reordered OOXML file matches");
+        assert_eq!(kind.extension(), expected_extension);
+    }
+}
+
+#[test]
+fn detects_ooxml_when_unrelated_part_is_first() {
+    let buf = zip_with_entries(&[
+        "customXml/item.xml",
+        "ppt/presentation.xml",
+        "docProps/core.xml",
+        "_rels/.rels",
+        "[Content_Types].xml",
+    ]);
+
+    let kind = infer::get(&buf).expect("order-independent OOXML file matches");
+    assert_eq!(kind.extension(), "pptx");
+}
+
+#[test]
+fn preserves_legacy_namespace_pair_detection() {
+    let buf = zip_with_entries(&[
+        "[Content_Types].xml",
+        "_rels/.rels",
+        "ppt/slides/_rels/slide1.xml.rels",
+        "ppt/slides/_rels/slide2.xml.rels",
+    ]);
+
+    let kind = infer::get(&buf).expect("legacy OOXML layout matches");
+    assert_eq!(kind.extension(), "pptx");
+}
+
+#[test]
+fn incomplete_local_header_does_not_panic() {
+    let kind = infer::get(b"PK\x03\x04").expect("ZIP signature matches");
+    assert_eq!(kind.extension(), "zip");
+}
+
+#[test]
+fn ordinary_zip_remains_zip() {
+    let buf = zip_with_entries(&["README.md", "assets/presentation.xml"]);
+    let kind = infer::get(&buf).expect("ZIP signature matches");
+    assert_eq!(kind.extension(), "zip");
+}
+
+#[test]
+fn ooxml_metadata_with_unrelated_office_namespace_remains_zip() {
+    let buf = zip_with_entries(&["[Content_Types].xml", "_rels/.rels", "ppt/assets.bin"]);
+    let kind = infer::get(&buf).expect("ZIP signature matches");
+    assert_eq!(kind.extension(), "zip");
+}
+
+#[test]
+fn structured_scan_ignores_local_header_signature_in_payload() {
+    let mut false_header = Vec::new();
+    append_empty_zip_entry(&mut false_header, "word/document.xml");
+
+    let mut buf = Vec::new();
+    append_zip_entry(&mut buf, "[Content_Types].xml", &[], &false_header, 0);
+    append_empty_zip_entry(&mut buf, "_rels/.rels");
+
+    let kind = infer::get(&buf).expect("ZIP signature matches");
+    assert_eq!(kind.extension(), "zip");
+}
+
+#[test]
+fn structured_scan_ignores_local_header_signature_in_extra_field() {
+    let mut false_header = Vec::new();
+    append_empty_zip_entry(&mut false_header, "xl/workbook.xml");
+
+    let mut buf = Vec::new();
+    append_zip_entry(&mut buf, "[Content_Types].xml", &false_header, &[], 0);
+    append_empty_zip_entry(&mut buf, "_rels/.rels");
+
+    let kind = infer::get(&buf).expect("ZIP signature matches");
+    assert_eq!(kind.extension(), "zip");
+}
+
+#[test]
+fn data_descriptor_entry_uses_conservative_fallback() {
+    let mut false_header = Vec::new();
+    append_empty_zip_entry(&mut false_header, "ppt/presentation.xml");
+
+    let mut buf = Vec::new();
+    append_zip_entry(&mut buf, "[Content_Types].xml", &[], &false_header, 0x0008);
+    append_empty_zip_entry(&mut buf, "_rels/.rels");
+
+    let kind = infer::get(&buf).expect("ZIP signature matches");
+    assert_eq!(kind.extension(), "zip");
+}
+
+#[test]
+fn zip64_entry_uses_conservative_fallback() {
+    let mut buf = zip_with_entries(&["[Content_Types].xml", "_rels/.rels", "ppt/presentation.xml"]);
+    buf[18..22].copy_from_slice(&u32::MAX.to_le_bytes());
+
+    let kind = infer::get(&buf).expect("ZIP signature matches");
+    assert_eq!(kind.extension(), "zip");
+}
+
+#[test]
+fn structured_scan_entry_limit_covers_the_path_read_window() {
+    // A ZIP local header is at least 30 bytes, so infer's 8 KiB path read can
+    // contain at most 273 complete headers. Keep the success case exactly at
+    // the limit and document the conservative direct-buffer boundary after it.
+    let mut within_limit = vec!["misc/part.xml"; 270];
+    within_limit.extend(["[Content_Types].xml", "_rels/.rels", "ppt/presentation.xml"]);
+    let kind = infer::get(&zip_with_entries(&within_limit)).expect("OOXML at limit matches");
+    assert_eq!(kind.extension(), "pptx");
+
+    let mut beyond_limit = vec!["misc/part.xml"; 271];
+    beyond_limit.extend(["[Content_Types].xml", "_rels/.rels", "ppt/presentation.xml"]);
+    let kind = infer::get(&zip_with_entries(&beyond_limit)).expect("ZIP signature matches");
+    assert_eq!(kind.extension(), "zip");
+}
+
+#[test]
+fn truncated_first_namespace_preserves_fast_path() {
+    let mut buf = vec![0_u8; 30];
+    buf[..4].copy_from_slice(b"PK\x03\x04");
+    buf[26..28].copy_from_slice(&100_u16.to_le_bytes());
+    buf.extend_from_slice(b"word/");
+
+    let kind = infer::get(&buf).expect("legacy prefix matches");
+    assert_eq!(kind.extension(), "docx");
+}