Skip to content

Commit 1f844c3

Browse files
mromaszewiczclaude
andauthored
feat: add BindRawQueryParameter for correct comma handling (#92)
Add BindRawQueryParameter which operates on the raw (undecoded) query string instead of pre-parsed url.Values. For form/explode=false parameters, this splits on literal commas before URL-decoding each part, correctly preserving %2C as a literal comma in values. Also adds findRawQueryParam internal helper, comprehensive tests, and round-trip (serialize/deserialize) tests. Deprecates BindQueryParameter, which cannot distinguish delimiter commas from literal commas because url.Values pre-decodes %2C. Fixes #91 Note: oapi-codegen/oapi-codegen will need to update its generated code to call BindRawQueryParameter (passing r.URL.RawQuery) once this change is released. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent ca4e933 commit 1f844c3

File tree

2 files changed

+587
-0
lines changed

2 files changed

+587
-0
lines changed

bindparam.go

Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,11 @@ func bindSplitPartsToDestinationStruct(paramName string, parts []string, explode
328328
// tell them apart. This code tries to fail, but the moral of the story is that
329329
// you shouldn't pass objects via form styled query arguments, just use
330330
// the Content parameter form.
331+
//
332+
// Deprecated: BindQueryParameter pre-decodes the query string via url.Values,
333+
// which makes it impossible to distinguish literal commas from delimiter commas
334+
// in form/explode=false parameters. Use BindRawQueryParameter instead, which
335+
// operates on the raw query string and handles encoded delimiters correctly.
331336
func BindQueryParameter(style string, explode bool, required bool, paramName string,
332337
queryParams url.Values, dest interface{}) error {
333338
return BindQueryParameterWithOptions(style, explode, required, paramName, queryParams, dest, BindQueryParameterOptions{})
@@ -538,6 +543,201 @@ func BindQueryParameterWithOptions(style string, explode bool, required bool, pa
538543
}
539544
}
540545

546+
// findRawQueryParam extracts the raw (still-percent-encoded) values for a given
547+
// parameter name from a raw query string, without URL-decoding the values.
548+
// The parameter key is decoded for comparison purposes, but the returned values
549+
// remain in their original encoded form.
550+
func findRawQueryParam(rawQuery, paramName string) (values []string, found bool) {
551+
for rawQuery != "" {
552+
var part string
553+
if i := strings.IndexByte(rawQuery, '&'); i >= 0 {
554+
part = rawQuery[:i]
555+
rawQuery = rawQuery[i+1:]
556+
} else {
557+
part = rawQuery
558+
rawQuery = ""
559+
}
560+
if part == "" {
561+
continue
562+
}
563+
key := part
564+
var val string
565+
if i := strings.IndexByte(part, '='); i >= 0 {
566+
key = part[:i]
567+
val = part[i+1:]
568+
}
569+
decodedKey, err := url.QueryUnescape(key)
570+
if err != nil {
571+
// Skip malformed keys.
572+
continue
573+
}
574+
if decodedKey == paramName {
575+
values = append(values, val)
576+
found = true
577+
}
578+
}
579+
return values, found
580+
}
581+
582+
// BindRawQueryParameter works like BindQueryParameter but operates on the raw
583+
// (undecoded) query string instead of pre-parsed url.Values. This correctly
584+
// handles form/explode=false parameters whose values contain literal commas
585+
// encoded as %2C — something that BindQueryParameter cannot do because
586+
// url.Values has already decoded %2C to ',' before we can split on the
587+
// delimiter comma.
588+
func BindRawQueryParameter(style string, explode bool, required bool, paramName string,
589+
rawQuery string, dest any) error {
590+
591+
// dv = destination value.
592+
dv := reflect.Indirect(reflect.ValueOf(dest))
593+
594+
// intermediate value form which is either dv or dv dereferenced.
595+
v := dv
596+
597+
// inner code will bind the string's value to this interface.
598+
var output any
599+
600+
// required params are never pointers, but it may happen that optional param
601+
// is not pointer as well if user decides to annotate it with
602+
// x-go-type-skip-optional-pointer
603+
var extraIndirect = !required && v.Kind() == reflect.Pointer
604+
if !extraIndirect {
605+
output = dest
606+
} else {
607+
if v.IsNil() {
608+
t := v.Type()
609+
newValue := reflect.New(t.Elem())
610+
output = newValue.Interface()
611+
} else {
612+
output = v.Interface()
613+
}
614+
v = reflect.Indirect(reflect.ValueOf(output))
615+
}
616+
617+
// This is the basic type of the destination object.
618+
t := v.Type()
619+
k := t.Kind()
620+
621+
switch style {
622+
case "form":
623+
if explode {
624+
// For the explode case, url.ParseQuery is fine — there are no
625+
// delimiter commas to confuse with literal commas.
626+
queryParams, err := url.ParseQuery(rawQuery)
627+
if err != nil {
628+
return fmt.Errorf("error parsing query string: %w", err)
629+
}
630+
values, found := queryParams[paramName]
631+
632+
switch k {
633+
case reflect.Slice:
634+
if !found {
635+
if required {
636+
return fmt.Errorf("query parameter '%s' is required", paramName)
637+
}
638+
return nil
639+
}
640+
err = bindSplitPartsToDestinationArray(values, output)
641+
case reflect.Struct:
642+
var fieldsPresent bool
643+
fieldsPresent, err = bindParamsToExplodedObject(paramName, queryParams, output)
644+
if !fieldsPresent {
645+
return nil
646+
}
647+
default:
648+
if len(values) == 0 {
649+
if required {
650+
return fmt.Errorf("query parameter '%s' is required", paramName)
651+
}
652+
return nil
653+
}
654+
if len(values) != 1 {
655+
return fmt.Errorf("multiple values for single value parameter '%s'", paramName)
656+
}
657+
if !found {
658+
if required {
659+
return fmt.Errorf("query parameter '%s' is required", paramName)
660+
}
661+
return nil
662+
}
663+
err = BindStringToObject(values[0], output)
664+
}
665+
if err != nil {
666+
return err
667+
}
668+
if extraIndirect {
669+
dv.Set(reflect.ValueOf(output))
670+
}
671+
return nil
672+
}
673+
674+
// form, explode=false — the core fix.
675+
// Use findRawQueryParam to get the still-encoded value, split on
676+
// literal ',' (which is the OpenAPI delimiter), then URL-decode
677+
// each resulting part individually.
678+
rawValues, found := findRawQueryParam(rawQuery, paramName)
679+
if !found {
680+
if required {
681+
return fmt.Errorf("query parameter '%s' is required", paramName)
682+
}
683+
return nil
684+
}
685+
if len(rawValues) != 1 {
686+
return fmt.Errorf("parameter '%s' is not exploded, but is specified multiple times", paramName)
687+
}
688+
689+
rawParts := strings.Split(rawValues[0], ",")
690+
parts := make([]string, len(rawParts))
691+
for i, rp := range rawParts {
692+
decoded, err := url.QueryUnescape(rp)
693+
if err != nil {
694+
return fmt.Errorf("error decoding query parameter '%s' part %q: %w", paramName, rp, err)
695+
}
696+
parts[i] = decoded
697+
}
698+
699+
var err error
700+
switch k {
701+
case reflect.Slice:
702+
err = bindSplitPartsToDestinationArray(parts, output)
703+
case reflect.Struct:
704+
err = bindSplitPartsToDestinationStruct(paramName, parts, explode, output)
705+
default:
706+
if len(parts) == 0 {
707+
if required {
708+
return fmt.Errorf("query parameter '%s' is required", paramName)
709+
}
710+
return nil
711+
}
712+
if len(parts) != 1 {
713+
return fmt.Errorf("multiple values for single value parameter '%s'", paramName)
714+
}
715+
err = BindStringToObject(parts[0], output)
716+
}
717+
if err != nil {
718+
return err
719+
}
720+
if extraIndirect {
721+
dv.Set(reflect.ValueOf(output))
722+
}
723+
return nil
724+
725+
case "deepObject":
726+
if !explode {
727+
return errors.New("deepObjects must be exploded")
728+
}
729+
queryParams, err := url.ParseQuery(rawQuery)
730+
if err != nil {
731+
return fmt.Errorf("error parsing query string: %w", err)
732+
}
733+
return UnmarshalDeepObject(dest, paramName, queryParams)
734+
case "spaceDelimited", "pipeDelimited":
735+
return fmt.Errorf("query arguments of style '%s' aren't yet supported", style)
736+
default:
737+
return fmt.Errorf("style '%s' on parameter '%s' is invalid", style, paramName)
738+
}
739+
}
740+
541741
// bindParamsToExplodedObject reflects the destination structure, and pulls the value for
542742
// each settable field from the given parameters map. This is to deal with the
543743
// exploded form styled object which may occupy any number of parameter names.

0 commit comments

Comments
 (0)