internal/graph: Escape tag labels correctly for dot

internal/graph/dotgraph.go

@@ -19,6 +19,7 @@ import (
 	"io"
 	"math"
 	"path/filepath"
+	"strconv"
 	"strings"
 
 	"github.com/google/pprof/internal/measurement"
@@ -483,9 +484,29 @@ func escapeAllForDot(in []string) []string {
 	return out
 }
 
-// escapeForDot escapes double quotes and backslashes, and replaces Graphviz's
-// "center" character (\n) with a left-justified character.
-// See https://graphviz.org/doc/info/attrs.html#k:escString for more info.
+// escapeForDot escapes str so it displays as a single line. It escapes double
+// quotes and backslashes, and replaces any non-printable characters with their
+// Go escaped equivalent. See:
+// https://graphviz.org/docs/attr-types/escString/
 func escapeForDot(str string) string {
-	return strings.ReplaceAll(strings.ReplaceAll(strings.ReplaceAll(str, `\`, `\\`), `"`, `\"`), "\n", `\l`)
+	var out strings.Builder
+	for _, rune := range str {
+		switch {
+		case rune == '\\':
+			out.WriteString(`\\`)
+		case rune == '"':
+			out.WriteString(`\"`)
+		case strconv.IsPrint(rune):
+			out.WriteRune(rune)
+		default:
+			// QuoteRune wraps the result in ''.
+			quoted := strconv.QuoteRune(rune)
+			quoted = quoted[1 : len(quoted)-1]
+			if quoted[0] == '\\' {
+				out.WriteByte('\\')
+			}
+			out.WriteString(quoted)
+		}
+	}
+	return out.String()
 }

internal/graph/dotgraph_test.go

@@ -150,6 +150,30 @@ func TestComposeWithNamesThatNeedEscaping(t *testing.T) {
 	compareGraphs(t, buf.Bytes(), "compose7.dot")
 }
 
+func TestComposeWithTagsThatNeedEscaping(t *testing.T) {
+	g := baseGraph()
+	a, c := baseAttrsAndConfig()
+	// Tag names are normally escaped by joinLabels.
+	g.Nodes[0].LabelTags["a"] = &Tag{
+		Name: escapeForDot(`label"quote"` + "\nline2"),
+		Cum:  10,
+		Flat: 10,
+	}
+	g.Nodes[0].NumericTags[""] = TagMap{
+		"b": &Tag{
+			Name: escapeForDot(`numeric"quote"`),
+			Cum:  20,
+			Flat: 20,
+			Unit: "ms",
+		},
+	}
+
+	var buf bytes.Buffer
+	ComposeDot(&buf, g, a, c)
+
+	compareGraphs(t, buf.Bytes(), "compose8.dot")
+}
+
 func baseGraph() *Graph {
 	src := &Node{
 		Info:        NodeInfo{Name: "src"},
@@ -344,16 +368,16 @@ func TestEscapeForDot(t *testing.T) {
 		input []string
 		want  []string
 	}{
+		{
+			desc:  "empty does nothing",
+			input: []string{``},
+			want:  []string{``},
+		},
 		{
 			desc:  "with multiple doubles quotes",
 			input: []string{`label: "foo" and "bar"`},
 			want:  []string{`label: \"foo\" and \"bar\"`},
 		},
-		{
-			desc:  "with graphviz center line character",
-			input: []string{"label: foo \n bar"},
-			want:  []string{`label: foo \l bar`},
-		},
 		{
 			desc:  "with two backslashes",
 			input: []string{`label: \\`},
@@ -369,6 +393,11 @@ func TestEscapeForDot(t *testing.T) {
 			input: []string{`label1: "foo"`, `label2: "bar"`},
 			want:  []string{`label1: \"foo\"`, `label2: \"bar\"`},
 		},
+		{
+			desc:  "escape newlines and control chars",
+			input: []string{"line1\nline2", "null:\x00", "alert:\a", "backspace:\b"},
+			want:  []string{`line1\\nline2`, `null:\\x00`, `alert:\\a`, `backspace:\\b`},
+		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
 			if got := escapeAllForDot(tc.input); !reflect.DeepEqual(got, tc.want) {

internal/graph/graph.go

@@ -270,7 +270,7 @@ func (e *Edge) WeightValue() int64 {
 
 // Tag represent sample annotations
 type Tag struct {
-	Name          string
+	Name          string // Must be escaped for dot
 	Unit          string // Describe the value, "" for non-numeric tags
 	Value         int64
 	Flat, FlatDiv int64
@@ -519,6 +519,7 @@ func (g *Graph) TrimTree(kept NodePtrSet) {
 	g.RemoveRedundantEdges()
 }
 
+// joinLabels returns the labels as a string escaped for dot.
 func joinLabels(s *profile.Sample) string {
 	if len(s.Label) == 0 {
 		return ""
@@ -527,11 +528,12 @@ func joinLabels(s *profile.Sample) string {
 	var labels []string
 	for key, vals := range s.Label {
 		for _, v := range vals {
-			labels = append(labels, key+":"+v)
+			labels = append(labels, escapeForDot(key+":"+v))
 		}
 	}
 	sort.Strings(labels)
-	return strings.Join(labels, `\n`)
+	// To show labels on separate lines, include "\n" in the dot output.
+	return strings.Join(labels, "\\n")
 }
 
 // isNegative returns true if the node is considered as "negative" for the

internal/graph/graph_test.go

@@ -531,3 +531,18 @@ func TestShortenFunctionName(t *testing.T) {
 		}
 	}
 }
+
+func TestJoinLabels(t *testing.T) {
+	input := &profile.Sample{
+		Label: map[string][]string{
+			"key1": {"v1", "v2"},
+			// value with an embedded newline: is escaped
+			"key2": {"value line1\nline2"},
+		},
+	}
+	const expected = `key1:v1\nkey1:v2\nkey2:value line1\\nline2`
+	output := joinLabels(input)
+	if output != expected {
+		t.Errorf("output=%#v != expected=%#v", output, expected)
+	}
+}

internal/graph/testdata/compose8.dot

@@ -0,0 +1,11 @@
+digraph "testtitle" {
+node [style=filled fillcolor="#f8f8f8"]
+subgraph cluster_L { "label1" [shape=box fontsize=16 label="label1\llabel2\llabel3: \"foo\"\l" tooltip="testtitle"] }
+N1 [label="src\n10 (10.00%)\nof 25 (25.00%)" id="node1" fontsize=22 shape=box tooltip="src (25)" color="#b23c00" fillcolor="#edddd5"]
+N1_0 [label = "label\"quote\"\\nline2" id="N1_0" fontsize=8 shape=box3d tooltip="10"]
+N1 -> N1_0 [label=" 10" weight=100 tooltip="10" labeltooltip="10"]
+NN1_0 [label = "numeric\"quote\"" id="NN1_0" fontsize=8 shape=box3d tooltip="20"]
+N1 -> NN1_0 [label=" 20" weight=100 tooltip="20" labeltooltip="20"]
+N2 [label="dest\n15 (15.00%)\nof 25 (25.00%)" id="node2" fontsize=24 shape=box tooltip="dest (25)" color="#b23c00" fillcolor="#edddd5"]
+N1 -> N2 [label=" 10" weight=11 color="#b28559" tooltip="src -> dest (10)" labeltooltip="src -> dest (10)" minlen=2]
+}

internal/report/report.go

@@ -1215,7 +1215,8 @@ func reportLabels(rpt *Report, g *graph.Graph, origCount, droppedNodes, droppedE
 	// Help new users understand the graph.
 	// A new line is intentionally added here to better show this message.
 	if fullHeaders {
-		label = append(label, "\nSee https://git.io/JfYMW for how to read the graph")
+		// Include an empty string to separate with a blank line.
+		label = append(label, "", "See https://git.io/JfYMW for how to read the graph")
 	}
 
 	return label