Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash due to index of of range in JSON encoder #249

Closed
lammel opened this issue Jul 12, 2020 · 2 comments
Closed

Crash due to index of of range in JSON encoder #249

lammel opened this issue Jul 12, 2020 · 2 comments

Comments

@lammel
Copy link

lammel commented Jul 12, 2020

We are seeing application crashes due to an error which seems to be caused by zerolog.

panic: runtime error: index out of range [-1]

goroutine 15 [running]:
github.com/rs/zerolog/internal/json.Encoder.AppendKey(...)
        /home/rl/.go/pkg/mod/github.com/rs/zerolog@v1.19.0/internal/json/base.go:7
github.com/rs/zerolog.(*Event).Str(0xc0000ae300, 0xac8fb3, 0x5, 0xac8ed2, 0x5, 0xc00008d9d8)
        /home/rl/.go/pkg/mod/github.com/rs/zerolog@v1.19.0/event.go:226 +0x1c2
github.com/rs/zerolog.(*Logger).newEvent(0xfe48a0, 0xc00069c000, 0x0, 0x4)
        /home/rl/.go/pkg/mod/github.com/rs/zerolog@v1.19.0/log.go:420 +0x32d
github.com/rs/zerolog.(*Logger).Debug(...)
        /home/rl/.go/pkg/mod/github.com/rs/zerolog@v1.19.0/log.go:293
github.com/rs/zerolog/log.Debug(...)
        /home/rl/.go/pkg/mod/github.com/rs/zerolog@v1.19.0/log/log.go:59
neotel.at/api/pcr-api/poller.pollEnterprises(0xc000284300, 0x0)
        /home/rl/work/pcr-api/poller/poller.go:85 +0xc3

In internal/json/base.go an array access might become out of range as:

func (e Encoder) AppendKey(dst []byte, key string) []byte {
	if dst[len(dst)-1] != '{' {
		dst = append(dst, ',')
	}
	return append(e.AppendString(dst, key), ':')
}

This could probably be fixed by using:

	if len(dst) > 0 && dst[len(dst)-1] != '{' {

But the issue might be something else, as we are logging quite data over the day and the crashed occur only around once a day.

@lammel
Copy link
Author

lammel commented Jul 14, 2020

Actually it seems the issue was introduced with commit 14dcf38 which removed the check for the length of the array to make it inlineable. This can cause the index out of range now in case the array is empty.

I'd suggest to readd the guard or see if there is a safe way to inline, without trading safety.

@lammel
Copy link
Author

lammel commented Jul 14, 2020

This issue is a duplicate of issue #239 which was fixed with PR #244.
Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant