Skip to content

Commit 13f457b

Browse files
mromaszewiczclaude
andcommitted
feat: add BindRawQueryParameter for correct comma handling
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 5bb2934 commit 13f457b

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
@@ -306,6 +306,11 @@ func bindSplitPartsToDestinationStruct(paramName string, parts []string, explode
306306
// tell them apart. This code tries to fail, but the moral of the story is that
307307
// you shouldn't pass objects via form styled query arguments, just use
308308
// the Content parameter form.
309+
//
310+
// Deprecated: BindQueryParameter pre-decodes the query string via url.Values,
311+
// which makes it impossible to distinguish literal commas from delimiter commas
312+
// in form/explode=false parameters. Use BindRawQueryParameter instead, which
313+
// operates on the raw query string and handles encoded delimiters correctly.
309314
func BindQueryParameter(style string, explode bool, required bool, paramName string,
310315
queryParams url.Values, dest interface{}) error {
311316

@@ -478,6 +483,201 @@ func BindQueryParameter(style string, explode bool, required bool, paramName str
478483
}
479484
}
480485

486+
// findRawQueryParam extracts the raw (still-percent-encoded) values for a given
487+
// parameter name from a raw query string, without URL-decoding the values.
488+
// The parameter key is decoded for comparison purposes, but the returned values
489+
// remain in their original encoded form.
490+
func findRawQueryParam(rawQuery, paramName string) (values []string, found bool) {
491+
for rawQuery != "" {
492+
var part string
493+
if i := strings.IndexByte(rawQuery, '&'); i >= 0 {
494+
part = rawQuery[:i]
495+
rawQuery = rawQuery[i+1:]
496+
} else {
497+
part = rawQuery
498+
rawQuery = ""
499+
}
500+
if part == "" {
501+
continue
502+
}
503+
key := part
504+
var val string
505+
if i := strings.IndexByte(part, '='); i >= 0 {
506+
key = part[:i]
507+
val = part[i+1:]
508+
}
509+
decodedKey, err := url.QueryUnescape(key)
510+
if err != nil {
511+
// Skip malformed keys.
512+
continue
513+
}
514+
if decodedKey == paramName {
515+
values = append(values, val)
516+
found = true
517+
}
518+
}
519+
return values, found
520+
}
521+
522+
// BindRawQueryParameter works like BindQueryParameter but operates on the raw
523+
// (undecoded) query string instead of pre-parsed url.Values. This correctly
524+
// handles form/explode=false parameters whose values contain literal commas
525+
// encoded as %2C — something that BindQueryParameter cannot do because
526+
// url.Values has already decoded %2C to ',' before we can split on the
527+
// delimiter comma.
528+
func BindRawQueryParameter(style string, explode bool, required bool, paramName string,
529+
rawQuery string, dest any) error {
530+
531+
// dv = destination value.
532+
dv := reflect.Indirect(reflect.ValueOf(dest))
533+
534+
// intermediate value form which is either dv or dv dereferenced.
535+
v := dv
536+
537+
// inner code will bind the string's value to this interface.
538+
var output any
539+
540+
// required params are never pointers, but it may happen that optional param
541+
// is not pointer as well if user decides to annotate it with
542+
// x-go-type-skip-optional-pointer
543+
var extraIndirect = !required && v.Kind() == reflect.Pointer
544+
if !extraIndirect {
545+
output = dest
546+
} else {
547+
if v.IsNil() {
548+
t := v.Type()
549+
newValue := reflect.New(t.Elem())
550+
output = newValue.Interface()
551+
} else {
552+
output = v.Interface()
553+
}
554+
v = reflect.Indirect(reflect.ValueOf(output))
555+
}
556+
557+
// This is the basic type of the destination object.
558+
t := v.Type()
559+
k := t.Kind()
560+
561+
switch style {
562+
case "form":
563+
if explode {
564+
// For the explode case, url.ParseQuery is fine — there are no
565+
// delimiter commas to confuse with literal commas.
566+
queryParams, err := url.ParseQuery(rawQuery)
567+
if err != nil {
568+
return fmt.Errorf("error parsing query string: %w", err)
569+
}
570+
values, found := queryParams[paramName]
571+
572+
switch k {
573+
case reflect.Slice:
574+
if !found {
575+
if required {
576+
return fmt.Errorf("query parameter '%s' is required", paramName)
577+
}
578+
return nil
579+
}
580+
err = bindSplitPartsToDestinationArray(values, output)
581+
case reflect.Struct:
582+
var fieldsPresent bool
583+
fieldsPresent, err = bindParamsToExplodedObject(paramName, queryParams, output)
584+
if !fieldsPresent {
585+
return nil
586+
}
587+
default:
588+
if len(values) == 0 {
589+
if required {
590+
return fmt.Errorf("query parameter '%s' is required", paramName)
591+
}
592+
return nil
593+
}
594+
if len(values) != 1 {
595+
return fmt.Errorf("multiple values for single value parameter '%s'", paramName)
596+
}
597+
if !found {
598+
if required {
599+
return fmt.Errorf("query parameter '%s' is required", paramName)
600+
}
601+
return nil
602+
}
603+
err = BindStringToObject(values[0], output)
604+
}
605+
if err != nil {
606+
return err
607+
}
608+
if extraIndirect {
609+
dv.Set(reflect.ValueOf(output))
610+
}
611+
return nil
612+
}
613+
614+
// form, explode=false — the core fix.
615+
// Use findRawQueryParam to get the still-encoded value, split on
616+
// literal ',' (which is the OpenAPI delimiter), then URL-decode
617+
// each resulting part individually.
618+
rawValues, found := findRawQueryParam(rawQuery, paramName)
619+
if !found {
620+
if required {
621+
return fmt.Errorf("query parameter '%s' is required", paramName)
622+
}
623+
return nil
624+
}
625+
if len(rawValues) != 1 {
626+
return fmt.Errorf("parameter '%s' is not exploded, but is specified multiple times", paramName)
627+
}
628+
629+
rawParts := strings.Split(rawValues[0], ",")
630+
parts := make([]string, len(rawParts))
631+
for i, rp := range rawParts {
632+
decoded, err := url.QueryUnescape(rp)
633+
if err != nil {
634+
return fmt.Errorf("error decoding query parameter '%s' part %q: %w", paramName, rp, err)
635+
}
636+
parts[i] = decoded
637+
}
638+
639+
var err error
640+
switch k {
641+
case reflect.Slice:
642+
err = bindSplitPartsToDestinationArray(parts, output)
643+
case reflect.Struct:
644+
err = bindSplitPartsToDestinationStruct(paramName, parts, explode, output)
645+
default:
646+
if len(parts) == 0 {
647+
if required {
648+
return fmt.Errorf("query parameter '%s' is required", paramName)
649+
}
650+
return nil
651+
}
652+
if len(parts) != 1 {
653+
return fmt.Errorf("multiple values for single value parameter '%s'", paramName)
654+
}
655+
err = BindStringToObject(parts[0], output)
656+
}
657+
if err != nil {
658+
return err
659+
}
660+
if extraIndirect {
661+
dv.Set(reflect.ValueOf(output))
662+
}
663+
return nil
664+
665+
case "deepObject":
666+
if !explode {
667+
return errors.New("deepObjects must be exploded")
668+
}
669+
queryParams, err := url.ParseQuery(rawQuery)
670+
if err != nil {
671+
return fmt.Errorf("error parsing query string: %w", err)
672+
}
673+
return UnmarshalDeepObject(dest, paramName, queryParams)
674+
case "spaceDelimited", "pipeDelimited":
675+
return fmt.Errorf("query arguments of style '%s' aren't yet supported", style)
676+
default:
677+
return fmt.Errorf("style '%s' on parameter '%s' is invalid", style, paramName)
678+
}
679+
}
680+
481681
// bindParamsToExplodedObject reflects the destination structure, and pulls the value for
482682
// each settable field from the given parameters map. This is to deal with the
483683
// exploded form styled object which may occupy any number of parameter names.

0 commit comments

Comments
 (0)