Fuzz parsers

[msiebuhr]

Still missing

• No corpus for ParseMetricSelector and ParseStmts. Should be done by writing test-cases to the appropriate files.
• No makefile-stuff is set up.

[msiebuhr]

In-progess pull, related to #667.

[juliusv]

Looks great in general, thanks! Though I want @fabxc to take a final look.

Will just make a couple of minor style comments now...

[juliusv]

Couple of style nits:

• For consistency with other Prometheus components, let's do this order:
  • license header comment (see e.g. https://github.com/prometheus/prometheus/blob/master/cmd/prometheus/main.go#L1-L12)
  • build tag comment
  • package definition
• We don't use /* ... */-style comments, so please switch to // ones.

[fabxc]

In general what @juliusv said.
The test corpus can always be extended later.

Don't mind about not having the Makefile stuff setup. I'm weirded out by the constant aliasing of trivial commands anyway – you cannot use them anyway once you want to add an extra flag or such.

[fabxc]

Nit: let's remove the blanks above the ifs for consistency with our other code.

[fabxc]

Looks good so far.

#949 turns runtime panics into an "unexpected error". This is still always a bug so the code has to be adjusted to turn this back into a panic.

[fabxc]

Please ping if you made changes so I know to take another look :)

What we still need is something like

if err.Error() == "unexpected error" {
    panic(err)
}

[juliusv]

Wouldn't it be better to use the variable errUnexpected for that comparison or does that not work for some reason?

[fabxc]

Yes :) forgot that I turned it into a variable recently.

On Thu, Aug 6, 2015 at 12:26 PM Julius Volz notifications@github.com
wrote:

Wouldn't it be better to use the variable errUnexpected for that
comparison or does that not work for some reason?

—
Reply to this email directly or view it on GitHub
#945 (comment)
.

[fabxc]

@msiebuhr ping.

You are probably busy but it would be really cool to have all your efforts merged :)

[fabxc]

Someone should take over this PR as it is really useful.

[msiebuhr]

I had totally forgotten about this!

Currently, I don't really have enough spare bandwidth to take care of this. I might get around to look at it in a few weeks time, but I can't make any promises.

[msiebuhr]

I've done a quick update on the branch, but it invariably gives me an import error when I try to build it:

>go-fuzz-build -func FuzzParseMetric -o FuzzParseMetric.zip github.com/prometheus/prometheus/promql
can't find imported package github.com/prometheus/common/model
>go-fuzz-build -func FuzzParseMetric -o FuzzParseMetric.zip github.com/prometheus/prometheus/promql
can't find imported package github.com/golang/protobuf/proto
>go-fuzz-build -func FuzzParseMetric -o FuzzParseMetric.zip github.com/prometheus/prometheus/promql
can't find imported package github.com/syndtr/goleveldb/leveldb/util

[msiebuhr]

I'm suspecting this is a consequence of dvyukov/go-fuzz#127, but I can't tell for certain.

[msiebuhr]

Turned out it was a vendoring-related bug in the fuzzer. Things should now work...

(And Dmitry kindly gave some tips on how to improve things somewhat, which I'm incorporating too.)

[fabxc]

Cool, thanks a lot for picking this back up.
Can this be merged already or do you want to add any more tweaks?

[msiebuhr]

I can't readily come up the anything to change...

[fabxc]

Okay, thanks. Wasn't sure because of your last comment.

[msiebuhr]

@fabxc Great! I applied the changes in ffc8cab...