From 98cf0c2274a686f85b7c2a51bc5dcf90034b1ff2 Mon Sep 17 00:00:00 2001 From: CRaLFa Date: Tue, 10 Nov 2026 12:37:35 +0900 Subject: [PATCH 1/3] feat: add process-wide JSON codec Add a JSONCodec that generated code and this package share, so the JSON implementation can be replaced in one place. - Add JSONCodec, StdJSONCodec, SetJSONCodec, JSONMarshal and JSONUnmarshal. - Route the json.Marshal and json.Unmarshal calls in bindform.go, bindjson.go, bindparam.go and deepobject.go through the codec. - Route the MarshalJSON and UnmarshalJSON methods in types/ through the codec. - Keep the codec in internal/jsoncodec, because runtime imports types and types cannot import runtime. Co-Authored-By: Claude Fable 5.2 --- bindform.go | 5 +- bindjson.go | 4 +- bindparam.go | 3 +- deepobject.go | 2 +- internal/jsoncodec/jsoncodec.go | 35 ++++++++++++++ jsoncodec.go | 29 ++++++++++++ jsoncodec_test.go | 83 +++++++++++++++++++++++++++++++++ types/date.go | 7 +-- types/duration.go | 7 +-- types/email.go | 7 +-- types/file.go | 7 +-- 11 files changed, 169 insertions(+), 20 deletions(-) create mode 100644 internal/jsoncodec/jsoncodec.go create mode 100644 jsoncodec.go create mode 100644 jsoncodec_test.go diff --git a/bindform.go b/bindform.go index d7f3666..cd99b7f 100644 --- a/bindform.go +++ b/bindform.go @@ -1,7 +1,6 @@ package runtime import ( - "encoding/json" "errors" "fmt" "mime/multipart" @@ -69,7 +68,7 @@ func BindForm(ptr interface{}, form map[string][]string, files map[string][]*mul value := values[0] if encoding.ContentType != "" { if strings.HasPrefix(encoding.ContentType, jsonContentType) { - if err := json.Unmarshal([]byte(value), ptr); err != nil { + if err := JSONUnmarshal([]byte(value), ptr); err != nil { return err } } @@ -122,7 +121,7 @@ func MarshalForm(ptr interface{}, encodings map[string]RequestBodyEncoding) (url tag = strings.Split(tag, ",")[0] // extract the name of the tag if encoding, ok := encodings[tag]; ok && encoding.ContentType != "" { if strings.HasPrefix(encoding.ContentType, jsonContentType) { - if data, err := json.Marshal(field); err != nil { //nolint:staticcheck + if data, err := JSONMarshal(field); err != nil { //nolint:staticcheck return nil, err } else { result[tag] = append(result[tag], string(data)) diff --git a/bindjson.go b/bindjson.go index 3a14b1a..91c8975 100644 --- a/bindjson.go +++ b/bindjson.go @@ -102,7 +102,7 @@ func bindJSONValue(value any, dst reflect.Value) error { } if u := jsonUnmarshalerOf(dst); u != nil { - data, err := json.Marshal(value) + data, err := JSONMarshal(value) if err != nil { return err } @@ -295,7 +295,7 @@ func bindQuotedJSONValue(value any, dst reflect.Value) error { if !ok || !dst.CanAddr() { return jsonTypeError(value, dst.Type()) } - return json.Unmarshal([]byte(s), dst.Addr().Interface()) + return JSONUnmarshal([]byte(s), dst.Addr().Interface()) } func jsonUnmarshalerOf(v reflect.Value) json.Unmarshaler { diff --git a/bindparam.go b/bindparam.go index eebd9b2..dec1a57 100644 --- a/bindparam.go +++ b/bindparam.go @@ -15,7 +15,6 @@ package runtime import ( "encoding" - "encoding/json" "errors" "fmt" "net/url" @@ -380,7 +379,7 @@ func bindSplitPartsToDestinationStruct(paramName string, parts []string, explode } } jsonParam := "{" + strings.Join(fields, ",") + "}" - err := json.Unmarshal([]byte(jsonParam), dest) + err := JSONUnmarshal([]byte(jsonParam), dest) if err != nil { return fmt.Errorf("error binding parameter %s fields: %w", paramName, err) } diff --git a/deepobject.go b/deepobject.go index f33a020..150aa48 100644 --- a/deepobject.go +++ b/deepobject.go @@ -94,7 +94,7 @@ func MarshalDeepObject(i interface{}, paramName string) (string, error) { return "", nil } } - buf, err := json.Marshal(i) + buf, err := JSONMarshal(i) if err != nil { return "", fmt.Errorf("failed to marshal input to JSON: %w", err) } diff --git a/internal/jsoncodec/jsoncodec.go b/internal/jsoncodec/jsoncodec.go new file mode 100644 index 0000000..ed64f9f --- /dev/null +++ b/internal/jsoncodec/jsoncodec.go @@ -0,0 +1,35 @@ +// Package jsoncodec holds the process-wide JSON codec. +// +// It is internal so that both the runtime package and runtime/types can share +// one codec: runtime imports types, so types cannot import runtime. +package jsoncodec + +import "encoding/json" + +// Codec mirrors runtime.JSONCodec. +type Codec interface { + Marshal(v any) ([]byte, error) + Unmarshal(data []byte, v any) error +} + +// codec is nil while the encoding/json default is in use. +var codec Codec + +// Set replaces the codec. nil restores the encoding/json default. +func Set(c Codec) { + codec = c +} + +func Marshal(v any) ([]byte, error) { + if codec == nil { + return json.Marshal(v) + } + return codec.Marshal(v) +} + +func Unmarshal(data []byte, v any) error { + if codec == nil { + return json.Unmarshal(data, v) + } + return codec.Unmarshal(data, v) +} diff --git a/jsoncodec.go b/jsoncodec.go new file mode 100644 index 0000000..1e3cd0b --- /dev/null +++ b/jsoncodec.go @@ -0,0 +1,29 @@ +package runtime + +import ( + "encoding/json" + + "github.com/oapi-codegen/runtime/internal/jsoncodec" +) + +// JSONCodec encodes and decodes JSON for generated code and for this package. +type JSONCodec interface { + Marshal(v any) ([]byte, error) + Unmarshal(data []byte, v any) error +} + +// StdJSONCodec is the default, backed by encoding/json. Embed it to replace one method. +type StdJSONCodec struct{} + +func (StdJSONCodec) Marshal(v any) ([]byte, error) { return json.Marshal(v) } +func (StdJSONCodec) Unmarshal(data []byte, v any) error { return json.Unmarshal(data, v) } + +// SetJSONCodec replaces the codec used by every generated package in the process and by +// this package. Call it during initialisation; it is not synchronised. nil restores the default. +func SetJSONCodec(c JSONCodec) { jsoncodec.Set(c) } + +// JSONMarshal encodes v with the current JSONCodec. +func JSONMarshal(v any) ([]byte, error) { return jsoncodec.Marshal(v) } + +// JSONUnmarshal decodes data into v with the current JSONCodec. +func JSONUnmarshal(data []byte, v any) error { return jsoncodec.Unmarshal(data, v) } diff --git a/jsoncodec_test.go b/jsoncodec_test.go new file mode 100644 index 0000000..92f982c --- /dev/null +++ b/jsoncodec_test.go @@ -0,0 +1,83 @@ +package runtime + +import ( + "bytes" + "encoding/json" + "testing" + "time" + + "github.com/oapi-codegen/runtime/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// noEscapeCodec overrides Marshal only, and counts the calls that reach it. +type noEscapeCodec struct { + StdJSONCodec + marshals, unmarshals *int +} + +func (c noEscapeCodec) Marshal(v any) ([]byte, error) { + *c.marshals++ + var buf bytes.Buffer + enc := json.NewEncoder(&buf) + enc.SetEscapeHTML(false) + if err := enc.Encode(v); err != nil { + return nil, err + } + return bytes.TrimSuffix(buf.Bytes(), []byte("\n")), nil +} + +func (c noEscapeCodec) Unmarshal(data []byte, v any) error { + *c.unmarshals++ + return c.StdJSONCodec.Unmarshal(data, v) +} + +func TestSetJSONCodec(t *testing.T) { + t.Cleanup(func() { SetJSONCodec(nil) }) + + const in = "" + + // Default: encoding/json, which escapes HTML. + escaped, err := json.Marshal(in) + require.NoError(t, err) + require.NotEqual(t, `""`, string(escaped)) + + got, err := JSONMarshal(in) + require.NoError(t, err) + assert.Equal(t, string(escaped), string(got)) + + var marshals, unmarshals int + SetJSONCodec(noEscapeCodec{marshals: &marshals, unmarshals: &unmarshals}) + + got, err = JSONMarshal(in) + require.NoError(t, err) + assert.Equal(t, `""`, string(got)) + assert.Equal(t, 1, marshals) + + var out string + require.NoError(t, JSONUnmarshal(got, &out)) + assert.Equal(t, in, out) + assert.Equal(t, 1, unmarshals) + + // The runtime's own encoding goes through the codec too. + _, err = MarshalDeepObject(map[string]string{"k": "v"}, "p") + require.NoError(t, err) + assert.Equal(t, 2, marshals) + + // So does runtime/types, which cannot import this package. + d := types.Date{Time: time.Date(2020, 1, 2, 0, 0, 0, 0, time.UTC)} + got, err = d.MarshalJSON() + require.NoError(t, err) + assert.Equal(t, `"2020-01-02"`, string(got)) + assert.Equal(t, 3, marshals) + require.NoError(t, d.UnmarshalJSON(got)) + assert.Equal(t, 2, unmarshals) + + // nil restores the default. + SetJSONCodec(nil) + got, err = JSONMarshal(in) + require.NoError(t, err) + assert.Equal(t, string(escaped), string(got)) + assert.Equal(t, 3, marshals) +} diff --git a/types/date.go b/types/date.go index 393dbda..a2eebad 100644 --- a/types/date.go +++ b/types/date.go @@ -1,8 +1,9 @@ package types import ( - "encoding/json" "time" + + "github.com/oapi-codegen/runtime/internal/jsoncodec" ) const DateFormat = "2006-01-02" @@ -12,12 +13,12 @@ type Date struct { } func (d Date) MarshalJSON() ([]byte, error) { - return json.Marshal(d.Format(DateFormat)) + return jsoncodec.Marshal(d.Format(DateFormat)) } func (d *Date) UnmarshalJSON(data []byte) error { var dateStr string - err := json.Unmarshal(data, &dateStr) + err := jsoncodec.Unmarshal(data, &dateStr) if err != nil { return err } diff --git a/types/duration.go b/types/duration.go index 2ef80c8..01803a9 100644 --- a/types/duration.go +++ b/types/duration.go @@ -1,12 +1,13 @@ package types import ( - "encoding/json" "errors" "fmt" "strconv" "strings" "time" + + "github.com/oapi-codegen/runtime/internal/jsoncodec" ) // Duration represents an RFC 3339 duration (the ISO 8601 duration grammar @@ -241,12 +242,12 @@ func FromTimeDuration(td time.Duration) (Duration, error) { } func (d Duration) MarshalJSON() ([]byte, error) { - return json.Marshal(d.String()) + return jsoncodec.Marshal(d.String()) } func (d *Duration) UnmarshalJSON(data []byte) error { var s string - if err := json.Unmarshal(data, &s); err != nil { + if err := jsoncodec.Unmarshal(data, &s); err != nil { return err } parsed, err := ParseDuration(s) diff --git a/types/email.go b/types/email.go index 234e7cc..e0ca71f 100644 --- a/types/email.go +++ b/types/email.go @@ -1,9 +1,10 @@ package types import ( - "encoding/json" "errors" "net/mail" + + "github.com/oapi-codegen/runtime/internal/jsoncodec" ) // ErrValidationEmail is the sentinel error returned when an email fails validation @@ -20,7 +21,7 @@ func (e Email) MarshalJSON() ([]byte, error) { return nil, ErrValidationEmail } - return json.Marshal(m.Address) + return jsoncodec.Marshal(m.Address) } func (e *Email) UnmarshalJSON(data []byte) error { @@ -29,7 +30,7 @@ func (e *Email) UnmarshalJSON(data []byte) error { } var s string - if err := json.Unmarshal(data, &s); err != nil { + if err := jsoncodec.Unmarshal(data, &s); err != nil { return err } diff --git a/types/file.go b/types/file.go index 7b26a0d..1b06687 100644 --- a/types/file.go +++ b/types/file.go @@ -2,9 +2,10 @@ package types import ( "bytes" - "encoding/json" "io" "mime/multipart" + + "github.com/oapi-codegen/runtime/internal/jsoncodec" ) type File struct { @@ -30,11 +31,11 @@ func (file File) MarshalJSON() ([]byte, error) { if err != nil { return nil, err } - return json.Marshal(b) + return jsoncodec.Marshal(b) } func (file *File) UnmarshalJSON(data []byte) error { - return json.Unmarshal(data, &file.data) + return jsoncodec.Unmarshal(data, &file.data) } func (file File) Bytes() ([]byte, error) { From 01480a40f2182973bc20d9d09e91e87d04c5fdee Mon Sep 17 00:00:00 2001 From: Marcin Romaszewicz Date: Tue, 10 Nov 2026 10:26:51 -0800 Subject: [PATCH 2/3] feat: stream bodies through the JSON codec, re-export the internal package Follow-up to the process-wide JSON codec, per the review on #183. Re-export rather than duplicate: the internal/jsoncodec package now also holds the default implementation, and the top-level file is type aliases and one-line wrappers, so neither the interface nor the default is defined twice. Add StreamingJSONCodec, optionally implemented by a JSONCodec, with MarshalWrite(io.Writer, any) and UnmarshalRead(io.Reader, any), and the helpers JSONMarshalWrite and JSONUnmarshalRead that use those methods when present and otherwise buffer through Marshal and Unmarshal. One SetJSONCodec registration covers both, so a codec that implements only the byte pair behaves coherently everywhere, and one that also streams is picked up automatically. The method names are encoding/json/v2's. StdJSONCodec implements only the byte pair. A codec that embeds it and overrides Marshal must not have encoding/json's Encoder promoted past that override for bodies. The default codec, which SetJSONCodec(nil) restores, is unexported and adds encoding/json's Encoder and Decoder, so behaviour without SetJSONCodec is unchanged. BindJSONBody decodes through JSONUnmarshalRead. Generated handlers take io.EOF as an optional body left out, and neither json.Unmarshal nor v2 reports empty input that way, so BindJSONBody checks for an empty or white-space-only body itself and returns io.EOF before the codec is involved. Co-Authored-By: Claude Fable 5.1 --- bindjson.go | 39 ++++++++++- internal/jsoncodec/jsoncodec.go | 85 ++++++++++++++++++----- jsoncodec.go | 64 +++++++++++++----- jsoncodec_test.go | 115 ++++++++++++++++++++++++++++++++ 4 files changed, 270 insertions(+), 33 deletions(-) diff --git a/bindjson.go b/bindjson.go index 91c8975..6901011 100644 --- a/bindjson.go +++ b/bindjson.go @@ -1,11 +1,13 @@ package runtime import ( + "bytes" "encoding" "encoding/base64" "encoding/json" "errors" "fmt" + "io" "net/http" "reflect" "strconv" @@ -31,8 +33,12 @@ type validatedBody interface { // case-insensitively, and the body is not decoded a second time. // // Without a validated value, or when the value cannot be bound into v, the -// body is decoded with encoding/json exactly as before, so error behaviour -// (including io.EOF for an empty body) is unchanged. +// body is decoded with the current JSONCodec (see SetJSONCodec), as a stream +// when it implements StreamingJSONCodec. With the default codec that is +// encoding/json's Decoder, so its error behaviour is unchanged. A body that is +// empty, or only white space, is io.EOF whichever codec is in use, as the +// Decoder reports it: generated handlers take that as an optional body left +// out. func BindJSONBody(r *http.Request, v any) error { if r != nil && r.Body != nil { if b, ok := r.Body.(validatedBody); ok { @@ -41,7 +47,34 @@ func BindJSONBody(r *http.Request, v any) error { } } } - return json.NewDecoder(r.Body).Decode(v) + body, err := nonEmptyBody(r.Body) + if err != nil { + return err + } + return JSONUnmarshalRead(body, v) +} + +// nonEmptyBody returns r positioned at its first byte that isn't JSON white +// space, or io.EOF when it has none, so that an empty body is io.EOF whichever +// codec decodes it. +func nonEmptyBody(r io.Reader) (io.Reader, error) { + if r == nil { + return nil, io.EOF + } + var b [1]byte + for { + n, err := r.Read(b[:]) + if n == 1 { + switch b[0] { + case ' ', '\t', '\r', '\n': + continue + } + return io.MultiReader(bytes.NewReader(b[:]), r), nil + } + if err != nil { + return nil, err + } + } } // BindJSONValue binds a decoded JSON value into v, which must be a non-nil diff --git a/internal/jsoncodec/jsoncodec.go b/internal/jsoncodec/jsoncodec.go index ed64f9f..39077ce 100644 --- a/internal/jsoncodec/jsoncodec.go +++ b/internal/jsoncodec/jsoncodec.go @@ -1,35 +1,90 @@ -// Package jsoncodec holds the process-wide JSON codec. +// Package jsoncodec holds the process-wide JSON codec that the runtime package +// and runtime/types share. // -// It is internal so that both the runtime package and runtime/types can share -// one codec: runtime imports types, so types cannot import runtime. +// It is internal because runtime imports types, so types cannot import +// runtime. The runtime package re-exports its symbols, as aliases of the types +// and one-line wrappers of the functions, so that nothing is defined twice. package jsoncodec -import "encoding/json" +import ( + "encoding/json" + "io" +) -// Codec mirrors runtime.JSONCodec. +// Codec encodes and decodes JSON. runtime.JSONCodec is an alias of it. type Codec interface { Marshal(v any) ([]byte, error) Unmarshal(data []byte, v any) error } -// codec is nil while the encoding/json default is in use. -var codec Codec +// Streaming is optionally implemented by a Codec that encodes to and decodes +// from a stream. runtime.StreamingJSONCodec is an alias of it. +type Streaming interface { + MarshalWrite(w io.Writer, v any) error + UnmarshalRead(r io.Reader, v any) error +} + +// Std is encoding/json's json.Marshal and json.Unmarshal. +// runtime.StdJSONCodec is an alias of it. +// +// It deliberately doesn't implement Streaming: a codec that embeds it and +// overrides Marshal or Unmarshal then encodes and decodes streams through +// its own methods, via MarshalWrite and UnmarshalRead's fall-through, rather +// than through encoding/json's Encoder and Decoder promoted from Std. +type Std struct{} + +func (Std) Marshal(v any) ([]byte, error) { return json.Marshal(v) } +func (Std) Unmarshal(data []byte, v any) error { return json.Unmarshal(data, v) } + +// std is the default codec: Std, plus encoding/json's Encoder and Decoder for +// streams, so that the default behaviour is encoding/json's throughout. +type std struct{ Std } + +func (std) MarshalWrite(w io.Writer, v any) error { return json.NewEncoder(w).Encode(v) } +func (std) UnmarshalRead(r io.Reader, v any) error { return json.NewDecoder(r).Decode(v) } + +var _ Streaming = std{} -// Set replaces the codec. nil restores the encoding/json default. +var codec Codec = std{} + +// Set replaces the codec. nil restores the default. func Set(c Codec) { + if c == nil { + c = std{} + } codec = c } -func Marshal(v any) ([]byte, error) { - if codec == nil { - return json.Marshal(v) +// Marshal encodes v with the codec. +func Marshal(v any) ([]byte, error) { return codec.Marshal(v) } + +// Unmarshal decodes data into v with the codec. +func Unmarshal(data []byte, v any) error { return codec.Unmarshal(data, v) } + +// MarshalWrite encodes v to w: with the codec's MarshalWrite when it +// implements Streaming, otherwise with Marshal followed by a single Write. +func MarshalWrite(w io.Writer, v any) error { + if s, ok := codec.(Streaming); ok { + return s.MarshalWrite(w, v) + } + data, err := codec.Marshal(v) + if err != nil { + return err } - return codec.Marshal(v) + _, err = w.Write(data) + return err } -func Unmarshal(data []byte, v any) error { - if codec == nil { - return json.Unmarshal(data, v) +// UnmarshalRead decodes v from r: with the codec's UnmarshalRead when it +// implements Streaming, otherwise by reading r to its end and calling +// Unmarshal. +func UnmarshalRead(r io.Reader, v any) error { + if s, ok := codec.(Streaming); ok { + return s.UnmarshalRead(r, v) + } + data, err := io.ReadAll(r) + if err != nil { + return err } return codec.Unmarshal(data, v) } diff --git a/jsoncodec.go b/jsoncodec.go index 1e3cd0b..0199b53 100644 --- a/jsoncodec.go +++ b/jsoncodec.go @@ -1,25 +1,49 @@ package runtime import ( - "encoding/json" + "io" "github.com/oapi-codegen/runtime/internal/jsoncodec" ) -// JSONCodec encodes and decodes JSON for generated code and for this package. -type JSONCodec interface { - Marshal(v any) ([]byte, error) - Unmarshal(data []byte, v any) error -} - -// StdJSONCodec is the default, backed by encoding/json. Embed it to replace one method. -type StdJSONCodec struct{} - -func (StdJSONCodec) Marshal(v any) ([]byte, error) { return json.Marshal(v) } -func (StdJSONCodec) Unmarshal(data []byte, v any) error { return json.Unmarshal(data, v) } - -// SetJSONCodec replaces the codec used by every generated package in the process and by -// this package. Call it during initialisation; it is not synchronised. nil restores the default. +// JSONCodec encodes and decodes JSON for this package and for the code +// oapi-codegen generates. It has two methods: +// +// Marshal(v any) ([]byte, error) +// Unmarshal(data []byte, v any) error +// +// A codec must produce and accept standard JSON: it selects an implementation +// and its options, such as encoding/json/v2 or an encoder that doesn't escape +// HTML, not a different format. A codec that also implements +// StreamingJSONCodec encodes and decodes request and response bodies as +// streams. +type JSONCodec = jsoncodec.Codec + +// StreamingJSONCodec is optionally implemented by a JSONCodec that encodes to +// and decodes from a stream, which request and response bodies use: +// +// MarshalWrite(w io.Writer, v any) error +// UnmarshalRead(r io.Reader, v any) error +// +// Without it, JSONMarshalWrite and JSONUnmarshalRead buffer the whole body +// through Marshal and Unmarshal. Whether MarshalWrite ends the value with a +// newline, and whether UnmarshalRead rejects data after the value, is up to +// the implementation: encoding/json's Encoder and Decoder do and don't, +// respectively. Neither this package nor generated code relies on either. +type StreamingJSONCodec = jsoncodec.Streaming + +// StdJSONCodec is encoding/json's json.Marshal and json.Unmarshal. Embed it in +// a codec to replace one method. +// +// It doesn't implement StreamingJSONCodec, so a codec that embeds it handles +// bodies through its own Marshal and Unmarshal. The default codec, which +// SetJSONCodec(nil) restores, adds encoding/json's Encoder and Decoder for +// them. +type StdJSONCodec = jsoncodec.Std + +// SetJSONCodec replaces the codec used by this package and by the code +// oapi-codegen generates, process-wide. Call it during initialisation; it is +// not synchronised. nil restores the default. func SetJSONCodec(c JSONCodec) { jsoncodec.Set(c) } // JSONMarshal encodes v with the current JSONCodec. @@ -27,3 +51,13 @@ func JSONMarshal(v any) ([]byte, error) { return jsoncodec.Marshal(v) } // JSONUnmarshal decodes data into v with the current JSONCodec. func JSONUnmarshal(data []byte, v any) error { return jsoncodec.Unmarshal(data, v) } + +// JSONMarshalWrite encodes v to w with the current JSONCodec: with its +// MarshalWrite when it implements StreamingJSONCodec, otherwise with Marshal +// followed by a single Write. +func JSONMarshalWrite(w io.Writer, v any) error { return jsoncodec.MarshalWrite(w, v) } + +// JSONUnmarshalRead decodes v from r with the current JSONCodec: with its +// UnmarshalRead when it implements StreamingJSONCodec, otherwise by reading r +// to its end and calling Unmarshal. +func JSONUnmarshalRead(r io.Reader, v any) error { return jsoncodec.UnmarshalRead(r, v) } diff --git a/jsoncodec_test.go b/jsoncodec_test.go index 92f982c..5016e19 100644 --- a/jsoncodec_test.go +++ b/jsoncodec_test.go @@ -3,6 +3,9 @@ package runtime import ( "bytes" "encoding/json" + "io" + "net/http" + "strings" "testing" "time" @@ -33,6 +36,29 @@ func (c noEscapeCodec) Unmarshal(data []byte, v any) error { return c.StdJSONCodec.Unmarshal(data, v) } +// streamingCodec implements StreamingJSONCodec, and counts the calls that +// reach each method. +type streamingCodec struct { + noEscapeCodec + writes, reads *int +} + +func (c streamingCodec) MarshalWrite(w io.Writer, v any) error { + *c.writes++ + _, err := io.WriteString(w, "streamed") + return err +} + +func (c streamingCodec) UnmarshalRead(r io.Reader, v any) error { + *c.reads++ + *v.(*string) = "streamed" + return nil +} + +var _ StreamingJSONCodec = streamingCodec{} + +// TestSetJSONCodec mutates the process-wide codec, so it must not run in +// parallel with other tests (they run after all sequential tests finish). func TestSetJSONCodec(t *testing.T) { t.Cleanup(func() { SetJSONCodec(nil) }) @@ -81,3 +107,92 @@ func TestSetJSONCodec(t *testing.T) { assert.Equal(t, string(escaped), string(got)) assert.Equal(t, 3, marshals) } + +func TestJSONCodec_Streams(t *testing.T) { + t.Cleanup(func() { SetJSONCodec(nil) }) + + const in = "" + + t.Run("default is encoding/json's Encoder and Decoder", func(t *testing.T) { + var buf bytes.Buffer + require.NoError(t, JSONMarshalWrite(&buf, in)) + var want bytes.Buffer + require.NoError(t, json.NewEncoder(&want).Encode(in)) + assert.Equal(t, want.String(), buf.String(), "HTML escaped, trailing newline") + + var out string + require.NoError(t, JSONUnmarshalRead(strings.NewReader(`"x"`), &out)) + assert.Equal(t, "x", out) + assert.ErrorIs(t, JSONUnmarshalRead(strings.NewReader(""), &out), io.EOF) + }) + + t.Run("StdJSONCodec itself doesn't stream", func(t *testing.T) { + _, ok := any(StdJSONCodec{}).(StreamingJSONCodec) + assert.False(t, ok, "embedding it must not promote stream methods past an overridden Marshal") + }) + + t.Run("a codec without stream methods is buffered through them", func(t *testing.T) { + var marshals, unmarshals int + SetJSONCodec(noEscapeCodec{marshals: &marshals, unmarshals: &unmarshals}) + + var buf bytes.Buffer + require.NoError(t, JSONMarshalWrite(&buf, in)) + assert.Equal(t, `""`, buf.String(), "the overridden Marshal, not encoding/json's Encoder") + assert.Equal(t, 1, marshals) + + var out string + require.NoError(t, JSONUnmarshalRead(strings.NewReader(`"x"`), &out)) + assert.Equal(t, "x", out) + assert.Equal(t, 1, unmarshals) + }) + + t.Run("a codec with stream methods streams", func(t *testing.T) { + var marshals, unmarshals, writes, reads int + SetJSONCodec(streamingCodec{ + noEscapeCodec: noEscapeCodec{marshals: &marshals, unmarshals: &unmarshals}, + writes: &writes, + reads: &reads, + }) + + var buf bytes.Buffer + require.NoError(t, JSONMarshalWrite(&buf, in)) + assert.Equal(t, "streamed", buf.String()) + + var out string + require.NoError(t, JSONUnmarshalRead(strings.NewReader(`"x"`), &out)) + assert.Equal(t, "streamed", out) + + assert.Equal(t, 1, writes) + assert.Equal(t, 1, reads) + assert.Equal(t, 0, marshals, "the byte methods are not used for streams") + assert.Equal(t, 0, unmarshals) + }) +} + +func TestBindJSONBody_Codec(t *testing.T) { + t.Cleanup(func() { SetJSONCodec(nil) }) + + newRequest := func(body string) *http.Request { + r, err := http.NewRequest(http.MethodPost, "/", strings.NewReader(body)) + require.NoError(t, err) + return r + } + + var marshals, unmarshals int + SetJSONCodec(noEscapeCodec{marshals: &marshals, unmarshals: &unmarshals}) + + t.Run("decodes through the codec", func(t *testing.T) { + var got map[string]string + require.NoError(t, BindJSONBody(newRequest(` {"name":"bob"}`), &got)) + assert.Equal(t, map[string]string{"name": "bob"}, got) + assert.Equal(t, 1, unmarshals) + }) + + t.Run("an empty body is io.EOF whichever codec is in use", func(t *testing.T) { + var got map[string]string + assert.ErrorIs(t, BindJSONBody(newRequest(""), &got), io.EOF) + assert.ErrorIs(t, BindJSONBody(newRequest(" \r\n\t"), &got), io.EOF) + assert.ErrorIs(t, BindJSONBody(&http.Request{}, &got), io.EOF) + assert.Equal(t, 1, unmarshals, "the codec never saw the empty bodies") + }) +} From a6857ac2d6d04036b316cef5e5014203bb5b3b3d Mon Sep 17 00:00:00 2001 From: Marcin Romaszewicz Date: Tue, 10 Nov 2026 10:43:35 -0800 Subject: [PATCH 3/3] fix: make io.EOF for an empty body the codec helper's contract Review follow-ups to the streaming codec. JSONUnmarshalRead, not only BindJSONBody, now returns io.EOF when its input is nil, empty or only JSON white space, whichever codec is in use and without involving it. The helper is exported for generated code, and a handler that checks errors.Is(err, io.EOF) must not depend on how the registered codec reports an empty input: encoding/json's Decoder says io.EOF, json.Unmarshal says "unexpected end of JSON input", and v2 says something else again. The bytes read to find the first non-white-space byte are replayed in front of the reader, so the codec sees the input unchanged and a syntax error's offset is what it was before. SetJSONCodec(StdJSONCodec{}) restores the default, as nil does. StdJSONCodec is the default's byte half, and passing it reads as "encoding/json", not as a request for the buffered stream path with its missing trailing newline and trailing-data rejection. The StdJSONCodec doc now spells out those two differences for codecs that embed it. Document the codec contract: it must be re-entrant, since the MarshalJSON methods in runtime/types and in generated union and additionalProperties types call back into it from inside its own Marshal; SetJSONCodec is for a program's main package, being process-wide and unsynchronised; and the codec doesn't reach JSON a web framework's binder or JSON method handles, nor a validated body bound without decoding it again. Fix two stale comments, and test a read error, the empty-input contract under nil, byte-only and streaming codecs, and the default path with leading white space. Co-Authored-By: Claude Fable 5.1 --- bindjson.go | 43 ++++---------------- internal/jsoncodec/jsoncodec.go | 41 ++++++++++++++++++-- jsoncodec.go | 34 +++++++++++++--- jsoncodec_test.go | 69 +++++++++++++++++++++++++++++++-- 4 files changed, 140 insertions(+), 47 deletions(-) diff --git a/bindjson.go b/bindjson.go index 6901011..6187d4d 100644 --- a/bindjson.go +++ b/bindjson.go @@ -1,7 +1,6 @@ package runtime import ( - "bytes" "encoding" "encoding/base64" "encoding/json" @@ -33,50 +32,24 @@ type validatedBody interface { // case-insensitively, and the body is not decoded a second time. // // Without a validated value, or when the value cannot be bound into v, the -// body is decoded with the current JSONCodec (see SetJSONCodec), as a stream -// when it implements StreamingJSONCodec. With the default codec that is -// encoding/json's Decoder, so its error behaviour is unchanged. A body that is -// empty, or only white space, is io.EOF whichever codec is in use, as the -// Decoder reports it: generated handlers take that as an optional body left -// out. +// body is decoded with JSONUnmarshalRead: the current JSONCodec (see +// SetJSONCodec), as a stream when it implements StreamingJSONCodec. With the +// default codec that is encoding/json's Decoder, so its error behaviour is +// unchanged. A body that is empty, or only white space, is io.EOF whichever +// codec is in use: generated handlers take that as an optional body left out. func BindJSONBody(r *http.Request, v any) error { + var body io.Reader if r != nil && r.Body != nil { if b, ok := r.Body.(validatedBody); ok { if err := BindJSONValue(b.ValidatedValue(), v); err == nil { return nil } } - } - body, err := nonEmptyBody(r.Body) - if err != nil { - return err + body = r.Body } return JSONUnmarshalRead(body, v) } -// nonEmptyBody returns r positioned at its first byte that isn't JSON white -// space, or io.EOF when it has none, so that an empty body is io.EOF whichever -// codec decodes it. -func nonEmptyBody(r io.Reader) (io.Reader, error) { - if r == nil { - return nil, io.EOF - } - var b [1]byte - for { - n, err := r.Read(b[:]) - if n == 1 { - switch b[0] { - case ' ', '\t', '\r', '\n': - continue - } - return io.MultiReader(bytes.NewReader(b[:]), r), nil - } - if err != nil { - return nil, err - } - } -} - // BindJSONValue binds a decoded JSON value into v, which must be a non-nil // pointer. value is the generic representation produced by decoding JSON into // an `any`: nil, bool, string, json.Number (or a Go numeric type), []any and @@ -113,7 +86,7 @@ var ( ) // errUnsupportedJSONDestination is returned for destination types the binder -// does not handle; BindJSONBody then falls back to encoding/json. +// does not handle; BindJSONBody then decodes the body with the JSONCodec. var errUnsupportedJSONDestination = errors.New("unsupported destination type") func bindJSONValue(value any, dst reflect.Value) error { diff --git a/internal/jsoncodec/jsoncodec.go b/internal/jsoncodec/jsoncodec.go index 39077ce..9ba34a2 100644 --- a/internal/jsoncodec/jsoncodec.go +++ b/internal/jsoncodec/jsoncodec.go @@ -7,6 +7,7 @@ package jsoncodec import ( + "bytes" "encoding/json" "io" ) @@ -47,9 +48,11 @@ var _ Streaming = std{} var codec Codec = std{} -// Set replaces the codec. nil restores the default. +// Set replaces the codec. nil restores the default, and so does Std itself: +// it is the default's byte half, and passing it is the natural way to say +// "encoding/json", not a request for the buffered stream path. func Set(c Codec) { - if c == nil { + if c == nil || c == (Std{}) { c = std{} } codec = c @@ -77,8 +80,14 @@ func MarshalWrite(w io.Writer, v any) error { // UnmarshalRead decodes v from r: with the codec's UnmarshalRead when it // implements Streaming, otherwise by reading r to its end and calling -// Unmarshal. +// Unmarshal. When r is nil or holds no value, nothing or only JSON white +// space, it returns io.EOF without involving the codec, whose own report of +// an empty input varies. func UnmarshalRead(r io.Reader, v any) error { + r, err := nonEmpty(r) + if err != nil { + return err + } if s, ok := codec.(Streaming); ok { return s.UnmarshalRead(r, v) } @@ -88,3 +97,29 @@ func UnmarshalRead(r io.Reader, v any) error { } return codec.Unmarshal(data, v) } + +// nonEmpty returns io.EOF when r is nil or has only JSON white space before +// its end, and otherwise r with the bytes read to find that out replayed in +// front of it, so that the codec sees the input unchanged, a syntax error's +// offset included. +func nonEmpty(r io.Reader) (io.Reader, error) { + if r == nil { + return nil, io.EOF + } + var read []byte + for { + var b [1]byte + n, err := r.Read(b[:]) + if n == 1 { + read = append(read, b[0]) + switch b[0] { + case ' ', '\t', '\r', '\n': + continue + } + return io.MultiReader(bytes.NewReader(read), r), nil + } + if err != nil { + return nil, err + } + } +} diff --git a/jsoncodec.go b/jsoncodec.go index 0199b53..3595e91 100644 --- a/jsoncodec.go +++ b/jsoncodec.go @@ -17,6 +17,12 @@ import ( // HTML, not a different format. A codec that also implements // StreamingJSONCodec encodes and decodes request and response bodies as // streams. +// +// A codec must be re-entrant. The MarshalJSON and UnmarshalJSON methods of +// the types in runtime/types, and of the union and additionalProperties +// types oapi-codegen generates, call back into the codec from inside the +// codec's own Marshal or Unmarshal of a value that contains them. A codec +// that holds a lock across Marshal deadlocks on the first such value. type JSONCodec = jsoncodec.Codec // StreamingJSONCodec is optionally implemented by a JSONCodec that encodes to @@ -30,20 +36,33 @@ type JSONCodec = jsoncodec.Codec // newline, and whether UnmarshalRead rejects data after the value, is up to // the implementation: encoding/json's Encoder and Decoder do and don't, // respectively. Neither this package nor generated code relies on either. +// UnmarshalRead is never handed an empty input: see JSONUnmarshalRead. type StreamingJSONCodec = jsoncodec.Streaming // StdJSONCodec is encoding/json's json.Marshal and json.Unmarshal. Embed it in // a codec to replace one method. // // It doesn't implement StreamingJSONCodec, so a codec that embeds it handles -// bodies through its own Marshal and Unmarshal. The default codec, which -// SetJSONCodec(nil) restores, adds encoding/json's Encoder and Decoder for -// them. +// bodies through its own Marshal and Unmarshal, buffered. That differs from +// the default in two ways: a response body has no trailing newline, and data +// after a request body's value is rejected where encoding/json's Decoder +// ignores it. SetJSONCodec treats StdJSONCodec itself as the default, so +// passing it doesn't select the buffered path. type StdJSONCodec = jsoncodec.Std // SetJSONCodec replaces the codec used by this package and by the code -// oapi-codegen generates, process-wide. Call it during initialisation; it is -// not synchronised. nil restores the default. +// oapi-codegen generates, process-wide. It is for a program's main package to +// call during initialisation, not for a library: it is not synchronised, and +// it changes the JSON of every generated package linked into the program, +// including those of other libraries. nil restores the default, encoding/json +// throughout, and so does StdJSONCodec. +// +// The codec covers what this package and generated code encode and decode +// themselves. It doesn't reach JSON that a web framework handles on their +// behalf: the strict servers for echo, gin, fiber and iris decode request +// bodies with the framework's binder, fiber's and iris's write responses with +// the framework's JSON method, and a request body a validation middleware +// decoded and kept (see BindJSONBody) is bound without decoding it again. func SetJSONCodec(c JSONCodec) { jsoncodec.Set(c) } // JSONMarshal encodes v with the current JSONCodec. @@ -60,4 +79,9 @@ func JSONMarshalWrite(w io.Writer, v any) error { return jsoncodec.MarshalWrite( // JSONUnmarshalRead decodes v from r with the current JSONCodec: with its // UnmarshalRead when it implements StreamingJSONCodec, otherwise by reading r // to its end and calling Unmarshal. +// +// When r is nil or holds no value, nothing or only JSON white space, it +// returns io.EOF whichever codec is in use, as encoding/json's Decoder does, +// and the codec isn't involved: generated handlers take io.EOF as an optional +// request body left out. Otherwise the codec sees r's input unchanged. func JSONUnmarshalRead(r io.Reader, v any) error { return jsoncodec.UnmarshalRead(r, v) } diff --git a/jsoncodec_test.go b/jsoncodec_test.go index 5016e19..726e4b3 100644 --- a/jsoncodec_test.go +++ b/jsoncodec_test.go @@ -3,6 +3,7 @@ package runtime import ( "bytes" "encoding/json" + "errors" "io" "net/http" "strings" @@ -14,7 +15,8 @@ import ( "github.com/stretchr/testify/require" ) -// noEscapeCodec overrides Marshal only, and counts the calls that reach it. +// noEscapeCodec overrides Marshal to leave HTML unescaped, and counts the +// calls that reach either method. type noEscapeCodec struct { StdJSONCodec marshals, unmarshals *int @@ -57,6 +59,11 @@ func (c streamingCodec) UnmarshalRead(r io.Reader, v any) error { var _ StreamingJSONCodec = streamingCodec{} +// errReader fails every Read with err. +type errReader struct{ err error } + +func (r errReader) Read([]byte) (int, error) { return 0, r.err } + // TestSetJSONCodec mutates the process-wide codec, so it must not run in // parallel with other tests (they run after all sequential tests finish). func TestSetJSONCodec(t *testing.T) { @@ -113,12 +120,14 @@ func TestJSONCodec_Streams(t *testing.T) { const in = "" + // encoded is in as the default encodes a body: HTML escaped, trailing newline. + var encoded bytes.Buffer + require.NoError(t, json.NewEncoder(&encoded).Encode(in)) + t.Run("default is encoding/json's Encoder and Decoder", func(t *testing.T) { var buf bytes.Buffer require.NoError(t, JSONMarshalWrite(&buf, in)) - var want bytes.Buffer - require.NoError(t, json.NewEncoder(&want).Encode(in)) - assert.Equal(t, want.String(), buf.String(), "HTML escaped, trailing newline") + assert.Equal(t, encoded.String(), buf.String()) var out string require.NoError(t, JSONUnmarshalRead(strings.NewReader(`"x"`), &out)) @@ -126,6 +135,13 @@ func TestJSONCodec_Streams(t *testing.T) { assert.ErrorIs(t, JSONUnmarshalRead(strings.NewReader(""), &out), io.EOF) }) + t.Run("StdJSONCodec is the default too", func(t *testing.T) { + SetJSONCodec(StdJSONCodec{}) + var buf bytes.Buffer + require.NoError(t, JSONMarshalWrite(&buf, in)) + assert.Equal(t, encoded.String(), buf.String(), "not the buffered path, which has no newline") + }) + t.Run("StdJSONCodec itself doesn't stream", func(t *testing.T) { _, ok := any(StdJSONCodec{}).(StreamingJSONCodec) assert.False(t, ok, "embedding it must not promote stream methods past an overridden Marshal") @@ -169,6 +185,50 @@ func TestJSONCodec_Streams(t *testing.T) { }) } +func TestJSONUnmarshalRead_EmptyInput(t *testing.T) { + t.Cleanup(func() { SetJSONCodec(nil) }) + + t.Run("no value is io.EOF whichever codec is in use", func(t *testing.T) { + var unmarshals, reads int + for _, codec := range []JSONCodec{ + nil, + noEscapeCodec{marshals: new(int), unmarshals: &unmarshals}, + streamingCodec{noEscapeCodec: noEscapeCodec{marshals: new(int), unmarshals: &unmarshals}, writes: new(int), reads: &reads}, + } { + SetJSONCodec(codec) + var out string + assert.ErrorIs(t, JSONUnmarshalRead(nil, &out), io.EOF) + assert.ErrorIs(t, JSONUnmarshalRead(strings.NewReader(""), &out), io.EOF) + assert.ErrorIs(t, JSONUnmarshalRead(strings.NewReader(" \r\n\t"), &out), io.EOF) + } + assert.Equal(t, 0, unmarshals, "the codec never saw the empty inputs") + assert.Equal(t, 0, reads) + }) + + t.Run("a read error is returned", func(t *testing.T) { + SetJSONCodec(nil) + readErr := errors.New("read failed") + var out string + assert.ErrorIs(t, JSONUnmarshalRead(errReader{readErr}, &out), readErr) + }) + + t.Run("the input reaches the codec unchanged", func(t *testing.T) { + SetJSONCodec(nil) + const in = " {x" + var want, got map[string]any + wantErr := json.NewDecoder(strings.NewReader(in)).Decode(&want) + gotErr := JSONUnmarshalRead(strings.NewReader(in), &got) + var wantSyntax, gotSyntax *json.SyntaxError + require.ErrorAs(t, wantErr, &wantSyntax) + require.ErrorAs(t, gotErr, &gotSyntax) + assert.Equal(t, wantSyntax.Offset, gotSyntax.Offset, "leading white space is replayed, not dropped") + assert.Equal(t, wantErr.Error(), gotErr.Error()) + + require.NoError(t, JSONUnmarshalRead(strings.NewReader("\n\t {\"a\":1}"), &got)) + assert.Equal(t, map[string]any{"a": 1.0}, got) + }) +} + func TestBindJSONBody_Codec(t *testing.T) { t.Cleanup(func() { SetJSONCodec(nil) }) @@ -193,6 +253,7 @@ func TestBindJSONBody_Codec(t *testing.T) { assert.ErrorIs(t, BindJSONBody(newRequest(""), &got), io.EOF) assert.ErrorIs(t, BindJSONBody(newRequest(" \r\n\t"), &got), io.EOF) assert.ErrorIs(t, BindJSONBody(&http.Request{}, &got), io.EOF) + assert.ErrorIs(t, BindJSONBody(nil, &got), io.EOF) assert.Equal(t, 1, unmarshals, "the codec never saw the empty bodies") }) }