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

Regression: panic when calling UpdateContext on a context not yet initialized #239

Closed
hairyhenderson opened this issue Jun 9, 2020 · 1 comment · Fixed by #244
Closed

Comments

@hairyhenderson
Copy link

I'm seeing a new panic with v0.19.0 that I didn't experience with v0.18.0:

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

goroutine 1 [running]:
github.com/rs/zerolog/internal/json.Encoder.AppendKey(...)
        /Users/hairyhenderson/go/pkg/mod/github.com/rs/zerolog@v1.19.0/internal/json/base.go:7
github.com/rs/zerolog.Context.Str(0x1144480, 0xc000098590, 0xff, 0x0, 0x0, 0xc0000cc400, 0x0, 0x1f4, 0x0, 0x0, ...)
        /Users/hairyhenderson/go/pkg/mod/github.com/rs/zerolog@v1.19.0/context.go:74 +0x202
main.main.func3(0x1144480, 0xc000098590, 0xff, 0x0, 0x0, 0xc0000cc400, 0x0, 0x1f4, 0x0, 0x0, ...)
        /Users/hairyhenderson/zlbug/main.go:21 +0xb8
github.com/rs/zerolog.(*Logger).UpdateContext(0xc0000d9e48, 0x1126520)
        /Users/hairyhenderson/go/pkg/mod/github.com/rs/zerolog@v1.19.0/log.go:255 +0xef
main.main()
        /Users/hairyhenderson/zlbug/main.go:20 +0x1b0
exit status 2

This code can reproduce it:

package main

import (
	"os"

	"github.com/rs/zerolog"
)

func main() {
	log := zerolog.New(zerolog.ConsoleWriter{
		Out:             os.Stdout,
		NoColor:         true,
		FormatTimestamp: func(i interface{}) string { return "" },
		FormatLevel:     func(i interface{}) string { return "" },
	})

	log.Info().Msg("Hello world")
	log.UpdateContext(func(c zerolog.Context) zerolog.Context {
		return c.Str("foo", "bar")
	})
	log.Info().Msg("kaboom")
}

If I add a line like log = log.With().Logger() before calling log.UpdateContext, it's OK.

This was introduced by #226, specifically here. The dst byte array is no longer being length-checked before attempting to access index len(dst)-1 (-1 when the array is empty - i.e. when it hasn't been initailized yet).

Note that I only ran into this in a unit test, and is easily worked around by calling log.With(), but it's reasonable to assume this will affect other code as well.

This workaround only works because enc.AppendBeginMarker is now called by With, and it tolerates a zero-length array and produces an array of length 1.

/cc @ffenix113 @rs

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

Successfully merging a pull request may close this issue.

1 participant