slog: TextHandler crash with nil TextMarshaller

[holiman]

What version of Go are you using (go version)?

$ go version

1.21 (play.golang.org)

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

triggers on play.golang.org

What did you do?

The slog package texthandler panics for certain cases of nil, specifically if the object being nil is a TextMarshaller. Other types of nil values are handled, as far as I can tell, and the json handler handles this case fine, so it seems to be that it's a bug.

Repro

package main

import (
	"log/slog"
	"os"
	"time"
)

type custom struct{}

func (c *custom) String() string {
	return "test"
}

func main() {
	logger := slog.New(slog.NewJSONHandler(os.Stdout, nil))
	logger.Info("hello", "stringer", (*custom)(nil))
	logger.Info("hello", "marshaller", (*time.Time)(nil))

	logger = slog.New(slog.NewTextHandler(os.Stdout, nil))
	logger.Info("hello", "stringer", (*custom)(nil))
	logger.Info("hello", "marshaller", (*time.Time)(nil))
}

Output

{"time":"2009-11-10T23:00:00Z","level":"INFO","msg":"hello","stringer":null}
{"time":"2009-11-10T23:00:00Z","level":"INFO","msg":"hello","marshaller":null}
time=2009-11-10T23:00:00.000Z level=INFO msg=hello stringer=test
panic: value method time.Time.MarshalText called using nil *Time pointer

goroutine 1 [running]:
time.(*Time).MarshalText(0xc000117888?)
	<autogenerated>:1 +0x3a
log/slog.appendTextValue(0xc000117888, {{}, 0x4cdee0?, {0x4cdee0?, 0x0?}})
	/usr/local/go-faketime/src/log/slog/text_handler.go:106 +0x28e
log/slog.(*handleState).appendValue(0xc000117888, {{}, 0x4d0fff?, {0x4cdee0?, 0x0?}})
	/usr/local/go-faketime/src/log/slog/handler.go:519 +0x31

Crashing code

func appendTextValue(s *handleState, v Value) error {
	switch v.Kind() {
	case KindString:
		s.appendString(v.str())
	case KindTime:
		s.appendTime(v.time())
	case KindAny:
		if tm, ok := v.any.(encoding.TextMarshaler); ok {
			data, err := tm.MarshalText() <-- boom

[holiman]

It can also be noted that the two handlers handle a fmt.Stringer differently, the json not evaluating it

{"time":"2009-11-10T23:00:00Z","level":"INFO","msg":"hello","stringer":null}

And the text handler evaluating it:

time=2009-11-10T23:00:00.000Z level=INFO msg=hello stringer=test

[callthingsoff]

https://go.dev/play/p/9XV1HnSDSUa?v=gotip

Could try dev-branch, seems work fine.

[mateusz834]

This was fixed by: #61648.

[holiman]

Oh nice. Do you happen to know if that fix was also applied for exp/slog?

[mateusz834]

https://go.dev/play/p/8Z9QN5CU5yq?v=gotip

It is not fixed there.

[holiman]

Yeah, just saw.
https://cs.opensource.google/go/x/exp/+/master:slog/handler.go;bpv=1 -- last slog: sync with log/slog was in May 2023, fix is from August

[holiman]

We (go-ethereum) are trying to move over to slog, but we have a policy of supporting the two latest go-versions. Hence, we were planning to use x/exp/slog for a couple of releases, but this bug is a bit of a blocker for that to happen.

I'm not quite sure how the x/exp is managed, if it's tracked on this repository or somewhere else. I would like to issue a request for the x/exp/slog package to be updated with the fix. Apologies if this is not the correct place to raise it, cc @jba

[jba]

Normally exp is unsupported, but we'll keep exp/slog working until the next release.

[holiman]

Seems fixed as of v0.0.0-20231110203233-9a3e6036ecaa
Thanks!