svg+html: better handling for comments (#689)

gabriel-vasile · web-flow · commit 1636be6602f6 · 2025-07-21T18:03:38.000+09:00
In #193 HTML documents starting with a comment were not detected anymore. This is against mime sniffing standard https://mimesniff.spec.whatwg.org/, but at the time it seemed best solution. file(1) also detects comments as HTML. This commit makes SVG detection skip optional comments and HTML back to detecting those as valid documents.
diff --git a/internal/charset/charset.go b/internal/charset/charset.go
@@ -209,15 +209,8 @@ func fromHTML(s scan.Bytes) string {
 	meta := []byte("<META")
 	lmeta := len(meta)
 	for {
-		if len(s) == 0 {
-			return ""
-		}
-		if bytes.HasPrefix(s, []byte("<!--")) {
-			// Offset by two (<!) because the starting and ending -- can be the same.j
-			s.Advance(2)
-			if i := bytes.Index(s, []byte("-->")); i != -1 {
-				s.Advance(i)
-			}
+		if markup.SkipAComment(&s) {
+			continue
 		}
 		if len(s) <= lmeta {
 			return ""
diff --git a/internal/magic/text.go b/internal/magic/text.go
@@ -29,6 +29,7 @@ var (
 		[]byte("<BODY"),
 		[]byte("<BR"),
 		[]byte("<P"),
+		[]byte("<!--"),
 	)
 	// XML matches an Extensible Markup Language file.
 	XML = markup([]byte("<?XML"))
@@ -236,13 +237,16 @@ func Svg(raw []byte, limit uint32) bool {
 // svgWithoutXMLDeclaration matches a SVG image that does not have an XML header.
 // Example:
 //
+//	<!-- xml comment ignored -->
 //	<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
 //	    <rect fill="#fff" stroke="#000" x="-70" y="-70" width="390" height="390"/>
 //	</svg>
 func svgWithoutXMLDeclaration(s scan.Bytes) bool {
 	for scan.ByteIsWS(s.Peek()) {
 		s.Advance(1)
 	}
+	for mkup.SkipAComment(&s) {
+	}
 	if !bytes.HasPrefix(s, []byte("<svg")) {
 		return false
 	}
diff --git a/internal/markup/markup.go b/internal/markup/markup.go
@@ -3,6 +3,8 @@
 package markup
 
 import (
+	"bytes"
+
 	"github.com/gabriel-vasile/mimetype/internal/scan"
 )
 
@@ -88,3 +90,14 @@ func getAValue(s *scan.Bytes) (_ []byte, hasMore bool) {
 		}
 	}
 }
+
+func SkipAComment(s *scan.Bytes) (skipped bool) {
+	if bytes.HasPrefix(*s, []byte("<!--")) {
+		// Offset by 2 len(<!) because the starting and ending -- can be the same.
+		if i := bytes.Index((*s)[2:], []byte("-->")); i != -1 {
+			s.Advance(i + 2 + 3) // 2 comes from len(<!) and 3 comes from len(-->).
+			return true
+		}
+	}
+	return false
+}
diff --git a/internal/markup/markup_test.go b/internal/markup/markup_test.go
@@ -229,3 +229,33 @@ func TestGetAllAttributes(t *testing.T) {
 		})
 	}
 }
+
+func TestSkipAComment(t *testing.T) {
+	tcases := []struct {
+		in      string
+		out     string
+		skipped bool
+	}{{
+		"", "", false,
+	}, {
+		"abc", "abc", false,
+	}, {
+		"<!--", "<!--", false, // not ending comment
+	}, {
+		"<!-- abc -->", "", true, // regular comment
+	}, {
+		"<!-->", "", true, // the beginning and ending -- are the same chars
+	}}
+	for _, tc := range tcases {
+		t.Run(tc.in, func(t *testing.T) {
+			s := scan.Bytes(tc.in)
+			skipped := SkipAComment(&s)
+			if tc.skipped != skipped {
+				t.Errorf("skipped got: %v, want: %v", skipped, tc.skipped)
+			}
+			if string(s) != tc.out {
+				t.Errorf("got: %v, want: %v", string(s), tc.out)
+			}
+		})
+	}
+}
diff --git a/mimetype_test.go b/mimetype_test.go
@@ -132,6 +132,12 @@ a,"b`,
 		"text/html; charset=iso-8859-1",
 		none,
 	},
+	{
+		"html with comment prefix",
+		`<!-- this comment should not affect --><html><head>`,
+		"text/html; charset=utf-8",
+		none,
+	},
 	{"ico 01", "\x00\x00\x01\x00", "image/x-icon", one},
 	{"ico 02", "\x00\x00\x02\x00", "image/x-icon", none},
 	{"ics", "BEGIN:VCALENDAR\n00", "text/calendar", one},
@@ -246,6 +252,12 @@ a,"b`,
 		"image/svg+xml",
 		all,
 	},
