diff --git a/bindparam.go b/bindparam.go index 435caa4..780f967 100644 --- a/bindparam.go +++ b/bindparam.go @@ -306,6 +306,11 @@ func bindSplitPartsToDestinationStruct(paramName string, parts []string, explode // tell them apart. This code tries to fail, but the moral of the story is that // you shouldn't pass objects via form styled query arguments, just use // the Content parameter form. +// +// Deprecated: BindQueryParameter pre-decodes the query string via url.Values, +// which makes it impossible to distinguish literal commas from delimiter commas +// in form/explode=false parameters. Use BindRawQueryParameter instead, which +// operates on the raw query string and handles encoded delimiters correctly. func BindQueryParameter(style string, explode bool, required bool, paramName string, queryParams url.Values, dest interface{}) error { @@ -478,6 +483,201 @@ func BindQueryParameter(style string, explode bool, required bool, paramName str } } +// findRawQueryParam extracts the raw (still-percent-encoded) values for a given +// parameter name from a raw query string, without URL-decoding the values. +// The parameter key is decoded for comparison purposes, but the returned values +// remain in their original encoded form. +func findRawQueryParam(rawQuery, paramName string) (values []string, found bool) { + for rawQuery != "" { + var part string + if i := strings.IndexByte(rawQuery, '&'); i >= 0 { + part = rawQuery[:i] + rawQuery = rawQuery[i+1:] + } else { + part = rawQuery + rawQuery = "" + } + if part == "" { + continue + } + key := part + var val string + if i := strings.IndexByte(part, '='); i >= 0 { + key = part[:i] + val = part[i+1:] + } + decodedKey, err := url.QueryUnescape(key) + if err != nil { + // Skip malformed keys. + continue + } + if decodedKey == paramName { + values = append(values, val) + found = true + } + } + return values, found +} + +// BindRawQueryParameter works like BindQueryParameter but operates on the raw +// (undecoded) query string instead of pre-parsed url.Values. This correctly +// handles form/explode=false parameters whose values contain literal commas +// encoded as %2C — something that BindQueryParameter cannot do because +// url.Values has already decoded %2C to ',' before we can split on the +// delimiter comma. +func BindRawQueryParameter(style string, explode bool, required bool, paramName string, + rawQuery string, dest any) error { + + // dv = destination value. + dv := reflect.Indirect(reflect.ValueOf(dest)) + + // intermediate value form which is either dv or dv dereferenced. + v := dv + + // inner code will bind the string's value to this interface. + var output any + + // required params are never pointers, but it may happen that optional param + // is not pointer as well if user decides to annotate it with + // x-go-type-skip-optional-pointer + var extraIndirect = !required && v.Kind() == reflect.Pointer + if !extraIndirect { + output = dest + } else { + if v.IsNil() { + t := v.Type() + newValue := reflect.New(t.Elem()) + output = newValue.Interface() + } else { + output = v.Interface() + } + v = reflect.Indirect(reflect.ValueOf(output)) + } + + // This is the basic type of the destination object. + t := v.Type() + k := t.Kind() + + switch style { + case "form": + if explode { + // For the explode case, url.ParseQuery is fine — there are no + // delimiter commas to confuse with literal commas. + queryParams, err := url.ParseQuery(rawQuery) + if err != nil { + return fmt.Errorf("error parsing query string: %w", err) + } + values, found := queryParams[paramName] + + switch k { + case reflect.Slice: + if !found { + if required { + return fmt.Errorf("query parameter '%s' is required", paramName) + } + return nil + } + err = bindSplitPartsToDestinationArray(values, output) + case reflect.Struct: + var fieldsPresent bool + fieldsPresent, err = bindParamsToExplodedObject(paramName, queryParams, output) + if !fieldsPresent { + return nil + } + default: + if len(values) == 0 { + if required { + return fmt.Errorf("query parameter '%s' is required", paramName) + } + return nil + } + if len(values) != 1 { + return fmt.Errorf("multiple values for single value parameter '%s'", paramName) + } + if !found { + if required { + return fmt.Errorf("query parameter '%s' is required", paramName) + } + return nil + } + err = BindStringToObject(values[0], output) + } + if err != nil { + return err + } + if extraIndirect { + dv.Set(reflect.ValueOf(output)) + } + return nil + } + + // form, explode=false — the core fix. + // Use findRawQueryParam to get the still-encoded value, split on + // literal ',' (which is the OpenAPI delimiter), then URL-decode + // each resulting part individually. + rawValues, found := findRawQueryParam(rawQuery, paramName) + if !found { + if required { + return fmt.Errorf("query parameter '%s' is required", paramName) + } + return nil + } + if len(rawValues) != 1 { + return fmt.Errorf("parameter '%s' is not exploded, but is specified multiple times", paramName) + } + + rawParts := strings.Split(rawValues[0], ",") + parts := make([]string, len(rawParts)) + for i, rp := range rawParts { + decoded, err := url.QueryUnescape(rp) + if err != nil { + return fmt.Errorf("error decoding query parameter '%s' part %q: %w", paramName, rp, err) + } + parts[i] = decoded + } + + var err error + switch k { + case reflect.Slice: + err = bindSplitPartsToDestinationArray(parts, output) + case reflect.Struct: + err = bindSplitPartsToDestinationStruct(paramName, parts, explode, output) + default: + if len(parts) == 0 { + if required { + return fmt.Errorf("query parameter '%s' is required", 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 + } + if extraIndirect { + dv.Set(reflect.ValueOf(output)) + } + return nil + + case "deepObject": + if !explode { + return errors.New("deepObjects must be exploded") + } + queryParams, err := url.ParseQuery(rawQuery) + if err != nil { + return fmt.Errorf("error parsing query string: %w", err) + } + return UnmarshalDeepObject(dest, paramName, queryParams) + case "spaceDelimited", "pipeDelimited": + return fmt.Errorf("query arguments of style '%s' aren't yet supported", style) + default: + return fmt.Errorf("style '%s' on parameter '%s' is invalid", style, paramName) + } +} + // bindParamsToExplodedObject reflects the destination structure, and pulls the value for // each settable field from the given parameters map. This is to deal with the // exploded form styled object which may occupy any number of parameter names. diff --git a/bindparam_test.go b/bindparam_test.go index eec3d3a..4099e44 100644 --- a/bindparam_test.go +++ b/bindparam_test.go @@ -18,6 +18,7 @@ import ( "fmt" "math/big" "net/url" + "reflect" "testing" "time" @@ -509,6 +510,392 @@ func TestBindParamsToExplodedObject(t *testing.T) { assert.EqualValues(t, &now, optDstTime.Time) } +func TestFindRawQueryParam(t *testing.T) { + tests := []struct { + name string + rawQuery string + paramName string + wantValues []string + wantFound bool + }{ + { + name: "simple value", + rawQuery: "color=red", + paramName: "color", + wantValues: []string{"red"}, + wantFound: true, + }, + { + name: "not found", + rawQuery: "color=red", + paramName: "size", + wantValues: nil, + wantFound: false, + }, + { + name: "empty query", + rawQuery: "", + paramName: "color", + wantValues: nil, + wantFound: false, + }, + { + name: "multiple values (exploded)", + rawQuery: "color=red&color=blue&color=green", + paramName: "color", + wantValues: []string{"red", "blue", "green"}, + wantFound: true, + }, + { + name: "comma in value stays encoded", + rawQuery: "color=a%2Cb", + paramName: "color", + wantValues: []string{"a%2Cb"}, + wantFound: true, + }, + { + name: "empty value", + rawQuery: "color=", + paramName: "color", + wantValues: []string{""}, + wantFound: true, + }, + { + name: "no equals sign", + rawQuery: "color", + paramName: "color", + wantValues: []string{""}, + wantFound: true, + }, + { + name: "encoded key", + rawQuery: "my%20color=red", + paramName: "my color", + wantValues: []string{"red"}, + wantFound: true, + }, + { + name: "mixed params", + rawQuery: "size=large&color=red&shape=round", + paramName: "color", + wantValues: []string{"red"}, + wantFound: true, + }, + { + name: "value with special chars", + rawQuery: "color=red%26blue%3Dgreen", + paramName: "color", + wantValues: []string{"red%26blue%3Dgreen"}, + wantFound: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + values, found := findRawQueryParam(tt.rawQuery, tt.paramName) + assert.Equal(t, tt.wantFound, found) + assert.Equal(t, tt.wantValues, values) + }) + } +} + +func TestBindRawQueryParameter(t *testing.T) { + type TestObject struct { + FirstName string `json:"firstName"` + Role string `json:"role"` + } + + t.Run("form/explode=false", func(t *testing.T) { + t.Run("string slice simple", func(t *testing.T) { + var dest []string + err := BindRawQueryParameter("form", false, true, "color", "color=red,green,blue", &dest) + require.NoError(t, err) + assert.Equal(t, []string{"red", "green", "blue"}, dest) + }) + + t.Run("string slice with comma in value", func(t *testing.T) { + var dest []string + err := BindRawQueryParameter("form", false, true, "color", "color=a,b,c%2Cd", &dest) + require.NoError(t, err) + assert.Equal(t, []string{"a", "b", "c,d"}, dest) + }) + + t.Run("string slice with multiple special chars", func(t *testing.T) { + var dest []string + err := BindRawQueryParameter("form", false, true, "color", "color=a%2Cb,c%26d,e%3Df", &dest) + require.NoError(t, err) + assert.Equal(t, []string{"a,b", "c&d", "e=f"}, dest) + }) + + t.Run("int slice", func(t *testing.T) { + var dest []int + err := BindRawQueryParameter("form", false, true, "ids", "ids=1,2,3", &dest) + require.NoError(t, err) + assert.Equal(t, []int{1, 2, 3}, dest) + }) + + t.Run("primitive string", func(t *testing.T) { + var dest string + err := BindRawQueryParameter("form", false, true, "color", "color=red", &dest) + require.NoError(t, err) + assert.Equal(t, "red", dest) + }) + + t.Run("primitive int", func(t *testing.T) { + var dest int + err := BindRawQueryParameter("form", false, true, "count", "count=42", &dest) + require.NoError(t, err) + assert.Equal(t, 42, dest) + }) + + t.Run("struct (object)", func(t *testing.T) { + var dest TestObject + err := BindRawQueryParameter("form", false, true, "id", "id=firstName,Alex,role,admin", &dest) + require.NoError(t, err) + assert.Equal(t, TestObject{FirstName: "Alex", Role: "admin"}, dest) + }) + + t.Run("struct with encoded comma in value", func(t *testing.T) { + var dest TestObject + err := BindRawQueryParameter("form", false, true, "id", "id=firstName,Alex%2CBob,role,admin", &dest) + require.NoError(t, err) + assert.Equal(t, TestObject{FirstName: "Alex,Bob", Role: "admin"}, dest) + }) + + t.Run("required missing", func(t *testing.T) { + var dest []string + err := BindRawQueryParameter("form", false, true, "color", "other=red", &dest) + assert.Error(t, err) + assert.Contains(t, err.Error(), "required") + }) + + t.Run("optional missing returns nil", func(t *testing.T) { + var dest *[]string + err := BindRawQueryParameter("form", false, false, "color", "other=red", &dest) + require.NoError(t, err) + assert.Nil(t, dest) + }) + + t.Run("optional present is populated", func(t *testing.T) { + var dest *string + err := BindRawQueryParameter("form", false, false, "color", "color=red", &dest) + require.NoError(t, err) + require.NotNil(t, dest) + assert.Equal(t, "red", *dest) + }) + + t.Run("duplicate param errors", func(t *testing.T) { + var dest []string + err := BindRawQueryParameter("form", false, true, "color", "color=red&color=blue", &dest) + assert.Error(t, err) + assert.Contains(t, err.Error(), "not exploded") + }) + }) + + t.Run("form/explode=true", func(t *testing.T) { + t.Run("string slice", func(t *testing.T) { + var dest []string + err := BindRawQueryParameter("form", true, true, "color", "color=red&color=green&color=blue", &dest) + require.NoError(t, err) + assert.Equal(t, []string{"red", "green", "blue"}, dest) + }) + + t.Run("int slice", func(t *testing.T) { + var dest []int + err := BindRawQueryParameter("form", true, true, "ids", "ids=1&ids=2&ids=3", &dest) + require.NoError(t, err) + assert.Equal(t, []int{1, 2, 3}, dest) + }) + + t.Run("primitive", func(t *testing.T) { + var dest string + err := BindRawQueryParameter("form", true, true, "color", "color=red", &dest) + require.NoError(t, err) + assert.Equal(t, "red", dest) + }) + + t.Run("struct", func(t *testing.T) { + var dest TestObject + err := BindRawQueryParameter("form", true, true, "id", "firstName=Alex&role=admin", &dest) + require.NoError(t, err) + assert.Equal(t, TestObject{FirstName: "Alex", Role: "admin"}, dest) + }) + + t.Run("required missing", func(t *testing.T) { + var dest []string + err := BindRawQueryParameter("form", true, true, "color", "other=red", &dest) + assert.Error(t, err) + }) + + t.Run("optional missing", func(t *testing.T) { + var dest *string + err := BindRawQueryParameter("form", true, false, "color", "other=red", &dest) + require.NoError(t, err) + assert.Nil(t, dest) + }) + }) + + t.Run("deepObject/explode=true", func(t *testing.T) { + type ID struct { + FirstName *string `json:"firstName"` + Role string `json:"role"` + } + var dest ID + err := BindRawQueryParameter("deepObject", true, false, "id", "id%5BfirstName%5D=Alex&id%5Brole%5D=admin", &dest) + require.NoError(t, err) + expectedName := "Alex" + assert.Equal(t, ID{FirstName: &expectedName, Role: "admin"}, dest) + }) + + t.Run("error cases", func(t *testing.T) { + t.Run("deepObject explode=false", func(t *testing.T) { + var dest TestObject + err := BindRawQueryParameter("deepObject", false, true, "id", "id=foo", &dest) + assert.Error(t, err) + assert.Contains(t, err.Error(), "exploded") + }) + + t.Run("spaceDelimited", func(t *testing.T) { + var dest []string + err := BindRawQueryParameter("spaceDelimited", false, true, "color", "color=a%20b%20c", &dest) + assert.Error(t, err) + assert.Contains(t, err.Error(), "spaceDelimited") + }) + + t.Run("pipeDelimited", func(t *testing.T) { + var dest []string + err := BindRawQueryParameter("pipeDelimited", false, true, "color", "color=a|b|c", &dest) + assert.Error(t, err) + assert.Contains(t, err.Error(), "pipeDelimited") + }) + + t.Run("unknown style", func(t *testing.T) { + var dest string + err := BindRawQueryParameter("unknown", false, true, "color", "color=red", &dest) + assert.Error(t, err) + assert.Contains(t, err.Error(), "invalid") + }) + }) +} + +func TestRoundTripQueryParameter(t *testing.T) { + type TestObject struct { + FirstName string `json:"firstName"` + Role string `json:"role"` + } + + tests := []struct { + name string + style string + explode bool + paramName string + value interface{} + dest interface{} // pointer to zero value of dest type + expected interface{} // expected value after round-trip + }{ + { + name: "form/false string slice", + style: "form", + explode: false, + paramName: "color", + value: []string{"red", "green", "blue"}, + dest: &[]string{}, + expected: []string{"red", "green", "blue"}, + }, + { + name: "form/false string slice with commas", + style: "form", + explode: false, + paramName: "color", + value: []string{"a,b", "c", "d,e,f"}, + dest: &[]string{}, + expected: []string{"a,b", "c", "d,e,f"}, + }, + { + name: "form/false int slice", + style: "form", + explode: false, + paramName: "ids", + value: []int{1, 2, 3}, + dest: &[]int{}, + expected: []int{1, 2, 3}, + }, + { + name: "form/false primitive string", + style: "form", + explode: false, + paramName: "color", + value: "red", + dest: new(string), + expected: "red", + }, + { + name: "form/false struct", + style: "form", + explode: false, + paramName: "id", + value: TestObject{FirstName: "Alex", Role: "admin"}, + dest: &TestObject{}, + expected: TestObject{FirstName: "Alex", Role: "admin"}, + }, + { + name: "form/true string slice", + style: "form", + explode: true, + paramName: "color", + value: []string{"red", "green", "blue"}, + dest: &[]string{}, + expected: []string{"red", "green", "blue"}, + }, + { + name: "form/true int slice", + style: "form", + explode: true, + paramName: "ids", + value: []int{1, 2, 3}, + dest: &[]int{}, + expected: []int{1, 2, 3}, + }, + { + name: "form/true struct", + style: "form", + explode: true, + paramName: "id", + value: TestObject{FirstName: "Alex", Role: "admin"}, + dest: &TestObject{}, + expected: TestObject{FirstName: "Alex", Role: "admin"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Serialize + raw, err := StyleParamWithLocation(tt.style, tt.explode, tt.paramName, ParamLocationQuery, tt.value) + require.NoError(t, err, "StyleParamWithLocation failed") + + // Deserialize + err = BindRawQueryParameter(tt.style, tt.explode, true, tt.paramName, raw, tt.dest) + require.NoError(t, err, "BindRawQueryParameter failed for raw=%q", raw) + + // Compare + actual := reflect.ValueOf(tt.dest).Elem().Interface() + assert.Equal(t, tt.expected, actual) + }) + } + + t.Run("deepObject/true struct", func(t *testing.T) { + original := TestObject{FirstName: "Alex", Role: "admin"} + + raw, err := StyleParamWithLocation("deepObject", true, "id", ParamLocationQuery, original) + require.NoError(t, err) + + var dest TestObject + err = BindRawQueryParameter("deepObject", true, true, "id", raw, &dest) + require.NoError(t, err) + assert.Equal(t, original, dest) + }) +} + func TestBindStyledParameterWithLocation(t *testing.T) { t.Run("bigNumber", func(t *testing.T) { expectedBig := big.NewInt(12345678910)