log/slog: remove special float handling from JSONHandler

Remove the special-case handling of NaN and infinities from
appendJSONValue, making JSONHandler behave almost exactly like
a json.Encoder without HTML escaping.

The only differences are:

- Encoding errors are turned into strings, instead of causing the Handle method to fail.

- Values of type `error` are displayed as strings by calling their `Error` method.

Fixes #59345.

Change-Id: Id06bd952bbfef596e864bd5fd3f9f4f178f738c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/486855
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>

diff --git a/src/log/slog/json_handler.go b/src/log/slog/json_handler.go
index a99a99f1c1a0c589e0193d5066048378edba1afb..ec25771245f2328e31c819e2270ea554600e482c 100644 (file)
--- a/src/log/slog/json_handler.go
+++ b/src/log/slog/json_handler.go
@@ -12,7 +12,6 @@ import (
        "fmt"
        "io"
        "log/slog/internal/buffer"
-       "math"
        "strconv"
        "time"
        "unicode/utf8"
@@ -75,12 +74,16 @@ func (h *JSONHandler) WithGroup(name string) Handler {
 // To modify these or other attributes, or remove them from the output, use
 // [HandlerOptions.ReplaceAttr].
 //
-// Values are formatted as with encoding/json.Marshal, with the following
-// exceptions:
-//   - Floating-point NaNs and infinities are formatted as one of the strings
-//     "NaN", "Infinity" or "-Infinity".
-//   - Levels are formatted as with Level.String.
-//   - HTML characters are not escaped.
+// Values are formatted as with an [encoding/json.Encoder] with SetEscapeHTML(false),
+// with two exceptions.
+//
+// First, an Attr whose Value is of type error is formatted as a string, by
+// calling its Error method. Only errors in Attrs receive this special treatment,
+// not errors embedded in structs, slices, maps or other data structures that
+// are processed by the encoding/json package.
+//
+// Second, an encoding failure does not cause Handle to return an error.
+// Instead, the error message is formatted as a string.
 //
 // Each call to Handle results in a single serialized call to io.Writer.Write.
 func (h *JSONHandler) Handle(_ context.Context, r Record) error {
@@ -108,22 +111,11 @@ func appendJSONValue(s *handleState, v Value) error {
        case KindUint64:
                *s.buf = strconv.AppendUint(*s.buf, v.Uint64(), 10)
        case KindFloat64:
-               f := v.Float64()
-               // json.Marshal fails on special floats, so handle them here.
-               switch {
-               case math.IsInf(f, 1):
-                       s.buf.WriteString(`"Infinity"`)
-               case math.IsInf(f, -1):
-                       s.buf.WriteString(`"-Infinity"`)
-               case math.IsNaN(f):
-                       s.buf.WriteString(`"NaN"`)
-               default:
-                       // json.Marshal is funny about floats; it doesn't
-                       // always match strconv.AppendFloat. So just call it.
-                       // That's expensive, but floats are rare.
-                       if err := appendJSONMarshal(s.buf, f); err != nil {
-                               return err
-                       }
+               // json.Marshal is funny about floats; it doesn't
+               // always match strconv.AppendFloat. So just call it.
+               // That's expensive, but floats are rare.
+               if err := appendJSONMarshal(s.buf, v.Float64()); err != nil {
+                       return err
                }
        case KindBool:
                *s.buf = strconv.AppendBool(*s.buf, v.Bool())
@@ -163,9 +155,7 @@ func appendJSONMarshal(buf *buffer.Buffer, v any) error {
 // It does not surround the string in quotation marks.
 //
 // Modified from encoding/json/encode.go:encodeState.string,
-// with escapeHTML set to true.
-//
-// TODO: review whether HTML escaping is necessary.
+// with escapeHTML set to false.
 func appendEscapedJSONString(buf []byte, s string) []byte {
        char := func(b byte) { buf = append(buf, b) }
        str := func(s string) { buf = append(buf, s...) }

diff --git a/src/log/slog/json_handler_test.go b/src/log/slog/json_handler_test.go
index d8457cb9eed48f3647104f992bc160b95b699ee3..61078caec816391e870934b29e50b008d106d9ca 100644 (file)
--- a/src/log/slog/json_handler_test.go
+++ b/src/log/slog/json_handler_test.go
@@ -74,7 +74,7 @@ type jsonMarshalerError struct {
 func (jsonMarshalerError) Error() string { return "oops" }
 
 func TestAppendJSONValue(t *testing.T) {
-       // On most values, jsonAppendAttrValue should agree with json.Marshal.
+       // jsonAppendAttrValue should always agree with json.Marshal.
        for _, value := range []any{
                "hello",
                `"[{escape}]"`,
@@ -89,8 +89,9 @@ func TestAppendJSONValue(t *testing.T) {
                testTime,
                jsonMarshaler{"xyz"},
                jsonMarshalerError{jsonMarshaler{"pqr"}},
+               LevelWarn,
        } {
-               got := jsonValueString(t, AnyValue(value))
+               got := jsonValueString(AnyValue(value))
                want, err := marshalJSON(value)
                if err != nil {
                        t.Fatal(err)
@@ -117,24 +118,23 @@ func TestJSONAppendAttrValueSpecial(t *testing.T) {
                value any
                want  string
        }{
-               {math.NaN(), `"NaN"`},
-               {math.Inf(+1), `"Infinity"`},
-               {math.Inf(-1), `"-Infinity"`},
-               {LevelWarn, `"WARN"`},
+               {math.NaN(), `"!ERROR:json: unsupported value: NaN"`},
+               {math.Inf(+1), `"!ERROR:json: unsupported value: +Inf"`},
+               {math.Inf(-1), `"!ERROR:json: unsupported value: -Inf"`},
+               {io.EOF, `"EOF"`},
        } {
-               got := jsonValueString(t, AnyValue(test.value))
+               got := jsonValueString(AnyValue(test.value))
                if got != test.want {
                        t.Errorf("%v: got %s, want %s", test.value, got, test.want)
                }
        }
 }
 
-func jsonValueString(t *testing.T, v Value) string {
-       t.Helper()
+func jsonValueString(v Value) string {
        var buf []byte
        s := &handleState{h: &commonHandler{json: true}, buf: (*buffer.Buffer)(&buf)}
        if err := appendJSONValue(s, v); err != nil {
-               t.Fatal(err)
+               s.appendError(err)
        }
        return string(buf)
 }