Repository: gabriel-vasile/mimetype
Pull request #688 (2025-07-15 18:37:56)

Jar first entry inside a zip
Add testcase for #639.
This PR is a continuation from #687 to make mimetype detection behave more like file(1).


Patch:
diff --git a/internal/magic/ms_office.go b/internal/magic/ms_office.go
index 38a5dba..d21b8e8 100644
--- a/internal/magic/ms_office.go
+++ b/internal/magic/ms_office.go
@@ -7,17 +7,26 @@ import (
 
 // Xlsx matches a Microsoft Excel 2007 file.
 func Xlsx(raw []byte, limit uint32) bool {
-	return zipContains(raw, []byte("xl/"), true, 100)
+	return msoxml(raw, zipEntries{{
+		name: []byte("xl/"),
+		dir:  true,
+	}}, 100)
 }
 
 // Docx matches a Microsoft Word 2007 file.
 func Docx(raw []byte, limit uint32) bool {
-	return zipContains(raw, []byte("word/"), true, 100)
+	return msoxml(raw, zipEntries{{
+		name: []byte("word/"),
+		dir:  true,
+	}}, 100)
 }
 
 // Pptx matches a Microsoft PowerPoint 2007 file.
 func Pptx(raw []byte, limit uint32) bool {
-	return zipContains(raw, []byte("ppt/"), true, 100)
+	return msoxml(raw, zipEntries{{
+		name: []byte("ppt/"),
+		dir:  true,
+	}}, 100)
 }
 
 // Ole matches an Open Linking and Embedding file.
diff --git a/internal/magic/zip.go b/internal/magic/zip.go
index 364662f..c1d1f8a 100644
--- a/internal/magic/zip.go
+++ b/internal/magic/zip.go
@@ -2,7 +2,6 @@ package magic
 
 import (
 	"bytes"
-	"encoding/binary"
 
 	"github.com/gabriel-vasile/mimetype/internal/scan"
 )
@@ -48,14 +47,16 @@ func Zip(raw []byte, limit uint32) bool {
 // 2. regular jars, same as above, just without the executable flag
 // https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=262278#c0
 // There is an argument to only check for manifest, since it's the common nominator
-// for both executable and non-executable versions. But zipContains is unreliable
-// because it does linear search for signatures (instead of relying on offsets
-// told by the file.)
+// for both executable and non-executable versions. But the traversing zip entries
+// is unreliable because it does linear search for signatures
+// (instead of relying on offsets told by the file.)
 func Jar(raw []byte, limit uint32) bool {
 	return executableJar(raw) ||
-		zipContains(raw, []byte("META-INF/MANIFEST.MF"), false, 1) ||
-		zipContains(raw, []byte("META-INF/"), false, 1)
-
+		zipHas(raw, zipEntries{{
+			name: []byte("META-INF/MANIFEST.MF"),
+		}, {
+			name: []byte("META-INF/"),
+		}}, 1)
 }
 
 // An executable Jar has a 0xCAFE flag enabled in the first zip entry.
@@ -73,97 +74,107 @@ func executableJar(b scan.Bytes) bool {
 	return ok && cafe == 0xCAFE
 }
 
-// zipContains goes over entries inside the raw zip and checks if any of them are
-// equal to sig. msoCheck makes it look for additional entries and checkLength
-// makes the lookAt limits the number of entries to look for.
-func zipContains(raw, sig []byte, msoCheck bool, lookAt int) bool {
-	b := scan.Bytes(raw)
-	pk := []byte("PK\003\004")
-	if len(b) < 0x1E {
-		return false
-	}
-
-	if !b.Advance(0x1E) {
-		return false
-	}
-	if bytes.HasPrefix(b, sig) {
-		return true
-	}
+// zipIterator iterates over a zip file returning the name of the zip entries
+// in that file.
+type zipIterator struct {
+	b scan.Bytes
+}
 
-	if msoCheck {
-		skipFiles := [][]byte{
-			[]byte("[Content_Types].xml"),
-			[]byte("_rels/.rels"),
-			[]byte("docProps"),
-			[]byte("customXml"),
-			[]byte("[trash]"),
-		}
+type zipEntries []struct {
+	name []byte
+	dir  bool // dir means checking just the prefix of the entry, not the whole path
+}
 
-		hasSkipFile := false
-		for _, sf := range skipFiles {
-			if bytes.HasPrefix(b, sf) {
-				hasSkipFile = true
-				break
-			}
+func (z zipEntries) match(file []byte) bool {
+	for i := range z {
+		if z[i].dir && bytes.HasPrefix(file, z[i].name) {
+			return true
 		}
-		if !hasSkipFile {
-			return false
+		if bytes.Equal(file, z[i].name) {
+			return true
 		}
 	}
+	return false
+}
 
-	searchOffset := binary.LittleEndian.Uint32(raw[18:]) + 49
-	if !b.Advance(int(searchOffset)) {
-		return false
+func zipHas(raw scan.Bytes, searchFor zipEntries, stopAfter int) bool {
+	iter := zipIterator{raw}
+	for i := 0; i < stopAfter; i++ {
+		f := iter.next()
+		if len(f) == 0 {
+			break
+		}
+		if searchFor.match(f) {
+			return true
+		}
 	}
 
-	nextHeader := bytes.Index(raw[searchOffset:], pk)
-	if !b.Advance(nextHeader) {
-		return false
-	}
-	// This is the second entry in zip.
-	if lookAt > 1 && bytes.HasPrefix(b, sig) {
-		return true
-	}
+	return false
+}
 
-	// Previously i was 4 at max, but #679 reported zip files where signatures
-	// occur later than 4. Because mimetype only looks at the file header, this
-	// for loop might as well be unbounded, ie: until the input bytes are all
-	// consumed. But users can call SetLimit(0) to make mimetype analyze whole
-	// files. So keep max 100 just in case. The reason I initially made it 4
-	// was because FILE(1) had this limit.
-	for i := 3; i < lookAt; i++ {
-		if !b.Advance(0x1A) {
-			return false
-		}
-		nextHeader = bytes.Index(b, pk)
-		if nextHeader == -1 {
-			return false
+// msoxml behaves like zipHas, but it puts restrictions on what the first zip
+// entry can be.
+func msoxml(raw scan.Bytes, searchFor zipEntries, stopAfter int) bool {
+	iter := zipIterator{raw}
+	for i := 0; i < stopAfter; i++ {
+		f := iter.next()
+		if len(f) == 0 {
+			break
 		}
-		if !b.Advance(nextHeader + 0x1E) {
-			return false
-		}
-		if bytes.HasPrefix(b, sig) {
+		if searchFor.match(f) {
 			return true
 		}
+		// If the first is not one of the next usually expected entries, 
+		// then abort this check.
+		if i == 0 {
+			if !bytes.Equal(f, []byte("[Content_Types].xml")) &&
+				!bytes.Equal(f, []byte("_rels/.rels")) &&
+				!bytes.Equal(f, []byte("docProps")) &&
+				!bytes.Equal(f, []byte("customXml")) &&
+				!bytes.Equal(f, []byte("[trash]")) {
+				return false
+			}
+		}
 	}
+
 	return false
 }
 
-// APK matches an Android Package Archive.
-// The source of signatures is https://github.com/file/file/blob/1778642b8ba3d947a779a36fcd81f8e807220a19/magic/Magdir/archive#L1820-L1887
-func APK(raw []byte, _ uint32) bool {
-	apkSignatures := [][]byte{
-		[]byte("AndroidManifest.xml"),
-		[]byte("META-INF/com/android/build/gradle/app-metadata.properties"),
-		[]byte("classes.dex"),
-		[]byte("resources.arsc"),
-		[]byte("res/drawable"),
+// next extracts the name of the next zip entry.
+func (i *zipIterator) next() []byte {
+	pk := []byte("PK\003\004")
+
+	if n := bytes.Index(i.b, pk); n != -1 {
+		i.b.Advance(n)
 	}
-	for _, sig := range apkSignatures {
-		if zipContains(raw, sig, false, 100) {
-			return true
-		}
+	if !i.b.Advance(0x1A) {
+		return nil
+	}
+	l, ok := i.b.Uint16()
+	if !ok {
+		return nil
+	}
+	if !i.b.Advance(0x02) {
+		return nil
+	}
+	if len(i.b) < int(l) {
+		return nil
 	}
+	return i.b[:l]
+}
 
-	return false
+// APK matches an Android Package Archive.
+// The source of signatures is https://github.com/file/file/blob/1778642b8ba3d947a779a36fcd81f8e807220a19/magic/Magdir/archive#L1820-L1887
+func APK(raw []byte, _ uint32) bool {
+	return zipHas(raw, zipEntries{{
+		name: []byte("AndroidManifest.xml"),
+	}, {
+		name: []byte("META-INF/com/android/build/gradle/app-metadata.properties"),
+	}, {
+		name: []byte("classes.dex"),
+	}, {
+		name: []byte("resources.arsc"),
+	}, {
+		name: []byte("res/drawable"),
+	}}, 100)
 }
diff --git a/internal/scan/bytes.go b/internal/scan/bytes.go
index c73063c..d661300 100644
--- a/internal/scan/bytes.go
+++ b/internal/scan/bytes.go
@@ -53,6 +53,16 @@ func (b *Bytes) Pop() byte {
 	return 0
 }
 
+// PopN pops n bytes from b or nil if b is empty.
+func (b *Bytes) PopN(n int) []byte {
+	if len(*b) >= n {
+		ret := (*b)[:n]
+		*b = (*b)[n:]
+		return ret
+	}
+	return nil
+}
+
 // PopUntil will advance b until, but not including, the first occurence of stopAt
 // character. If no occurence is found, then it will advance until the end of b.
 // The returned Bytes is a slice of all the bytes that we're advanced over.


Test patch:
diff --git a/internal/magic/zip_test.go b/internal/magic/zip_test.go
index 802d322..f7fe3f7 100644
--- a/internal/magic/zip_test.go
+++ b/internal/magic/zip_test.go
@@ -65,6 +65,7 @@ func TestZeroZip(t *testing.T) {
 	}, {
 		name:  "customXml, and other files, but word/ is the 7th file", // we only check until 6th file
 		files: []string{"customXml", "1", "2", "3", "4", "5", "word/"},
+		docx:  true,
 	}, {
 		name:  "customXml, word/ xl/ pptx/ after 5 files",
 		files: []string{"1", "2", "3", "4", "5", "customXml", "word/", "xl/", "ppt/"},
@@ -123,7 +124,7 @@ func TestZeroZip(t *testing.T) {
 		pptx: true,
 	}}
 
-	for _, tc := range tcases {
+	for i, tc := range tcases {
 		t.Run(tc.name, func(t *testing.T) {
 			buf, err := createZip(tc.files)
 			if err != nil {
@@ -136,8 +137,10 @@ func TestZeroZip(t *testing.T) {
 			jar := Jar(buf.Bytes(), 0)
 
 			if tc.docx != docx || tc.xlsx != xlsx || tc.pptx != pptx || tc.jar != jar {
-				t.Errorf(`expected %t %t %t %t;
-                got %t %t %t %t`, tc.docx, tc.xlsx, tc.pptx, tc.jar, docx, xlsx, pptx, jar)
+				t.Errorf(`
+         docx	xlsx	pptx	jar %d
+expected %t	%t	%t	%t;
+     got %t	%t	%t	%t`, i, tc.docx, tc.xlsx, tc.pptx, tc.jar, docx, xlsx, pptx, jar)
 			}
 
 			// #400 - xlsx, docx, pptx put as is (compression lvl 0) inside a zip
@@ -153,8 +156,10 @@ func TestZeroZip(t *testing.T) {
 			jar = Jar(uncompressedZip.Bytes(), 0)
 
 			if docx || xlsx || pptx || jar {
-				t.Errorf(`uncompressedZip: expected false, false, false;
-                got %t %t %t %t`, docx, xlsx, pptx, jar)
+				t.Errorf(`
+uncompressedZip: docx	xlsx	pptx	jar %d
+        expected false	false	false	false
+             got %t	%t	%t	%t`, i, docx, xlsx, pptx, jar)
 			}
 		})
 	}
diff --git a/internal/scan/bytes_test.go b/internal/scan/bytes_test.go
index e7c0f70..d29a90a 100644
--- a/internal/scan/bytes_test.go
+++ b/internal/scan/bytes_test.go
@@ -56,6 +56,47 @@ func TestPop(t *testing.T) {
 		})
 	}
 }
+
+func TestPopN(t *testing.T) {
+	tcases := []struct {
+		name   string
+		in     string
+		n      int
+		popped string
+		left   string
+	}{{
+		"empty", "", 0, "", "",
+	}, {
+		"1,0", "1", 0, "", "1",
+	}, {
+		"12,0", "12", 0, "", "12",
+	}, {
+		"1,1", "1", 1, "1", "",
+	}, {
+		"12,1", "12", 1, "1", "2",
+	}, {
+		"123,1", "123", 1, "1", "23",
+	}, {
+		"123,2", "123", 2, "12", "3",
+	}, {
+		"123,3", "123", 3, "123", "",
+	}, {
+		"123,4", "123", 4, "", "123",
+	}}
+
+	for _, tc := range tcases {
+		t.Run(tc.name, func(t *testing.T) {
+			b := Bytes(tc.in)
+			popped := b.PopN(tc.n)
+			if string(b) != tc.left {
+				t.Errorf("left: got: %s, want: %s", string(b), tc.left)
+			}
+			if string(popped) != tc.popped {
+				t.Errorf("popped: got: %s, want: %s", string(popped), tc.popped)
+			}
+		})
+	}
+}
 func TestTrim(t *testing.T) {
 	tcases := []struct {
 		name  string
diff --git a/mimetype_test.go b/mimetype_test.go
index 78371c0..a320ed9 100644
--- a/mimetype_test.go
+++ b/mimetype_test.go
@@ -13,6 +13,11 @@ import (
 	"testing"
 )
 
+// testcases are used for correctness and benchmarks.
+// testcase data is provided as a string for convenience,
+// but that makes benchmarks allocate more. It can be observed
+// for testcases that call fromDisk. Those allocate more because
+// they read a lot from disk.
 type testcase struct {
 	name         string
 	data         string
@@ -133,7 +138,8 @@ a,"b`,
 	{"ics dos", "BEGIN:VCALENDAR\r\n00", "text/calendar", none},
 	{"txt iso88591", "\x0a\xe6\xf8\xe6\xf8\xe5\xe6\xf8\xe5\xe5\x0a", "text/plain; charset=iso-8859-1", none},
 	{"jar", fromDisk("jar.jar"), "application/java-archive", all},
-	{"executable jar", "PK\x03\x04" + offset(0x1A, string([]byte{0xFE, 0xCA})), "application/java-archive", none},
+	{"jar executable", "PK\x03\x04" + offset(0x1A, string([]byte{0xFE, 0xCA})), "application/java-archive", none},
+	{"jar in zip #639", fromDisk("jar_in_zip.zip"), "application/zip", none},
 	{"jp2", "\x00\x00\x00\x0c\x6a\x50\x20\x20\x0d\x0a\x87\x0a\x00\x00\x00\x14\x66\x74\x79\x70\x6a\x70\x32\x20", "image/jp2", one},
 	{"jpf", "\x00\x00\x00\x0c\x6a\x50\x20\x20\x0d\x0a\x87\x0a\x00\x00\x00\x1c\x66\x74\x79\x70\x6a\x70\x78\x20", "image/jpx", one},
 	{"jpg", "\xFF\xD8\xFF", "image/jpeg", one},