[MM-13158] Initial attempt at extracting SVG dimensions (#10332)

* initial attempt at extracting svg dimensions

* rafactor SVG dimensions extraction

* pass SVG parsing errors to calling context

* tweaks to svg parsing placement

- also stopped trying to pre/post process SVG’s as images

* add svg parsing tests

* updates for PR change requests

* code review updates

* correct a conditional typo

app/file.go

@@ -673,6 +673,20 @@ func (t *uploadFileTask) runPlugins() *model.AppError {
 }
 
 func (t *uploadFileTask) preprocessImage() *model.AppError {
+	// If SVG, attempt to extract dimensions and then return
+	if t.fileinfo.MimeType == "image/svg+xml" {
+		svgInfo, err := parseSVG(t.newReader())
+		if err != nil {
+			mlog.Error("Failed to parse SVG", mlog.Err(err))
+		}
+		if svgInfo.Width > 0 && svgInfo.Height > 0 {
+			t.fileinfo.Width = svgInfo.Width
+			t.fileinfo.Height = svgInfo.Height
+		}
+		t.fileinfo.HasPreviewImage = false
+		return nil
+	}
+
 	// If we fail to decode, return "as is".
 	config, _, err := image.DecodeConfig(t.newReader())
 	if err != nil {
@@ -723,6 +737,11 @@ func (t *uploadFileTask) preprocessImage() *model.AppError {
 }
 
 func (t *uploadFileTask) postprocessImage() {
+	// don't try to process SVG files
+	if t.fileinfo.MimeType == "image/svg+xml" {
+		return
+	}
+
 	decoded, typ := t.decoded, t.imageType
 	if decoded == nil {
 		var err error

app/svg.go

@@ -0,0 +1,56 @@
+// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package app
+
+import (
+	"encoding/xml"
+	"io"
+	"regexp"
+	"strconv"
+
+	"github.com/pkg/errors"
+)
+
+type SVGInfo struct {
+	Width  int
+	Height int
+}
+
+func parseSVG(svgReader io.Reader) (SVGInfo, error) {
+	var parsedSVG struct {
+		Width   string `xml:"width,attr,omitempty"`
+		Height  string `xml:"height,attr,omitempty"`
+		ViewBox string `xml:"viewBox,attr,omitempty"`
+	}
+	svgInfo := SVGInfo{
+		Width:  0,
+		Height: 0,
+	}
+	viewBoxPattern := regexp.MustCompile("^([0-9]+)[, ]+([0-9]+)[, ]+([0-9]+)[, ]+([0-9]+)$")
+	dimensionPattern := regexp.MustCompile("(?i)^([0-9]+)(?:px)?$")
+
+	// decode provided SVG
+	if err := xml.NewDecoder(svgReader).Decode(&parsedSVG); err != nil {
+		return svgInfo, err
+	}
+
+	// prefer viewbox for SVG dimensions over width/height
+	if viewBoxMatches := viewBoxPattern.FindStringSubmatch(parsedSVG.ViewBox); len(viewBoxMatches) == 5 {
+		svgInfo.Width, _ = strconv.Atoi(viewBoxMatches[3])
+		svgInfo.Height, _ = strconv.Atoi(viewBoxMatches[4])
+	} else if len(parsedSVG.Width) > 0 && len(parsedSVG.Height) > 0 {
+		widthMatches := dimensionPattern.FindStringSubmatch(parsedSVG.Width)
+		heightMatches := dimensionPattern.FindStringSubmatch(parsedSVG.Height)
+		if len(widthMatches) == 2 && len(heightMatches) == 2 {
+			svgInfo.Width, _ = strconv.Atoi(widthMatches[1])
+			svgInfo.Height, _ = strconv.Atoi(heightMatches[1])
+		}
+	}
+
+	// if width and/or height are still zero, create new error
+	if svgInfo.Width == 0 || svgInfo.Height == 0 {
+		return svgInfo, errors.New("unable to extract SVG dimensions")
+	}
+	return svgInfo, nil
+}

app/svg_test.go

@@ -0,0 +1,75 @@
+// Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package app
+
+import (
+	"fmt"
+	"io"
+	"strings"
+	"testing"
+)
+
+func generateSVGData(width int, height int, useViewBox bool, useDimensions bool, useAlternateFormat bool) io.Reader {
+	var (
+		viewBoxAttribute = ""
+		widthAttribute   = ""
+		heightAttribute  = ""
+	)
+	if useViewBox == true {
+		separator := " "
+		if useAlternateFormat == true {
+			separator = ", "
+		}
+		viewBoxAttribute = fmt.Sprintf(` viewBox="0%s0%s%d%s%d"`, separator, separator, width, separator, height)
+	}
+	if useDimensions == true && width > 0 && height > 0 {
+		units := ""
+		if useAlternateFormat == true {
+			units = "px"
+		}
+		widthAttribute = fmt.Sprintf(` width="%d%s"`, width, units)
+		heightAttribute = fmt.Sprintf(` height="%d%s"`, height, units)
+	}
+	svgString := fmt.Sprintf(`<svg%s%s%s></svg>`, widthAttribute, heightAttribute, viewBoxAttribute)
+	return strings.NewReader(svgString)
+}
+
+func TestParseValidSVGData(t *testing.T) {
+	var width, height int = 300, 300
+	validSVGs := []io.Reader{
+		generateSVGData(width, height, true, true, false),  // properly formed viewBox, width & height
+		generateSVGData(width, height, true, true, true),   // properly formed viewBox, width & height; alternate format
+		generateSVGData(width, height, false, true, false), // missing viewBox, properly formed width & height
+		generateSVGData(width, height, false, true, true),  // missing viewBox, properly formed width & height; alternate format
+	}
+	for index, svg := range validSVGs {
+		svgInfo, err := parseSVG(svg)
+		if err != nil {
+			t.Errorf("Should be able to parse SVG attributes at index %d, but was not able to: err = %v", index, err)
+		} else {
+			if svgInfo.Width != width {
+				t.Errorf("Expecting a width of %d for SVG at index %d, but it was %d instead.", width, index, svgInfo.Width)
+			}
+
+			if svgInfo.Height != height {
+				t.Errorf("Expecting a height of %d for SVG at index %d, but it was %d instead.", height, index, svgInfo.Height)
+			}
+		}
+	}
+}
+
+func TestParseInvalidSVGData(t *testing.T) {
+	var width, height int = 300, 300
+	invalidSVGs := []io.Reader{
+		generateSVGData(width, height, false, false, false), // missing viewBox, width & height
+		generateSVGData(width, 0, false, true, false),       // missing viewBox, malformed width & height
+		generateSVGData(300, 0, false, true, false),         // missing viewBox, malformed height, properly formed width
+	}
+	for index, svg := range invalidSVGs {
+		_, err := parseSVG(svg)
+		if err == nil {
+			t.Errorf("Should not be able to parse SVG attributes at index %d, but was definitely able to!", index)
+		}
+	}
+}