Refactor backend SVG package and add tests (#26335)

Introduce a well-tested `svg.Normalize` function.
Make `RenderHTML` faster and more stable.

modules/html/html.go

@@ -6,28 +6,20 @@ package html
 // ParseSizeAndClass get size and class from string with default values
 // If present, "others" expects the new size first and then the classes to use
 func ParseSizeAndClass(defaultSize int, defaultClass string, others ...any) (int, string) {
-	if len(others) == 0 {
-		return defaultSize, defaultClass
-	}
-
 	size := defaultSize
-	_size, ok := others[0].(int)
+	if len(others) >= 1 {
-	if ok && _size != 0 {
+		if v, ok := others[0].(int); ok && v != 0 {
-		size = _size
+			size = v
-	}
+		}
-
+	}
-	if len(others) == 1 {
+	class := defaultClass
-		return size, defaultClass
+	if len(others) >= 2 {
-	}
+		if v, ok := others[1].(string); ok && v != "" {
-
+			if class != "" {
-	class := defaultClass
+				class += " "
-	if _class, ok := others[1].(string); ok && _class != "" {
+			}
-		if defaultClass == "" {
+			class += v
-			class = _class
-		} else {
-			class = defaultClass + " " + _class
 		}
 	}
-
 	return size, class
 }

modules/svg/processor.go

@@ -0,0 +1,59 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package svg
+
+import (
+	"bytes"
+	"fmt"
+	"regexp"
+	"sync"
+)
+
+type normalizeVarsStruct struct {
+	reXMLDoc,
+	reComment,
+	reAttrXMLNs,
+	reAttrSize,
+	reAttrClassPrefix *regexp.Regexp
+}
+
+var (
+	normalizeVars     *normalizeVarsStruct
+	normalizeVarsOnce sync.Once
+)
+
+// Normalize normalizes the SVG content: set default width/height, remove unnecessary tags/attributes
+// It's designed to work with valid SVG content. For invalid SVG content, the returned content is not guaranteed.
+func Normalize(data []byte, size int) []byte {
+	normalizeVarsOnce.Do(func() {
+		normalizeVars = &normalizeVarsStruct{
+			reXMLDoc:  regexp.MustCompile(`(?s)<\?xml.*?>`),
+			reComment: regexp.MustCompile(`(?s)<!--.*?-->`),
+
+			reAttrXMLNs:       regexp.MustCompile(`(?s)\s+xmlns\s*=\s*"[^"]*"`),
+			reAttrSize:        regexp.MustCompile(`(?s)\s+(width|height)\s*=\s*"[^"]+"`),
+			reAttrClassPrefix: regexp.MustCompile(`(?s)\s+class\s*=\s*"`),
+		}
+	})
+	data = normalizeVars.reXMLDoc.ReplaceAll(data, nil)
+	data = normalizeVars.reComment.ReplaceAll(data, nil)
+
+	data = bytes.TrimSpace(data)
+	svgTag, svgRemaining, ok := bytes.Cut(data, []byte(">"))
+	if !ok || !bytes.HasPrefix(svgTag, []byte(`<svg`)) {
+		return data
+	}
+	normalized := bytes.Clone(svgTag)
+	normalized = normalizeVars.reAttrXMLNs.ReplaceAll(normalized, nil)
+	normalized = normalizeVars.reAttrSize.ReplaceAll(normalized, nil)
+	normalized = normalizeVars.reAttrClassPrefix.ReplaceAll(normalized, []byte(` class="`))
+	normalized = bytes.TrimSpace(normalized)
+	normalized = fmt.Appendf(normalized, ` width="%d" height="%d"`, size, size)
+	if !bytes.Contains(normalized, []byte(` class="`)) {
+		normalized = append(normalized, ` class="svg"`...)
+	}
+	normalized = append(normalized, '>')
+	normalized = append(normalized, svgRemaining...)
+	return normalized
+}

modules/svg/processor_test.go

@@ -0,0 +1,29 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package svg
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestNormalize(t *testing.T) {
+	res := Normalize([]byte("foo"), 1)
+	assert.Equal(t, "foo", string(res))
+
+	res = Normalize([]byte(`<?xml version="1.0"?>
+<!--
+comment
+-->
+<svg xmlns = "...">content</svg>`), 1)
+	assert.Equal(t, `<svg width="1" height="1" class="svg">content</svg>`, string(res))
+
+	res = Normalize([]byte(`<svg
+width="100"
+class="svg-icon"
+>content</svg>`), 16)
+
+	assert.Equal(t, `<svg class="svg-icon" width="16" height="16">content</svg>`, string(res))
+}

modules/svg/svg.go

@@ -7,42 +7,35 @@ import (
 	"fmt"
 	"html/template"
 	"path"
-	"regexp"
 	"strings"
 
-	"code.gitea.io/gitea/modules/html"
+	gitea_html "code.gitea.io/gitea/modules/html"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/public"
 )
 
-var (
+var svgIcons map[string]string
-	// SVGs contains discovered SVGs
-	SVGs = map[string]string{}
-
-	widthRe  = regexp.MustCompile(`width="[0-9]+?"`)
-	heightRe = regexp.MustCompile(`height="[0-9]+?"`)
-)
 
 const defaultSize = 16
 
-// Init discovers SVGs and populates the `SVGs` variable
+// Init discovers SVG icons and populates the `svgIcons` variable
 func Init() error {
-	files, err := public.AssetFS().ListFiles("assets/img/svg")
+	const svgAssetsPath = "assets/img/svg"
+	files, err := public.AssetFS().ListFiles(svgAssetsPath)
 	if err != nil {
 		return err
 	}
 
-	// Remove `xmlns` because inline SVG does not need it
+	svgIcons = make(map[string]string, len(files))
-	reXmlns := regexp.MustCompile(`(<svg\b[^>]*?)\s+xmlns="[^"]*"`)
 	for _, file := range files {
 		if path.Ext(file) != ".svg" {
 			continue
 		}
-		bs, err := public.AssetFS().ReadFile("assets/img/svg", file)
+		bs, err := public.AssetFS().ReadFile(svgAssetsPath, file)
 		if err != nil {
 			log.Error("Failed to read SVG file %s: %v", file, err)
 		} else {
-			SVGs[file[:len(file)-4]] = reXmlns.ReplaceAllString(string(bs), "$1")
+			svgIcons[file[:len(file)-4]] = string(Normalize(bs, defaultSize))
 		}
 	}
 	return nil
@@ -50,12 +43,12 @@ func Init() error {
 
 // RenderHTML renders icons - arguments icon name (string), size (int), class (string)
 func RenderHTML(icon string, others ...any) template.HTML {
-	size, class := html.ParseSizeAndClass(defaultSize, "", others...)
+	size, class := gitea_html.ParseSizeAndClass(defaultSize, "", others...)
-
+	if svgStr, ok := svgIcons[icon]; ok {
-	if svgStr, ok := SVGs[icon]; ok {
+		// the code is somewhat hacky, but it just works, because the SVG contents are all normalized
 		if size != defaultSize {
-			svgStr = widthRe.ReplaceAllString(svgStr, fmt.Sprintf(`width="%d"`, size))
+			svgStr = strings.Replace(svgStr, fmt.Sprintf(`width="%d"`, defaultSize), fmt.Sprintf(`width="%d"`, size), 1)
-			svgStr = heightRe.ReplaceAllString(svgStr, fmt.Sprintf(`height="%d"`, size))
+			svgStr = strings.Replace(svgStr, fmt.Sprintf(`height="%d"`, defaultSize), fmt.Sprintf(`height="%d"`, size), 1)
 		}
 		if class != "" {
 			svgStr = strings.Replace(svgStr, `class="`, fmt.Sprintf(`class="%s `, class), 1)