From c97bb85d9af2ddb13b4bcfad7df912711220a828 Mon Sep 17 00:00:00 2001 From: Andrii Nechaiev Date: Fri, 17 Jul 2026 13:45:40 +0000 Subject: [PATCH] 308: fix allowReserved rejecting percent-encoded reserved character --- helpers/parameter_utilities.go | 25 ++++++++ helpers/parameter_utilities_test.go | 11 ++++ parameters/query_parameters.go | 23 +++++-- parameters/query_parameters_test.go | 98 +++++++++++++++++++++++++++++ parameters/validation_functions.go | 8 ++- 5 files changed, 157 insertions(+), 8 deletions(-) diff --git a/helpers/parameter_utilities.go b/helpers/parameter_utilities.go index e38a982..46de99c 100644 --- a/helpers/parameter_utilities.go +++ b/helpers/parameter_utilities.go @@ -6,6 +6,7 @@ package helpers import ( "fmt" "net/http" + "net/url" "slices" "strconv" "strings" @@ -21,10 +22,34 @@ import ( type QueryParam struct { Key string Values []string + RawValues []string // values as sent on the wire, index-aligned with Values Property string PropertyPath []string } +// ExtractRawQueryValues parses a URL's RawQuery into a map of decoded key to raw +// (still percent-encoded) values, index-aligned per key with request.URL.Query(). +func ExtractRawQueryValues(rawQuery string) map[string][]string { + rawValues := make(map[string][]string) + for rawQuery != "" { + var segment string + segment, rawQuery, _ = strings.Cut(rawQuery, "&") + if segment == "" || strings.Contains(segment, ";") { + continue + } + rawKey, rawValue, _ := strings.Cut(segment, "=") + key, err := url.QueryUnescape(rawKey) + if err != nil { + continue + } + if _, err = url.QueryUnescape(rawValue); err != nil { + continue + } + rawValues[key] = append(rawValues[key], rawValue) + } + return rawValues +} + // ExtractParamsForOperation will extract the parameters for the operation based on the request method. // Both the path level params and the method level params will be returned. func ExtractParamsForOperation(request *http.Request, item *v3.PathItem) []*v3.Parameter { diff --git a/helpers/parameter_utilities_test.go b/helpers/parameter_utilities_test.go index 29776e0..6a450c8 100644 --- a/helpers/parameter_utilities_test.go +++ b/helpers/parameter_utilities_test.go @@ -1419,3 +1419,14 @@ func TestEffectiveSecurityForOperation(t *testing.T) { require.Equal(t, headSecurity, result) }) } + +func TestExtractRawQueryValues(t *testing.T) { + raw := ExtractRawQueryValues( + "filter%5Bdate%5D=2026-05-17%2C2026-07-17&tag=a&tag=b%2Cc&novalue&&bad;pair=1&bad%zzkey=1&key=bad%zzvalue", + ) + require.Equal(t, map[string][]string{ + "filter[date]": {"2026-05-17%2C2026-07-17"}, + "tag": {"a", "b%2Cc"}, + "novalue": {""}, + }, raw) +} diff --git a/parameters/query_parameters.go b/parameters/query_parameters.go index 1c8086e..974ad82 100644 --- a/parameters/query_parameters.go +++ b/parameters/query_parameters.go @@ -61,26 +61,31 @@ func (v *paramValidator) ValidateQueryParamsWithPathItem(request *http.Request, } } + rawQueryValues := helpers.ExtractRawQueryValues(request.URL.RawQuery) + for qKey, qVal := range request.URL.Query() { // check if the query key exactly matches a spec parameter name (e.g., "match[]") // if so, store it literally without deepObject stripping if specParamNames[qKey] { queryParams[qKey] = append(queryParams[qKey], &helpers.QueryParam{ - Key: qKey, - Values: qVal, + Key: qKey, + Values: qVal, + RawValues: rawQueryValues[qKey], }) } else if stripped, propertyPath, ok := helpers.ParseDeepObjectKey(qKey); ok { // check if the param is encoded as a property / deepObject queryParams[stripped] = append(queryParams[stripped], &helpers.QueryParam{ Key: stripped, Values: qVal, + RawValues: rawQueryValues[qKey], Property: propertyPath[0], PropertyPath: propertyPath, }) } else { queryParams[qKey] = append(queryParams[qKey], &helpers.QueryParam{ - Key: qKey, - Values: qVal, + Key: qKey, + Values: qVal, + RawValues: rawQueryValues[qKey], }) } } @@ -126,14 +131,20 @@ doneLooking: pType := sch.Type // for each param, check each type - for _, ef := range fp.Values { + for efIdx, ef := range fp.Values { // check allowReserved values. If this is set to true, then we can allow the // following characters // :/?#[]@!$&'()*+,;= // to be present as they are, without being URLEncoded. if !params[p].AllowReserved { - if rxRxp.MatchString(ef) && params[p].IsExploded() { + // check the raw value: a percent-encoded reserved character is compliant, + // only a literal one violates allowReserved. '+' reads as space (form encoding). + checkValue := ef + if efIdx < len(fp.RawValues) { + checkValue = strings.ReplaceAll(fp.RawValues[efIdx], "+", " ") + } + if rxRxp.MatchString(checkValue) && params[p].IsExploded() { validationErrors = append(validationErrors, errors.IncorrectReservedValues(params[p], ef, sch, pathValue, operation, renderedSchema)) } diff --git a/parameters/query_parameters_test.go b/parameters/query_parameters_test.go index 9cff869..69f8048 100644 --- a/parameters/query_parameters_test.go +++ b/parameters/query_parameters_test.go @@ -2164,6 +2164,104 @@ paths: "reserved values are correctly encoded, for example: '%24%24oh'", errors[0].HowToFix) } +func TestNewValidator_QueryParamAllowReserved_PercentEncodedValueAccepted(t *testing.T) { + spec := `openapi: 3.1.0 +paths: + /a/fishy/on/a/dishy: + get: + parameters: + - name: fishy + in: query + required: true + explode: true + schema: + type: array + items: + type: string + operationId: locateFishy` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + + m, _ := doc.BuildV3Model() + + v := NewParameterValidator(&m.Model) + + // '%24%24oh' is the encoding the reserved-values error's HowToFix suggests for '$$oh' + request, _ := http.NewRequest(http.MethodGet, + "https://things.com/a/fishy/on/a/dishy?fishy=%24%24oh", nil) + + valid, errors := v.ValidateQueryParams(request) + assert.True(t, valid) + assert.Len(t, errors, 0) +} + +func TestNewValidator_QueryParamAllowReserved_DeepObjectPercentEncodedComma(t *testing.T) { + spec := `openapi: 3.1.0 +paths: + /a/fishy/on/a/dishy: + get: + parameters: + - name: filter + in: query + required: true + style: deepObject + explode: true + schema: + type: object + properties: + dateRange: + type: string + operationId: locateFishy` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + + m, _ := doc.BuildV3Model() + + v := NewParameterValidator(&m.Model) + + // %2C is how a compliant client sends a comma when allowReserved is false + request, _ := http.NewRequest(http.MethodGet, + "https://things.com/a/fishy/on/a/dishy?filter[dateRange]=2026-05-17%2C2026-07-17", nil) + + valid, errors := v.ValidateQueryParams(request) + assert.True(t, valid) + assert.Len(t, errors, 0) +} + +func TestNewValidator_QueryParamAllowReserved_DeepObjectRawCommaRejected(t *testing.T) { + spec := `openapi: 3.1.0 +paths: + /a/fishy/on/a/dishy: + get: + parameters: + - name: filter + in: query + required: true + style: deepObject + explode: true + schema: + type: object + properties: + dateRange: + type: string + operationId: locateFishy` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + + m, _ := doc.BuildV3Model() + + v := NewParameterValidator(&m.Model) + + // a literal comma violates allowReserved: false; the raw '[' ']' in the key must not be flagged + request, _ := http.NewRequest(http.MethodGet, + "https://things.com/a/fishy/on/a/dishy?filter[dateRange]=2026-05-17,2026-07-17", nil) + + valid, errors := v.ValidateQueryParams(request) + assert.False(t, valid) + require.Len(t, errors, 1) + assert.Equal(t, "Query parameter 'filter' value contains reserved values", errors[0].Message) +} + func TestNewValidator_QueryParamValidateStyle_ValidObjectArrayNoExplode(t *testing.T) { spec := `openapi: 3.1.0 paths: diff --git a/parameters/validation_functions.go b/parameters/validation_functions.go index 069277a..91f7e91 100644 --- a/parameters/validation_functions.go +++ b/parameters/validation_functions.go @@ -284,8 +284,12 @@ stopValidation: } } default: - // check for a delimited list. - if helpers.DoesFormParamContainDelimiter(qp.Values[i], param.Style) { + // check for a delimited list; a percent-encoded comma is data, only a literal comma delimits. + checkValue := qp.Values[i] + if i < len(qp.RawValues) { + checkValue = qp.RawValues[i] + } + if helpers.DoesFormParamContainDelimiter(checkValue, param.Style) { if param.Explode != nil && *param.Explode { validationErrors = append(validationErrors, errors.IncorrectFormEncoding(param, qp, i)) break stopValidation