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
45 changes: 29 additions & 16 deletions bindparam.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,16 @@ type BindStyledParameterOptions struct {
// AllowReserved, when true, indicates that the parameter value may
// contain RFC 3986 reserved characters without percent-encoding.
AllowReserved bool
// ValueIsUnescaped, when true, indicates that the value has already had
// its URL percent-encoding removed (typically by the router) and is
// bound verbatim. The default treats the value as still escaped and
// unescapes it according to ParamLocation — the historical behavior,
// preserved for existing callers. Routers differ in whether they
// deliver path parameters raw or already decoded — a mismatch either
// double-decodes values containing literal percent signs or leaves
// them encoded — so the generated wrapper sets this to match its
// framework. See https://github.com/oapi-codegen/runtime/issues/35
ValueIsUnescaped bool
}

// BindStyledParameterWithOptions binds a parameter as described in the Path Parameters
Expand All @@ -95,23 +105,26 @@ func BindStyledParameterWithOptions(style string, paramName string, value string
}
}

// Based on the location of the parameter, we need to unescape it properly.
var err error
switch opts.ParamLocation {
case ParamLocationQuery, ParamLocationUndefined:
// We unescape undefined parameter locations here for older generated code,
// since prior to this refactoring, they always query unescaped.
value, err = url.QueryUnescape(value)
if err != nil {
return fmt.Errorf("error unescaping query parameter '%s': %w", paramName, err)
}
case ParamLocationPath:
value, err = url.PathUnescape(value)
if err != nil {
return fmt.Errorf("error unescaping path parameter '%s': %w", paramName, err)
// Unless the caller says the router already unescaped the value, it is
// unescaped here according to the location of the parameter. Undefined
// locations are query-unescaped for older generated code, which always
// bound query-unescaped values.
if !opts.ValueIsUnescaped {
var err error
switch opts.ParamLocation {
case ParamLocationQuery, ParamLocationUndefined:
value, err = url.QueryUnescape(value)
if err != nil {
return fmt.Errorf("error unescaping query parameter '%s': %w", paramName, err)
}
case ParamLocationPath:
value, err = url.PathUnescape(value)
if err != nil {
return fmt.Errorf("error unescaping path parameter '%s': %w", paramName, err)
}
default:
// Headers and cookies aren't escaped.
}
default:
// Headers and cookies aren't escaped.
}

// If the destination implements encoding.TextUnmarshaler we use it for binding
Expand Down
56 changes: 56 additions & 0 deletions bindparam_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1623,3 +1623,59 @@ func TestRoundTripQueryParameter_Delimited(t *testing.T) {
})
}
}

// TestBindStyledParameterValueIsUnescaped covers issue #35: routers differ
// in whether they deliver path parameters raw or already decoded, so the
// generated wrapper declares it via ValueIsUnescaped rather than the binder
// unconditionally unescaping (which double-decoded values containing
// literal percent signs). The zero value preserves the historical
// unescaping for existing callers.
func TestBindStyledParameterValueIsUnescaped(t *testing.T) {
var dst string

// By default, values are treated as escaped and unescaped by location —
// the historical behavior of existing generated code.
err := BindStyledParameterWithOptions("simple", "param1", "discount%2520", &dst, BindStyledParameterOptions{
ParamLocation: ParamLocationPath,
Required: true,
})
require.NoError(t, err)
assert.Equal(t, "discount%20", dst)

err = BindStyledParameterWithOptions("form", "param1", "a+b%2Fc", &dst, BindStyledParameterOptions{
ParamLocation: ParamLocationQuery,
Required: true,
})
require.NoError(t, err)
assert.Equal(t, "a b/c", dst)

// A router-decoded value binds verbatim when the wrapper says so: a
// value that legitimately contains a percent sequence is not decoded
// again...
err = BindStyledParameterWithOptions("simple", "param1", "discount%20", &dst, BindStyledParameterOptions{
ParamLocation: ParamLocationPath,
Required: true,
ValueIsUnescaped: true,
})
require.NoError(t, err)
assert.Equal(t, "discount%20", dst)

// ...and a bare percent doesn't fail as an invalid escape.
err = BindStyledParameterWithOptions("simple", "param1", "15%off", &dst, BindStyledParameterOptions{
ParamLocation: ParamLocationPath,
Required: true,
ValueIsUnescaped: true,
})
require.NoError(t, err)
assert.Equal(t, "15%off", dst)

// The deprecated entry points always unescaped; the zero value keeps
// that behavior for the old generated code that calls them.
err = BindStyledParameterWithLocation("simple", false, "param1", ParamLocationPath, "discount%2520", &dst)
require.NoError(t, err)
assert.Equal(t, "discount%20", dst)

err = BindStyledParameter("simple", false, "param1", "a+b", &dst)
require.NoError(t, err)
assert.Equal(t, "a b", dst)
}