+	{
+		"svg with comment prefix",
+		`<!-- this comment should not affect --><svg xmlns="http://www.w3.org/2000/svg"`,
+		"image/svg+xml",
+		none,
+	},
 
 	{"swf", "CWS", "application/x-shockwave-flash", one},
 	{"tar", fromDisk("tar.tar"), "application/x-tar", all},
diff --git a/supported_mimes.md b/supported_mimes.md
@@ -147,8 +147,8 @@ Extension | MIME type | Aliases
 **.jxr** | image/jxr | image/vnd.ms-photo
 **.parquet** | application/vnd.apache.parquet | application/x-parquet
 **.txt** | text/plain | -
-**.html** | text/html | -
 **.svg** | image/svg+xml | -
+**.html** | text/html | -
 **.xml** | text/xml | application/xml
 **.rss** | application/rss+xml | text/rss
 **.atom** | application/atom+xml | -
diff --git a/tree.go b/tree.go
@@ -81,7 +81,7 @@ var (
 		alias("application/x-ogg")
 	oggAudio = newMIME("audio/ogg", ".oga", magic.OggAudio)
 	oggVideo = newMIME("video/ogg", ".ogv", magic.OggVideo)
-	text     = newMIME("text/plain", ".txt", magic.Text, html, svg, xml, php, js, lua, perl, python, json, ndJSON, rtf, srt, tcl, csv, tsv, vCard, iCalendar, warc, vtt)
+	text     = newMIME("text/plain", ".txt", magic.Text, svg, html, xml, php, js, lua, perl, python, json, ndJSON, rtf, srt, tcl, csv, tsv, vCard, iCalendar, warc, vtt)
 	xml      = newMIME("text/xml", ".xml", magic.XML, rss, atom, x3d, kml, xliff, collada, gml, gpx, tcx, amf, threemf, xfdf, owl2).
 			alias("application/xml")
 	json    = newMIME("application/json", ".json", magic.JSON, geoJSON, har, gltf)

Original file line number	Diff line number	Diff line change
`@@ -3,6 +3,8 @@`
`3`	`3`	`package markup`
`4`	`4`
`5`	`5`	`import (`
	`6`	`+ "bytes"`
	`7`	`+`
`6`	`8`	`"github.com/gabriel-vasile/mimetype/internal/scan"`
`7`	`9`	`)`
`8`	`10`
`@@ -88,3 +90,14 @@ func getAValue(s *scan.Bytes) (_ []byte, hasMore bool) {`
`88`	`90`	`}`
`89`	`91`	`}`
`90`	`92`	`}`
	`93`	`+`
	`94`	`+func SkipAComment(s *scan.Bytes) (skipped bool) {`
	`95`	`+ if bytes.HasPrefix(*s, []byte("<!--")) {`
	`96`	`+ // Offset by 2 len(<!) because the starting and ending -- can be the same.`
	`97`	`+ if i := bytes.Index((*s)[2:], []byte("-->")); i != -1 {`
	`98`	`+ s.Advance(i + 2 + 3) // 2 comes from len(<!) and 3 comes from len(-->).`
	`99`	`+ return true`
	`100`	`+ }`
	`101`	`+ }`
	`102`	`+ return false`
	`103`	`+}`