From fbc1dd74de9071cda0b7f65c5e4c8dde3314a274 Mon Sep 17 00:00:00 2001 From: Marcin Romaszewicz Date: Tue, 24 Feb 2026 17:43:36 -0800 Subject: [PATCH] fix: add Type/Format-aware parameter binding and styling for []byte (#97) When an OpenAPI spec uses type: string, format: byte, the generated Go code produces *[]byte fields. Previously the runtime treated []byte as a generic []uint8 slice -- splitting on commas and parsing individual integers -- instead of base64-encoding/decoding it as a single value. This commit adds Type and Format string fields (matching the OpenAPI spec type/format) to the Options structs for parameter binding and styling functions. When Format is "byte" and the destination is []byte, the runtime base64-encodes (styling) or base64-decodes (binding) the value as a single string. New WithOptions functions and options structs: - BindStringToObjectWithOptions + BindStringToObjectOptions - StyleParamWithOptions + StyleParamOptions - BindQueryParameterWithOptions + BindQueryParameterOptions Extended existing options struct: - BindStyledParameterOptions: added Type and Format fields Existing functions delegate to WithOptions with zero-value options, preserving backward compatibility for all current callers. Encoding uses base64.StdEncoding (standard alphabet, padded) per OpenAPI 3.0 / RFC 4648 Section 4. Decoding is lenient: it inspects padding and URL-safe characters to select the correct decoder, avoiding the silent corruption that occurs when RawStdEncoding accepts padded input. Co-Authored-By: Claude Opus 4.6 --- bindparam.go | 64 +++++++++++++++++++++++++++++++- bindparam_test.go | 89 +++++++++++++++++++++++++++++++++++++++++++++ bindstring.go | 27 ++++++++++++++ bindstring_test.go | 48 ++++++++++++++++++++++++ paramformat.go | 52 ++++++++++++++++++++++++++ paramformat_test.go | 80 ++++++++++++++++++++++++++++++++++++++++ styleparam.go | 42 +++++++++++++++++---- styleparam_test.go | 36 ++++++++++++++++++ 8 files changed, 428 insertions(+), 10 deletions(-) create mode 100644 paramformat.go create mode 100644 paramformat_test.go diff --git a/bindparam.go b/bindparam.go index 435caa4..922ecec 100644 --- a/bindparam.go +++ b/bindparam.go @@ -63,6 +63,12 @@ type BindStyledParameterOptions struct { Explode bool // Whether the parameter is required in the query Required bool + // Type is the OpenAPI type of the parameter (e.g. "string", "integer"). + Type string + // Format is the OpenAPI format of the parameter (e.g. "byte", "date-time"). + // When set to "byte" and the destination is []byte, the value is + // base64-decoded rather than treated as a generic slice. + Format string } // BindStyledParameterWithOptions binds a parameter as described in the Path Parameters @@ -121,6 +127,22 @@ func BindStyledParameterWithOptions(style string, paramName string, value string } if t.Kind() == reflect.Slice { + if opts.Format == "byte" && isByteSlice(t) { + parts, err := splitStyledParameter(style, opts.Explode, false, paramName, value) + if err != nil { + return fmt.Errorf("error splitting input '%s' into parts: %w", value, err) + } + if len(parts) != 1 { + return fmt.Errorf("expected single base64 value for byte slice parameter '%s', got %d parts", paramName, len(parts)) + } + decoded, err := base64Decode(parts[0]) + if err != nil { + return fmt.Errorf("error decoding base64 parameter '%s': %w", paramName, err) + } + v.SetBytes(decoded) + return nil + } + // Chop up the parameter into parts based on its style parts, err := splitStyledParameter(style, opts.Explode, false, paramName, value) if err != nil { @@ -308,6 +330,22 @@ func bindSplitPartsToDestinationStruct(paramName string, parts []string, explode // the Content parameter form. func BindQueryParameter(style string, explode bool, required bool, paramName string, queryParams url.Values, dest interface{}) error { + return BindQueryParameterWithOptions(style, explode, required, paramName, queryParams, dest, BindQueryParameterOptions{}) +} + +// BindQueryParameterOptions defines optional arguments for BindQueryParameterWithOptions. +type BindQueryParameterOptions struct { + // Type is the OpenAPI type of the parameter (e.g. "string", "integer"). + Type string + // Format is the OpenAPI format of the parameter (e.g. "byte", "date-time"). + // When set to "byte" and the destination is []byte, the value is + // base64-decoded rather than treated as a generic slice. + Format string +} + +// BindQueryParameterWithOptions works like BindQueryParameter with additional options. +func BindQueryParameterWithOptions(style string, explode bool, required bool, paramName string, + queryParams url.Values, dest interface{}, opts BindQueryParameterOptions) error { // dv = destination value. dv := reflect.Indirect(reflect.ValueOf(dest)) @@ -378,7 +416,18 @@ func BindQueryParameter(style string, explode bool, required bool, paramName str return nil } } - err = bindSplitPartsToDestinationArray(values, output) + if opts.Format == "byte" && isByteSlice(t) { + if len(values) != 1 { + return fmt.Errorf("expected single base64 value for byte slice parameter '%s', got %d values", paramName, len(values)) + } + decoded, decErr := base64Decode(values[0]) + if decErr != nil { + return fmt.Errorf("error decoding base64 parameter '%s': %w", paramName, decErr) + } + v.SetBytes(decoded) + } else { + err = bindSplitPartsToDestinationArray(values, output) + } case reflect.Struct: // This case is really annoying, and error prone, but the // form style object binding doesn't tell us which arguments @@ -442,7 +491,18 @@ func BindQueryParameter(style string, explode bool, required bool, paramName str var err error switch k { case reflect.Slice: - err = bindSplitPartsToDestinationArray(parts, output) + if opts.Format == "byte" && isByteSlice(t) { + // For non-exploded form, the value was split on commas above. + // Rejoin to get the original base64 string. + raw := strings.Join(parts, ",") + decoded, decErr := base64Decode(raw) + if decErr != nil { + return fmt.Errorf("error decoding base64 parameter '%s': %w", paramName, decErr) + } + v.SetBytes(decoded) + } else { + err = bindSplitPartsToDestinationArray(parts, output) + } case reflect.Struct: err = bindSplitPartsToDestinationStruct(paramName, parts, explode, output) default: diff --git a/bindparam_test.go b/bindparam_test.go index 079332f..036f2b5 100644 --- a/bindparam_test.go +++ b/bindparam_test.go @@ -27,6 +27,95 @@ import ( "github.com/oapi-codegen/runtime/types" ) +// TestBindStyledParameter_ByteSlice tests that BindStyledParameterWithOptions +// correctly handles *[]byte destinations by base64-decoding the parameter value, +// rather than treating []byte as a generic slice and splitting on commas. +// See: https://github.com/oapi-codegen/runtime/issues/97 +func TestBindStyledParameter_ByteSlice(t *testing.T) { + expected := []byte("test") + + tests := []struct { + name string + style string + explode bool + value string + }{ + {"simple/no-explode", "simple", false, "dGVzdA=="}, + {"simple/explode", "simple", true, "dGVzdA=="}, + {"label/no-explode", "label", false, ".dGVzdA=="}, + {"label/explode", "label", true, ".dGVzdA=="}, + {"matrix/no-explode", "matrix", false, ";data=dGVzdA=="}, + {"matrix/explode", "matrix", true, ";data=dGVzdA=="}, + {"form/no-explode", "form", false, "data=dGVzdA=="}, + {"form/explode", "form", true, "data=dGVzdA=="}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var dest []byte + err := BindStyledParameterWithOptions(tc.style, "data", tc.value, &dest, BindStyledParameterOptions{ + ParamLocation: ParamLocationUndefined, + Explode: tc.explode, + Required: true, + Type: "string", + Format: "byte", + }) + require.NoError(t, err) + assert.Equal(t, expected, dest) + }) + } +} + +// TestBindQueryParameter_ByteSlice tests that BindQueryParameter correctly +// handles *[]byte destinations by base64-decoding the query parameter value. +// See: https://github.com/oapi-codegen/runtime/issues/97 +func TestBindQueryParameter_ByteSlice(t *testing.T) { + expected := []byte("test") + + opts := BindQueryParameterOptions{Type: "string", Format: "byte"} + + t.Run("form/explode/required", func(t *testing.T) { + var dest []byte + queryParams := url.Values{"data": {"dGVzdA=="}} + err := BindQueryParameterWithOptions("form", true, true, "data", queryParams, &dest, opts) + require.NoError(t, err) + assert.Equal(t, expected, dest) + }) + + t.Run("form/no-explode/required", func(t *testing.T) { + var dest []byte + queryParams := url.Values{"data": {"dGVzdA=="}} + err := BindQueryParameterWithOptions("form", false, true, "data", queryParams, &dest, opts) + require.NoError(t, err) + assert.Equal(t, expected, dest) + }) + + t.Run("form/explode/optional/present", func(t *testing.T) { + var dest *[]byte + queryParams := url.Values{"data": {"dGVzdA=="}} + err := BindQueryParameterWithOptions("form", true, false, "data", queryParams, &dest, opts) + require.NoError(t, err) + require.NotNil(t, dest) + assert.Equal(t, expected, *dest) + }) + + t.Run("form/explode/optional/absent", func(t *testing.T) { + var dest *[]byte + queryParams := url.Values{} + err := BindQueryParameterWithOptions("form", true, false, "data", queryParams, &dest, opts) + require.NoError(t, err) + assert.Nil(t, dest) + }) + + t.Run("form/explode/optional/empty", func(t *testing.T) { + var dest []byte + queryParams := url.Values{"data": {""}} + err := BindQueryParameterWithOptions("form", true, false, "data", queryParams, &dest, opts) + require.NoError(t, err) + assert.Equal(t, []byte{}, dest) + }) +} + // MockBinder is just an independent version of Binder that has the Bind implemented type MockBinder struct { time.Time diff --git a/bindstring.go b/bindstring.go index 764036d..f38cf84 100644 --- a/bindstring.go +++ b/bindstring.go @@ -31,6 +31,22 @@ import ( // know the destination type each place that we use this, is to generate code // to read each specific type. func BindStringToObject(src string, dst interface{}) error { + return BindStringToObjectWithOptions(src, dst, BindStringToObjectOptions{}) +} + +// BindStringToObjectOptions defines optional arguments for BindStringToObjectWithOptions. +type BindStringToObjectOptions struct { + // Type is the OpenAPI type of the parameter (e.g. "string", "integer"). + Type string + // Format is the OpenAPI format of the parameter (e.g. "byte", "date-time"). + // When set to "byte" and the destination is []byte, the source string is + // base64-decoded rather than treated as a generic slice. + Format string +} + +// BindStringToObjectWithOptions takes a string, and attempts to assign it to the destination +// interface via whatever type conversion is necessary, with additional options. +func BindStringToObjectWithOptions(src string, dst interface{}, opts BindStringToObjectOptions) error { var err error v := reflect.ValueOf(dst) @@ -59,6 +75,17 @@ func BindStringToObject(src string, dst interface{}) error { } switch t.Kind() { + case reflect.Slice: + if opts.Format == "byte" && isByteSlice(t) { + decoded, decErr := base64Decode(src) + if decErr != nil { + return fmt.Errorf("error binding string parameter: %w", decErr) + } + v.SetBytes(decoded) + return nil + } + // Non-binary slices fall through to the default error case. + fallthrough case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: var val int64 val, err = strconv.ParseInt(src, 10, 64) diff --git a/bindstring_test.go b/bindstring_test.go index d3c4909..043917b 100644 --- a/bindstring_test.go +++ b/bindstring_test.go @@ -14,12 +14,14 @@ package runtime import ( + "encoding/base64" "fmt" "math" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/oapi-codegen/runtime/types" ) @@ -210,3 +212,49 @@ func TestBindStringToObject(t *testing.T) { assert.Equal(t, dstUUID.String(), uuidString) } + +// TestBindStringToObject_ByteSlice tests that BindStringToObject correctly handles +// *[]byte destinations by base64-decoding the input string, rather than failing +// or treating []byte as a generic slice. +// See: https://github.com/oapi-codegen/runtime/issues/97 +func TestBindStringToObject_ByteSlice(t *testing.T) { + opts := BindStringToObjectOptions{Type: "string", Format: "byte"} + + t.Run("valid base64 with padding", func(t *testing.T) { + var dest []byte + err := BindStringToObjectWithOptions("dGVzdA==", &dest, opts) + require.NoError(t, err) + assert.Equal(t, []byte("test"), dest) + }) + + t.Run("valid base64 without padding", func(t *testing.T) { + var dest []byte + err := BindStringToObjectWithOptions("dGVzdA", &dest, opts) + require.NoError(t, err) + assert.Equal(t, []byte("test"), dest) + }) + + t.Run("URL-safe base64", func(t *testing.T) { + // "<>+" in standard base64 is "PDw/Pz4+" but URL-safe uses "PDw_Pz4-" + input := "PDw_Pz4-" + var dest []byte + err := BindStringToObjectWithOptions(input, &dest, opts) + require.NoError(t, err) + expected, decErr := base64.RawURLEncoding.DecodeString("PDw_Pz4-") + require.NoError(t, decErr) + assert.Equal(t, expected, dest) + }) + + t.Run("empty string", func(t *testing.T) { + var dest []byte + err := BindStringToObjectWithOptions("", &dest, opts) + require.NoError(t, err) + assert.Equal(t, []byte{}, dest) + }) + + t.Run("invalid base64", func(t *testing.T) { + var dest []byte + err := BindStringToObjectWithOptions("!!!not-base64!!!", &dest, opts) + assert.Error(t, err) + }) +} diff --git a/paramformat.go b/paramformat.go new file mode 100644 index 0000000..0450593 --- /dev/null +++ b/paramformat.go @@ -0,0 +1,52 @@ +package runtime + +import ( + "encoding/base64" + "fmt" + "reflect" + "strings" +) + +// isByteSlice reports whether t is []byte (or equivalently []uint8). +func isByteSlice(t reflect.Type) bool { + return t.Kind() == reflect.Slice && t.Elem().Kind() == reflect.Uint8 +} + +// base64Decode decodes s as base64. +// +// Per OpenAPI 3.0, format: byte uses RFC 4648 Section 4 (standard alphabet, +// padded). We use padding presence to select the right decoder, rather than +// blindly cascading (which can produce corrupt output when RawStdEncoding +// silently accepts padded input and treats '=' as data). +// +// The logic: +// 1. If s contains '=' padding → standard padded decoder (Std or URL based on alphabet). +// 2. If s contains URL-safe characters ('_' or '-') → RawURLEncoding. +// 3. Otherwise → RawStdEncoding (unpadded, standard alphabet). +func base64Decode(s string) ([]byte, error) { + if s == "" { + return []byte{}, nil + } + + if strings.ContainsRune(s, '=') { + // Padded input. Pick alphabet based on whether URL-safe chars are present. + if strings.ContainsAny(s, "-_") { + return base64Decode1(base64.URLEncoding, s) + } + return base64Decode1(base64.StdEncoding, s) + } + + // Unpadded input. Pick alphabet based on whether URL-safe chars are present. + if strings.ContainsAny(s, "-_") { + return base64Decode1(base64.RawURLEncoding, s) + } + return base64Decode1(base64.RawStdEncoding, s) +} + +func base64Decode1(enc *base64.Encoding, s string) ([]byte, error) { + b, err := enc.DecodeString(s) + if err != nil { + return nil, fmt.Errorf("failed to base64-decode string %q: %w", s, err) + } + return b, nil +} diff --git a/paramformat_test.go b/paramformat_test.go new file mode 100644 index 0000000..a6354d4 --- /dev/null +++ b/paramformat_test.go @@ -0,0 +1,80 @@ +package runtime + +import ( + "encoding/base64" + "reflect" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestIsByteSlice(t *testing.T) { + assert.True(t, isByteSlice(reflect.TypeOf([]byte{}))) + assert.True(t, isByteSlice(reflect.TypeOf([]uint8{}))) + + assert.False(t, isByteSlice(reflect.TypeOf([]int{}))) + assert.False(t, isByteSlice(reflect.TypeOf([]string{}))) + assert.False(t, isByteSlice(reflect.TypeOf(""))) + assert.False(t, isByteSlice(reflect.TypeOf(0))) + + // Type alias for []byte should also be detected + type MyBytes []byte + assert.True(t, isByteSlice(reflect.TypeOf(MyBytes{}))) +} + +func TestBase64Decode(t *testing.T) { + t.Run("standard encoding with padding", func(t *testing.T) { + // "test" → StdEncoding → "dGVzdA==" + b, err := base64Decode("dGVzdA==") + require.NoError(t, err) + assert.Equal(t, []byte("test"), b) + }) + + t.Run("standard encoding without padding", func(t *testing.T) { + // "test" → RawStdEncoding → "dGVzdA" + b, err := base64Decode("dGVzdA") + require.NoError(t, err) + assert.Equal(t, []byte("test"), b) + }) + + t.Run("URL-safe encoding without padding", func(t *testing.T) { + // Contains '_' and '-' → dispatches to RawURLEncoding + input := "PDw_Pz4-" + b, err := base64Decode(input) + require.NoError(t, err) + expected, decErr := base64.RawURLEncoding.DecodeString(input) + require.NoError(t, decErr) + assert.Equal(t, expected, b) + }) + + t.Run("URL-safe encoding with padding", func(t *testing.T) { + // Contains '_' and '=' → dispatches to URLEncoding (padded) + input := base64.URLEncoding.EncodeToString([]byte("<>")) + b, err := base64Decode(input) + require.NoError(t, err) + assert.Equal(t, []byte("<>"), b) + }) + + t.Run("empty string", func(t *testing.T) { + b, err := base64Decode("") + require.NoError(t, err) + assert.Equal(t, []byte{}, b) + }) + + t.Run("invalid base64", func(t *testing.T) { + _, err := base64Decode("!!!not-base64!!!") + assert.Error(t, err) + }) + + t.Run("padded input not corrupted by wrong decoder", func(t *testing.T) { + // This is the key correctness test. With the old blind cascade, + // RawStdEncoding would "succeed" on padded input but produce + // garbage bytes (treating '=' as data). The new logic dispatches + // padded input to StdEncoding directly. + input := base64.StdEncoding.EncodeToString([]byte("hello world")) + b, err := base64Decode(input) + require.NoError(t, err) + assert.Equal(t, []byte("hello world"), b) + }) +} diff --git a/styleparam.go b/styleparam.go index 8491c85..389aa2a 100644 --- a/styleparam.go +++ b/styleparam.go @@ -16,6 +16,7 @@ package runtime import ( "bytes" "encoding" + "encoding/base64" "encoding/json" "errors" "fmt" @@ -51,10 +52,31 @@ func StyleParam(style string, explode bool, paramName string, value interface{}) return StyleParamWithLocation(style, explode, paramName, ParamLocationUndefined, value) } -// Given an input value, such as a primitive type, array or object, turn it -// into a parameter based on style/explode definition, performing whatever -// escaping is necessary based on parameter location +// StyleParamWithLocation serializes a Go value into an OpenAPI-styled parameter +// string, performing escaping based on parameter location. func StyleParamWithLocation(style string, explode bool, paramName string, paramLocation ParamLocation, value interface{}) (string, error) { + return StyleParamWithOptions(style, explode, paramName, value, StyleParamOptions{ + ParamLocation: paramLocation, + }) +} + +// StyleParamOptions defines optional arguments for StyleParamWithOptions. +type StyleParamOptions struct { + // ParamLocation controls URL escaping behavior. + ParamLocation ParamLocation + // Type is the OpenAPI type of the parameter (e.g. "string", "integer"). + Type string + // Format is the OpenAPI format of the parameter (e.g. "byte", "date-time"). + // When set to "byte" and the value is []byte, it is base64-encoded as a + // single string rather than treated as a generic slice of uint8. + Format string + // Required indicates whether the parameter is required. + Required bool +} + +// StyleParamWithOptions serializes a Go value into an OpenAPI-styled parameter +// string with additional options. +func StyleParamWithOptions(style string, explode bool, paramName string, value interface{}, opts StyleParamOptions) (string, error) { t := reflect.TypeOf(value) v := reflect.ValueOf(value) @@ -83,24 +105,28 @@ func StyleParamWithLocation(style string, explode bool, paramName string, paramL return "", fmt.Errorf("error marshaling '%s' as text: %w", value, err) } - return stylePrimitive(style, explode, paramName, paramLocation, string(b)) + return stylePrimitive(style, explode, paramName, opts.ParamLocation, string(b)) } } switch t.Kind() { case reflect.Slice: + if opts.Format == "byte" && isByteSlice(t) { + encoded := base64.StdEncoding.EncodeToString(v.Bytes()) + return stylePrimitive(style, explode, paramName, opts.ParamLocation, encoded) + } n := v.Len() sliceVal := make([]interface{}, n) for i := 0; i < n; i++ { sliceVal[i] = v.Index(i).Interface() } - return styleSlice(style, explode, paramName, paramLocation, sliceVal) + return styleSlice(style, explode, paramName, opts.ParamLocation, sliceVal) case reflect.Struct: - return styleStruct(style, explode, paramName, paramLocation, value) + return styleStruct(style, explode, paramName, opts.ParamLocation, value) case reflect.Map: - return styleMap(style, explode, paramName, paramLocation, value) + return styleMap(style, explode, paramName, opts.ParamLocation, value) default: - return stylePrimitive(style, explode, paramName, paramLocation, value) + return stylePrimitive(style, explode, paramName, opts.ParamLocation, value) } } diff --git a/styleparam_test.go b/styleparam_test.go index b980140..4d0f3fe 100644 --- a/styleparam_test.go +++ b/styleparam_test.go @@ -692,6 +692,42 @@ func TestStyleParam(t *testing.T) { } +// TestStyleParam_ByteSlice tests that StyleParamWithLocation correctly handles +// []byte values by base64-encoding them as a single string, rather than treating +// them as a generic slice of uint8 values. +// See: https://github.com/oapi-codegen/runtime/issues/97 +func TestStyleParam_ByteSlice(t *testing.T) { + input := []byte("test") + + tests := []struct { + name string + style string + explode bool + expected string + }{ + {"simple/no-explode", "simple", false, "dGVzdA%3D%3D"}, + {"simple/explode", "simple", true, "dGVzdA%3D%3D"}, + {"label/no-explode", "label", false, ".dGVzdA%3D%3D"}, + {"label/explode", "label", true, ".dGVzdA%3D%3D"}, + {"matrix/no-explode", "matrix", false, ";data=dGVzdA%3D%3D"}, + {"matrix/explode", "matrix", true, ";data=dGVzdA%3D%3D"}, + {"form/no-explode", "form", false, "data=dGVzdA%3D%3D"}, + {"form/explode", "form", true, "data=dGVzdA%3D%3D"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result, err := StyleParamWithOptions(tc.style, tc.explode, "data", input, StyleParamOptions{ + ParamLocation: ParamLocationQuery, + Type: "string", + Format: "byte", + }) + assert.NoError(t, err) + assert.EqualValues(t, tc.expected, result) + }) + } +} + // Issue 37 - https://github.com/oapi-codegen/runtime/issues/37 func TestIssue37(t *testing.T) { styles := []string{