google / google/pprof

quotes in CPU profile tags not escaped properly

Open
#769 1 comment 0 reactions 0 assignees View on GitHub
Priority: p3 type: bug
Dominant language
Go
Stars
9.3k
Forks
671
Avg merge
1d 11h
Merged PRs (30d)
10

Description

### What version of pprof are you using?

00490a63f31712a6991b73391dd3decdada278d0 (i.e. main as of today), but the problem exists in all prior versions. I first stumbled upon it in Go 1.19.

### What operating system and processor architecture are you using?

Irrelevant

### What did you do?

Run `pprof -dot` on the provided file (this is a CPU profile taken of a CockroachDB 23.1 node, but these details don't matter). Note how the resulting `.dot` file is invalid since the `"` occurring in a label was not properly escaped:

```
N36_0 [label = "range_str:12419/2:/Table/136/1/"{NHCH-…-PWN-a"}" id="N36_0" fontsize=8 shape=box3d tooltip="0.01s"]
```

[pprof.pb.gz.txt](https://github.com/google/pprof/files/11230678/pprof.pb.gz.txt)
[pprof.dot.txt](https://github.com/google/pprof/files/11230679/pprof.dot.txt)

### What did you expect to see?

The `"` in the label should have been escaped, resulting in a valid `.dot` file.

### What did you see instead?

An invalid `dot` file, which in particular implies that the graph view in pprof isn't working. The line above should have been

```
N36_0 [label = "range_str:12419/2:/Table/136/1/\"{NHCH-…-PWN-a\"}" id="N36_0" fontsize=8 shape=box3d tooltip="0.01s"]
```

which [would work](https://dreampuf.github.io/GraphvizOnline/#digraph%20G%20%7B%0A%20%20start%20-%3E%20a0%20%5Blabel%3D%22range_str%3A12419%2F2%3A%2FTable%2F136%2F1%2F%5C%22%7BNHCH-%E2%80%A6-PWN-a%5C%22%7D%22%5D%3B%0A%7D).

I gave it a half-hearted attempt at fixing, but it seemed like there might be more than one place where the escaping isn't happening properly and it wasn't super clear to me where we'd need to throw in an `escapeForDot`[^1] on top of here:

https://github.com/google/pprof/blob/a41b82acbcb1f6a6d0998b0fddeb1f1adb7d8fc1/internal/graph/dotgraph.go#L251

Also, it seems useful to add testing for these sorts of things.

[^1]: https://github.com/google/pprof/blob/a41b82acbcb1f6a6d0998b0fddeb1f1adb7d8fc1/internal/graph/dotgraph.go#L490-L495

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.