diff --git a/http/binder.go b/http/binder.go index 2b88984..3aa1ca3 100644 --- a/http/binder.go +++ b/http/binder.go @@ -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 @@ -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) diff --git a/http/binder_query_multi_test.go b/http/binder_query_multi_test.go new file mode 100644 index 0000000..996dad3 --- /dev/null +++ b/http/binder_query_multi_test.go @@ -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") +}