humanlog crashes if exactly one of "skip" or "keep" are defined in the configuration file

[timoreimann]

Hello 👋

if I create a config.json file like this

{
    "skip": ["foo", "bar"]
}

humanlog (v0.7.5) crashes with

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x8 pc=0x102bd0e54]

goroutine 1 [running]:
main.newApp.func6(0x140000db1e0)
    /home/runner/work/humanlog/humanlog/cmd/humanlog/main.go:301 +0x684
github.com/urfave/cli.HandleAction({0x102cd7e00?, 0x14000081680?}, 0x140000eb180?)
    /home/runner/work/humanlog/humanlog/vendor/github.com/urfave/cli/app.go:524 +0x58
github.com/urfave/cli.(*App).Run(0x140000eb180, {0x140000961f0, 0x1, 0x1})
    /home/runner/work/humanlog/humanlog/vendor/github.com/urfave/cli/app.go:286 +0x570
main.main()
    /home/runner/work/humanlog/humanlog/cmd/humanlog/main.go:65 +0xd8

I believe this happens because both fields are defined as string slice pointers *[]string, and when only one is specified the other is nil and thus causes a nil pointer dereference here:

if len(*cfg.Skip) > 0 && len(*cfg.Keep) > 0 {

(It does not happen when no configuration file is used because the fields get defaulted to non-nil empty string slice pointers in that case.)

A workaround is to define the other field as an empty JSON array in the configuration file. Ideally though, that shouldn't be required.

One fix could be to extend the condition by a nilness check before de-referencing. However, I do wonder if those fields could be regular string slices instead -- a quick scan of mine didn't yield that the code cares about distinguishing between absent and empty values, but I could have missed something. If not though, then my personal suggestion would be to de-pointerize the fields in order to simplify things.

Let me know what you think. Either way, I'd be happy to submit a quick PR.

[aybabtme]

Hi @timoreimann, sorry I've been a bit busy to take care of this project lately. If you send a PR I'll merge it. Otherwise it might be a bit before I can loop back on this issue.

[timoreimann]

sure thing @aybabtme, happy to submit something. Thanks, and stay tuned :)

[aybabtme]

@KevRiver this is a good one to get started with

[aybabtme]

@timoreimann seems like this was fixed by @KevRiver (thanks!)

[timoreimann]

Sorry for having dropped the ball on this one. @KevRiver thanks for addressing the issue!