Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 37 additions & 23 deletions bindparam.go
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,24 @@ func BindQueryParameterWithOptions(style string, explode bool, required bool, pa
if len(values) != 1 {
return fmt.Errorf("parameter '%s' is not exploded, but is specified multiple times", paramName)
}

// For primitive types, the raw value should be used as-is
// without splitting on commas. Per the OpenAPI specification,
// explode has no effect on primitive types — the serialization
// is the same regardless of the explode value. Comma splitting
// is only meaningful for array and object types.
// See: https://swagger.io/docs/specification/serialization/
if k != reflect.Slice && k != reflect.Struct && k != reflect.Map {
err := BindStringToObject(values[0], output)
if err != nil {
return err
}
if extraIndirect {
dv.Set(reflect.ValueOf(output))
}
return nil
}

switch style {
case "spaceDelimited":
parts = strings.Split(values[0], " ")
Expand Down Expand Up @@ -568,18 +586,6 @@ func BindQueryParameterWithOptions(style string, explode bool, required bool, pa
default:
err = bindSplitPartsToDestinationStruct(paramName, parts, explode, output)
}
default:
if len(parts) == 0 {
if required {
return &RequiredParameterError{ParamName: paramName}
} else {
return nil
}
}
if len(parts) != 1 {
return fmt.Errorf("multiple values for single value parameter '%s'", paramName)
}
err = BindStringToObject(parts[0], output)
}
if err != nil {
return err
Expand Down Expand Up @@ -745,6 +751,25 @@ func BindRawQueryParameter(style string, explode bool, required bool, paramName
return fmt.Errorf("parameter '%s' is not exploded, but is specified multiple times", paramName)
}

// For primitive types, decode the raw value as-is without splitting
// on commas. Per the OpenAPI specification, explode has no effect on
// primitive types. Comma splitting is only meaningful for array and
// object types.
if k != reflect.Slice && k != reflect.Struct && k != reflect.Map {
decoded, err := url.QueryUnescape(rawValues[0])
if err != nil {
return fmt.Errorf("error decoding query parameter '%s' value %q: %w", paramName, rawValues[0], err)
}
err = BindStringToObject(decoded, output)
if err != nil {
return err
}
if extraIndirect {
dv.Set(reflect.ValueOf(output))
}
return nil
}

var rawParts []string
switch style {
case "spaceDelimited":
Expand Down Expand Up @@ -773,17 +798,6 @@ func BindRawQueryParameter(style string, explode bool, required bool, paramName
err = bindSplitPartsToDestinationArray(parts, output)
case reflect.Struct:
err = bindSplitPartsToDestinationStruct(paramName, parts, explode, output)
default:
if len(parts) == 0 {
if required {
return &RequiredParameterError{ParamName: paramName}
}
return nil
}
if len(parts) != 1 {
return fmt.Errorf("multiple values for single value parameter '%s'", paramName)
}
err = BindStringToObject(parts[0], output)
}
if err != nil {
return err
Expand Down
42 changes: 42 additions & 0 deletions bindparam_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,30 @@ func TestBindQueryParameter(t *testing.T) {
assert.Equal(t, expectedDate, *date)
})

// Regression test: primitive string with explode=false should not be
// split on commas. Per the OpenAPI specification, explode has no effect
// on primitive types — the value must be bound as-is.
t.Run("string_form_no_explode_required_with_commas", func(t *testing.T) {
var scope string
queryParams := url.Values{
"scope": {"openid,profile,email"},
}
err := BindQueryParameter("form", false, true, "scope", queryParams, &scope)
assert.NoError(t, err)
assert.Equal(t, "openid,profile,email", scope)
})

t.Run("string_form_no_explode_optional_with_commas", func(t *testing.T) {
var scope *string
queryParams := url.Values{
"scope": {"openid,profile,email"},
}
err := BindQueryParameter("form", false, false, "scope", queryParams, &scope)
assert.NoError(t, err)
require.NotNil(t, scope)
assert.Equal(t, "openid,profile,email", *scope)
})

// time.Time has the same bug as types.Date for form/no-explode.
t.Run("time_form_no_explode_required", func(t *testing.T) {
expectedTime := time.Date(2020, 12, 9, 16, 9, 53, 0, time.UTC)
Expand Down Expand Up @@ -974,6 +998,24 @@ func TestBindRawQueryParameter(t *testing.T) {
assert.Equal(t, "red", *dest)
})

// Regression test: primitive string with explode=false should not be
// split on commas. Per the OpenAPI specification, explode has no effect
// on primitive types.
t.Run("string with commas required", func(t *testing.T) {
var dest string
err := BindRawQueryParameter("form", false, true, "scope", "scope=openid%2Cprofile%2Cemail", &dest)
require.NoError(t, err)
assert.Equal(t, "openid,profile,email", dest)
})

t.Run("string with commas optional", func(t *testing.T) {
var dest *string
err := BindRawQueryParameter("form", false, false, "scope", "scope=openid%2Cprofile%2Cemail", &dest)
require.NoError(t, err)
require.NotNil(t, dest)
assert.Equal(t, "openid,profile,email", *dest)
})

t.Run("duplicate param errors", func(t *testing.T) {
var dest []string
err := BindRawQueryParameter("form", false, true, "color", "color=red&color=blue", &dest)
Expand Down
Loading