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
36 changes: 34 additions & 2 deletions http/binder.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,6 @@ func (c *Ctx) bindQueryParam(field reflect.StructField, fieldValue reflect.Value
paramName = field.Name
}

value := c.Query(paramName)

// Determine if field is required using consistent precedence:
// 1. optional:"true" - explicitly optional (highest priority)
// 2. required:"true" - explicitly required
Expand All @@ -198,6 +196,40 @@ func (c *Ctx) bindQueryParam(field reflect.StructField, fieldValue reflect.Value
// 5. default: non-pointer types are required
required := isBindFieldRequired(field, tag)

// Repeated parameters (resource=a&resource=b) fill a slice field. Reading
// a single value through Query would keep the first occurrence and drop
// the rest, which for something like an RFC 8707 resource indicator
// silently narrows what the caller asked for.
if isMultiValueTarget(fieldValue) {
present := c.queryValues()[paramName]
if len(present) == 0 {
if required {
errors.AddWithCode(paramName, "query parameter is required", val.ErrCodeRequired, nil)

return nil
}

if defaultVal := field.Tag.Get("default"); defaultVal != "" {
present = strings.Split(defaultVal, ",")
}
}

switch len(present) {
case 0:
return nil
case 1:
// A single occurrence goes through setFieldValue so that a
// comma-separated value (scope=openid,profile) expands the same way
// it always has. Splitting only ever applies to a lone value;
// repeated parameters are taken verbatim.
return setFieldValue(fieldValue, present[0], paramName, errors)
default:
return setSliceFieldValue(fieldValue, present, paramName, errors)
}
}

value := c.Query(paramName)

if required && value == "" {
errors.AddWithCode(paramName, "query parameter is required", val.ErrCodeRequired, nil)

Expand Down
90 changes: 90 additions & 0 deletions http/binder_query_multi_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package http

import (
"context"
"net/http"
"net/http/httptest"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// QueryMultiRequest mirrors an OAuth2 authorization request: RFC 8707 defines
// `resource` as repeatable, so the field has to collect every occurrence
// rather than the first one.
type QueryMultiRequest struct {
ClientID string `query:"client_id"`
Resources []string `query:"resource,omitempty"`
Scopes []string `default:"openid,profile" query:"scope,omitempty"`
}

func getQuery(t *testing.T, target string) *Ctx {
t.Helper()

req := httptest.NewRequestWithContext(context.Background(), http.MethodGet, target, nil)

return NewContext(httptest.NewRecorder(), req, nil).(*Ctx)
}

func TestBindRequest_RepeatedQueryParamFillsSlice(t *testing.T) {
var req QueryMultiRequest
require.NoError(t, getQuery(t,
"/authorize?client_id=abc&resource=https://a.example.com&resource=https://b.example.com").
BindRequest(&req))

assert.Equal(t, "abc", req.ClientID)
assert.Equal(t, []string{"https://a.example.com", "https://b.example.com"}, req.Resources,
"every occurrence of a repeated query parameter must reach the slice")
}

func TestBindRequest_SingleQueryParamStillFillsSlice(t *testing.T) {
var req QueryMultiRequest
require.NoError(t, getQuery(t, "/authorize?client_id=abc&resource=https://a.example.com").
BindRequest(&req))

assert.Equal(t, []string{"https://a.example.com"}, req.Resources)
}

// A lone value keeps expanding on commas, the way scope=openid,profile always
// has. Only repeated parameters are taken verbatim.
func TestBindRequest_LoneCommaValueStillExpands(t *testing.T) {
var req QueryMultiRequest
require.NoError(t, getQuery(t, "/authorize?client_id=abc&scope=openid,email").BindRequest(&req))

assert.Equal(t, []string{"openid", "email"}, req.Scopes)
}

func TestBindRequest_RepeatedQueryParamIsTakenVerbatim(t *testing.T) {
var req QueryMultiRequest
require.NoError(t, getQuery(t, "/authorize?client_id=abc&scope=openid&scope=a,b").BindRequest(&req))

assert.Equal(t, []string{"openid", "a,b"}, req.Scopes,
"a repeated parameter must not be split further, the same as the form path")
}

func TestBindRequest_AbsentRepeatedQueryParamLeavesSliceEmpty(t *testing.T) {
var req QueryMultiRequest
require.NoError(t, getQuery(t, "/authorize?client_id=abc").BindRequest(&req))

assert.Empty(t, req.Resources)
}

func TestBindRequest_AbsentQuerySliceTakesItsDefault(t *testing.T) {
var req QueryMultiRequest
require.NoError(t, getQuery(t, "/authorize?client_id=abc").BindRequest(&req))

assert.Equal(t, []string{"openid", "profile"}, req.Scopes)
}

func TestBindRequest_RequiredQuerySliceReportsWhenAbsent(t *testing.T) {
type required struct {
Resources []string `query:"resource" required:"true"`
}

var req required

err := getQuery(t, "/authorize").BindRequest(&req)
require.Error(t, err)
assert.Contains(t, err.Error(), "resource")
}