diff --git a/internal/graph/dotgraph.go b/internal/graph/dotgraph.go index 8008675248..972f1d2dc8 100644 --- a/internal/graph/dotgraph.go +++ b/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() } diff --git a/internal/graph/dotgraph_test.go b/internal/graph/dotgraph_test.go index 08d80eb024..5c891e04ee 100644 --- a/internal/graph/dotgraph_test.go +++ b/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) { diff --git a/internal/graph/graph.go b/internal/graph/graph.go index 74b904c402..8a6211b675 100644 --- a/internal/graph/graph.go +++ b/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 diff --git a/internal/graph/graph_test.go b/internal/graph/graph_test.go index bdcb984ee2..d13e36e83b 100644 --- a/internal/graph/graph_test.go +++ b/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) + } +} diff --git a/internal/graph/testdata/compose8.dot b/internal/graph/testdata/compose8.dot new file mode 100644 index 0000000000..3247d5d111 --- /dev/null +++ b/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] +} diff --git a/internal/report/report.go b/internal/report/report.go index e2fb00314c..8295933b8f 100644 --- a/internal/report/report.go +++ b/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