Merge pull request #518 from alixander/fix-query-param-escape

fix icon query param escaping

ci/release/changelogs/next.md

@@ -16,5 +16,6 @@
 - Fixed an issue where text could overflow in sql_table shapes. [#458](https://github.com/terrastruct/d2/pull/458)
 - Fixed an issue with elk layouts accounting for edge labels as if they were placed on the side of the edge. [#483](https://github.com/terrastruct/d2/pull/483)
 - Fixed an issue where dagre layouts may not have enough spacing for all edge labels. [#484](https://github.com/terrastruct/d2/pull/484)
+- Icons with query parameters are now being escaped to valid SVG XML. [#438](https://github.com/terrastruct/d2/issues/438)
 - Fixed connections being clipped if they were at the very top or left edges of the diagram. [#493](https://github.com/terrastruct/d2/pull/493)
 - Fixed edge case where style being defined in same scope as sql_table caused compiler to skip compiling sql_table. [#506](https://github.com/terrastruct/d2/issues/506)

d2renderers/d2svg/d2svg.go

@@ -8,6 +8,7 @@ import (
 	"errors"
 	"fmt"
 	"hash/fnv"
+	"html"
 	"io"
 	"sort"
 	"strings"
@@ -656,7 +657,7 @@ func drawShape(writer io.Writer, targetShape d2target.Shape, sketchRunner *d2ske
 
 	case d2target.ShapeImage:
 		fmt.Fprintf(writer, `<image href="%s" x="%d" y="%d" width="%d" height="%d" style="%s" />`,
-			targetShape.Icon.String(),
+			html.EscapeString(targetShape.Icon.String()),
 			targetShape.Pos.X, targetShape.Pos.Y, targetShape.Width, targetShape.Height, style)
 
 	// TODO should standardize "" to rectangle
@@ -716,7 +717,7 @@ func drawShape(writer io.Writer, targetShape d2target.Shape, sketchRunner *d2ske
 		tl := iconPosition.GetPointOnBox(box, label.PADDING, float64(iconSize), float64(iconSize))
 
 		fmt.Fprintf(writer, `<image href="%s" x="%f" y="%f" width="%d" height="%d" />`,
-			targetShape.Icon.String(),
+			html.EscapeString(targetShape.Icon.String()),
 			tl.X,
 			tl.Y,
 			iconSize,

e2etests/e2e_test.go

@@ -139,6 +139,7 @@ func run(t *testing.T, tc testCase) {
 			defer os.Remove(pathGotSVG)
 		}
 
+		// Check that it's valid SVG
 		var xmlParsed interface{}
 		err = xml.Unmarshal(svgBytes, &xmlParsed)
 		assert.Success(t, err)

e2etests/regression_test.go

@@ -171,6 +171,13 @@ build_workflow: lambda-build.yaml {
 	S3 <-> Terraform: Pulls zip to deploy
 	Terraform -> AWS: Changes the live lambdas
 }
+`,
+		},
+		{
+			name: "query_param_escape",
+			script: `my network: {
+  icon: https://icons.terrastruct.com/infra/019-network.svg?fuga=1&hoge
+}
 `,
 		},
 	}

e2etests/testdata/regression/query_param_escape/dagre/board.exp.json

@@ -0,0 +1,58 @@
+{
+  "name": "",
+  "fontFamily": "SourceSansPro",
+  "shapes": [
+    {
+      "id": "my network",
+      "type": "",
+      "pos": {
+        "x": 0,
+        "y": 0
+      },
+      "width": 190,
+      "height": 152,
+      "opacity": 1,
+      "strokeDash": 0,
+      "strokeWidth": 2,
+      "borderRadius": 0,
+      "fill": "#F7F8FE",
+      "stroke": "#0D32B2",
+      "shadow": false,
+      "3d": false,
+      "multiple": false,
+      "tooltip": "",
+      "link": "",
+      "icon": {
+        "Scheme": "https",
+        "Opaque": "",
+        "User": null,
+        "Host": "icons.terrastruct.com",
+        "Path": "/infra/019-network.svg",
+        "RawPath": "",
+        "ForceQuery": false,
+        "RawQuery": "fuga=1&hoge",
+        "Fragment": "",
+        "RawFragment": ""
+      },
+      "iconPosition": "INSIDE_MIDDLE_CENTER",
+      "blend": false,
+      "fields": null,
+      "methods": null,
+      "columns": null,
+      "label": "my network",
+      "fontSize": 16,
+      "fontFamily": "DEFAULT",
+      "language": "",
+      "color": "#0A0F25",
+      "italic": false,
+      "bold": true,
+      "underline": false,
+      "labelWidth": 90,
+      "labelHeight": 26,
+      "labelPosition": "INSIDE_TOP_CENTER",
+      "zIndex": 0,
+      "level": 1
+    }
+  ],
+  "connections": []
+}

e2etests/testdata/regression/query_param_escape/elk/board.exp.json

@@ -0,0 +1,58 @@
+{
+  "name": "",
+  "fontFamily": "SourceSansPro",
+  "shapes": [
+    {
+      "id": "my network",
+      "type": "",
+      "pos": {
+        "x": 12,
+        "y": 12
+      },
+      "width": 190,
+      "height": 152,
+      "opacity": 1,
+      "strokeDash": 0,
+      "strokeWidth": 2,
+      "borderRadius": 0,
+      "fill": "#F7F8FE",
+      "stroke": "#0D32B2",
+      "shadow": false,
+      "3d": false,
+      "multiple": false,
+      "tooltip": "",
+      "link": "",
+      "icon": {
+        "Scheme": "https",
+        "Opaque": "",
+        "User": null,
+        "Host": "icons.terrastruct.com",
+        "Path": "/infra/019-network.svg",
+        "RawPath": "",
+        "ForceQuery": false,
+        "RawQuery": "fuga=1&hoge",
+        "Fragment": "",
+        "RawFragment": ""
+      },
+      "iconPosition": "INSIDE_MIDDLE_CENTER",
+      "blend": false,
+      "fields": null,
+      "methods": null,
+      "columns": null,
+      "label": "my network",
+      "fontSize": 16,
+      "fontFamily": "DEFAULT",
+      "language": "",
+      "color": "#0A0F25",
+      "italic": false,
+      "bold": true,
+      "underline": false,
+      "labelWidth": 90,
+      "labelHeight": 26,
+      "labelPosition": "INSIDE_TOP_CENTER",
+      "zIndex": 0,
+      "level": 1
+    }
+  ],
+  "connections": []
+}