diff --git a/errors/parameter_errors.go b/errors/parameter_errors.go index 6667b76..48ee25a 100644 --- a/errors/parameter_errors.go +++ b/errors/parameter_errors.go @@ -87,6 +87,47 @@ func InvalidDeepObject(param *v3.Parameter, qp *helpers.QueryParam) *ValidationE } } +func deepObjectPathForError(qp *helpers.QueryParam) string { + if qp == nil { + return "" + } + if len(qp.PropertyPath) > 0 { + return strings.Join(qp.PropertyPath, ".") + } + return qp.Property +} + +func deepObjectBracketPathForError(qp *helpers.QueryParam) string { + if qp == nil { + return "" + } + if len(qp.PropertyPath) > 0 { + return strings.Join(qp.PropertyPath, "][") + } + return qp.Property +} + +func InvalidDeepObjectPathConflict(param *v3.Parameter, prefixParam, nestedParam *helpers.QueryParam) *ValidationError { + specLine, specCol := paramStyleLineCol(param) + prefixPath := deepObjectPathForError(prefixParam) + nestedPath := deepObjectPathForError(nestedParam) + return &ValidationError{ + ValidationType: helpers.ParameterValidation, + ValidationSubType: helpers.ParameterValidationQuery, + Message: fmt.Sprintf("Query parameter '%s' is not a valid deepObject", param.Name), + Reason: fmt.Sprintf("The query parameter '%s' has the 'deepObject' style defined, "+ + "but the property path '%s' is also used as a nested object prefix for '%s'", + param.Name, prefixPath, nestedPath), + SpecLine: specLine, + SpecCol: specCol, + ParameterName: param.Name, + Context: param, + HowToFix: fmt.Sprintf(HowToFixParamInvalidDeepObjectPathConflict, + param.Name, deepObjectBracketPathForError(prefixParam), + param.Name, deepObjectBracketPathForError(nestedParam)), + } +} + func QueryParameterMissing(param *v3.Parameter, pathTemplate string, operation string, renderedSchema string) *ValidationError { keywordLocation := helpers.ConstructParameterJSONPointer(pathTemplate, operation, param.Name, "required") specLine, specCol := paramRequiredLineCol(param) diff --git a/errors/parameter_errors_test.go b/errors/parameter_errors_test.go index 4050271..e9a5e51 100644 --- a/errors/parameter_errors_test.go +++ b/errors/parameter_errors_test.go @@ -290,6 +290,66 @@ func TestInvalidDeepObject(t *testing.T) { require.Contains(t, err.HowToFix, "testParam=value1|value2") } +func TestInvalidDeepObjectPathConflict(t *testing.T) { + param := createMockParameterWithDeepObjectStyle() + prefixParam := &helpers.QueryParam{ + Key: "testParam", + Values: []string{"bad"}, + Property: "nested", + PropertyPath: []string{"nested"}, + } + nestedParam := &helpers.QueryParam{ + Key: "testParam", + Values: []string{"ok"}, + Property: "nested", + PropertyPath: []string{"nested", "child"}, + } + + err := InvalidDeepObjectPathConflict(param, prefixParam, nestedParam) + + require.NotNil(t, err) + require.Equal(t, helpers.ParameterValidation, err.ValidationType) + require.Equal(t, helpers.ParameterValidationQuery, err.ValidationSubType) + require.Equal(t, "testParam", err.ParameterName) + require.Contains(t, err.Message, "Query parameter 'testParam' is not a valid deepObject") + require.Contains(t, err.Reason, "property path 'nested'") + require.Contains(t, err.Reason, "'nested.child'") + require.Contains(t, err.HowToFix, "testParam[nested]") + require.Contains(t, err.HowToFix, "testParam[nested][child]") +} + +func TestInvalidDeepObjectPathConflict_NilPaths(t *testing.T) { + param := createMockParameterWithDeepObjectStyle() + + err := InvalidDeepObjectPathConflict(param, nil, nil) + + require.NotNil(t, err) + require.Contains(t, err.Reason, "property path ''") + require.Contains(t, err.HowToFix, "testParam[]") +} + +func TestInvalidDeepObjectPathConflict_PropertyFallback(t *testing.T) { + param := createMockParameterWithDeepObjectStyle() + prefixParam := &helpers.QueryParam{ + Key: "testParam", + Values: []string{"bad"}, + Property: "nested", + } + nestedParam := &helpers.QueryParam{ + Key: "testParam", + Values: []string{"ok"}, + Property: "nested.child", + } + + err := InvalidDeepObjectPathConflict(param, prefixParam, nestedParam) + + require.NotNil(t, err) + require.Contains(t, err.Reason, "property path 'nested'") + require.Contains(t, err.Reason, "'nested.child'") + require.Contains(t, err.HowToFix, "testParam[nested]") + require.Contains(t, err.HowToFix, "testParam[nested.child]") +} + func createMockParameterForBooleanArray() *v3.Parameter { param := &lowv3.Parameter{ Name: low.NodeReference[string]{Value: "testCookieParam"}, @@ -1290,6 +1350,18 @@ func TestParameterErrors_NilGoLowNodes(t *testing.T) { require.Equal(t, 0, err.SpecCol) }) + t.Run("InvalidDeepObjectPathConflict", func(t *testing.T) { + err := InvalidDeepObjectPathConflict(param, qp, &helpers.QueryParam{ + Key: "test", + Values: []string{"ok"}, + Property: "foo", + PropertyPath: []string{"foo", "bar"}, + }) + require.NotNil(t, err) + require.Equal(t, 1, err.SpecLine) + require.Equal(t, 0, err.SpecCol) + }) + t.Run("QueryParameterMissing", func(t *testing.T) { err := QueryParameterMissing(param, "/test", "get", "{}") require.NotNil(t, err) diff --git a/errors/parameters_howtofix.go b/errors/parameters_howtofix.go index b884700..143197e 100644 --- a/errors/parameters_howtofix.go +++ b/errors/parameters_howtofix.go @@ -24,18 +24,19 @@ const ( "they should be separated by pipes '|'. For example: '%s'" HowToFixParamInvalidDeepObjectMultipleValues string = "There can only be a single value per property name, " + "deepObject parameters should contain the property key in square brackets next to the parameter name. For example: '%s'" - HowToFixInvalidJSON string = "The JSON submitted is invalid, please check the syntax" - HowToFixInvalidUrlEncoded string = "Ensure URL Encoded submitted is well-formed and matches schema structure" - HowToFixDecodingError string = "The object can't be decoded, so make sure it's being encoded correctly according to the spec." - HowToFixInvalidContentType string = "The content type is invalid, Use one of the %d supported types for this operation: %s" - HowToFixInvalidResponseCode string = "The service is responding with a code that is not defined in the spec, fix the service or add the code to the specification" - HowToFixInvalidEncoding string = "Ensure the correct encoding has been used on the object" - HowToFixMissingValue string = "Ensure the value has been set" - HowToFixPath string = "Check the path is correct, and check that the correct HTTP method has been used (e.g. GET, POST, PUT, DELETE)" - HowToFixPathMethod string = "Add the missing operation to the contract for the path" - HowToFixInvalidMaxItems string = "Reduce the number of items in the array to %d or less" - HowToFixInvalidMinItems string = "Increase the number of items in the array to %d or more" - HowToFixMissingHeader string = "Make sure the service responding sets the required headers with this response code" - HowToFixInvalidRenderedSchema string = "Check the request schema for circular references or invalid structures" - HowToFixInvalidJsonSchema string = "Check the request schema for invalid JSON Schema syntax, complex regex patterns, or unsupported schema constructs" + HowToFixParamInvalidDeepObjectPathConflict string = "Use either '%s[%s]' or nested properties like '%s[%s]', not both" + HowToFixInvalidJSON string = "The JSON submitted is invalid, please check the syntax" + HowToFixInvalidUrlEncoded string = "Ensure URL Encoded submitted is well-formed and matches schema structure" + HowToFixDecodingError string = "The object can't be decoded, so make sure it's being encoded correctly according to the spec." + HowToFixInvalidContentType string = "The content type is invalid, Use one of the %d supported types for this operation: %s" + HowToFixInvalidResponseCode string = "The service is responding with a code that is not defined in the spec, fix the service or add the code to the specification" + HowToFixInvalidEncoding string = "Ensure the correct encoding has been used on the object" + HowToFixMissingValue string = "Ensure the value has been set" + HowToFixPath string = "Check the path is correct, and check that the correct HTTP method has been used (e.g. GET, POST, PUT, DELETE)" + HowToFixPathMethod string = "Add the missing operation to the contract for the path" + HowToFixInvalidMaxItems string = "Reduce the number of items in the array to %d or less" + HowToFixInvalidMinItems string = "Increase the number of items in the array to %d or more" + HowToFixMissingHeader string = "Make sure the service responding sets the required headers with this response code" + HowToFixInvalidRenderedSchema string = "Check the request schema for circular references or invalid structures" + HowToFixInvalidJsonSchema string = "Check the request schema for invalid JSON Schema syntax, complex regex patterns, or unsupported schema constructs" ) diff --git a/helpers/parameter_utilities.go b/helpers/parameter_utilities.go index 3ca3e1c..e38a982 100644 --- a/helpers/parameter_utilities.go +++ b/helpers/parameter_utilities.go @@ -19,9 +19,10 @@ import ( // it's used for complex query types that need to be parsed and tracked differently depending // on the encoding styles used. type QueryParam struct { - Key string - Values []string - Property string + Key string + Values []string + Property string + PropertyPath []string } // ExtractParamsForOperation will extract the parameters for the operation based on the request method. @@ -203,21 +204,214 @@ func getPropertySchema(objectSchema *base.Schema, propertyName string) *base.Sch return proxy.Schema() } +func getAdditionalPropertiesSchema(objectSchema *base.Schema) *base.Schema { + if objectSchema == nil || objectSchema.AdditionalProperties == nil || !objectSchema.AdditionalProperties.IsA() || + objectSchema.AdditionalProperties.A == nil { + return nil + } + return objectSchema.AdditionalProperties.A.Schema() +} + +func getSchemaForObjectProperty(objectSchema *base.Schema, propertyName string) *base.Schema { + if propSchema := getPropertySchema(objectSchema, propertyName); propSchema != nil { + return propSchema + } + return getAdditionalPropertiesSchema(objectSchema) +} + +// ParseDeepObjectKey splits a query-string key using qs-style deepObject bracket notation. +// It returns ok=false for non-deepObject keys or malformed bracket paths. +func ParseDeepObjectKey(key string) (baseName string, propertyPath []string, ok bool) { + open := strings.IndexRune(key, '[') + if open <= 0 { + return "", nil, false + } + baseName = key[:open] + rest := key[open:] + for len(rest) > 0 { + if rest[0] != '[' { + return "", nil, false + } + close := strings.IndexRune(rest, ']') + if close <= 1 { + return "", nil, false + } + propertyPath = append(propertyPath, rest[1:close]) + rest = rest[close+1:] + } + return baseName, propertyPath, len(propertyPath) > 0 +} + // castWithSchema casts a string value consulting the schema for the property's declared type. -// If the schema says the property is a string, the value is returned as-is (no numeric/bool guessing). +// If the schema says the property, or array property item, is a string, the value is returned +// as-is (no numeric/bool guessing). // For other declared types, it falls back to cast() which produces correct results for integer, // number, and boolean values. The explicit string check prevents the most common miscast: numeric- // looking strings like "10" being converted to int64 when the schema declares type: string. func castWithSchema(v string, objectSchema *base.Schema, propertyName string) any { - propSchema := getPropertySchema(objectSchema, propertyName) + propSchema := schemaForPropertyPath(objectSchema, []string{propertyName}) + if schemaPreservesStringValue(propSchema) { + return v + } + if propSchema == nil && schemaPreservesStringValue(objectSchema) { + return v + } + return cast(v) +} + +func queryParamPropertyPath(v *QueryParam) []string { + if len(v.PropertyPath) > 0 { + return v.PropertyPath + } + return []string{v.Property} +} + +func queryParamDeepObjectPath(v *QueryParam) []string { + if v == nil { + return nil + } + if len(v.PropertyPath) > 0 { + return v.PropertyPath + } + if v.Property != "" { + return []string{v.Property} + } + return nil +} + +func schemaForPropertyPath(objectSchema *base.Schema, propertyPath []string) *base.Schema { + current := objectSchema + for _, propertyName := range propertyPath { + current = getSchemaForObjectProperty(current, propertyName) + if current == nil { + return nil + } + } + return current +} + +func schemaTypeIncludes(sch *base.Schema, schemaType string) bool { + return sch != nil && slices.Contains(sch.Type, schemaType) +} + +func schemaArrayItems(sch *base.Schema) *base.Schema { + if sch == nil || sch.Items == nil || !sch.Items.IsA() || sch.Items.A == nil { + return nil + } + return sch.Items.A.Schema() +} + +func schemaPreservesStringValue(sch *base.Schema) bool { + if schemaTypeIncludes(sch, String) { + return true + } + return schemaTypeIncludes(sch, Array) && schemaTypeIncludes(schemaArrayItems(sch), String) +} + +// DeepObjectAllowsMultipleValues reports whether repeated values are allowed for a deepObject +// property. It preserves existing top-level array/additionalProperties behavior and adds support +// for nested properties declared as arrays. +func DeepObjectAllowsMultipleValues(objectSchema *base.Schema, qp *QueryParam) bool { + if objectSchema == nil { + return false + } + if schemaTypeIncludes(objectSchema, Array) { + return true + } + propertyPath := queryParamPropertyPath(qp) + if schemaTypeIncludes(schemaForPropertyPath(objectSchema, propertyPath), Array) { + return true + } + return schemaTypeIncludes(getAdditionalPropertiesSchema(objectSchema), Array) +} + +func schemaForCastingPath(objectSchema *base.Schema, propertyPath []string) *base.Schema { + propSchema := schemaForPropertyPath(objectSchema, propertyPath) if propSchema != nil { - for _, t := range propSchema.Type { - if t == String { - return v + return propSchema + } + if schemaTypeIncludes(objectSchema, Array) { + return objectSchema + } + return nil +} + +func castWithSchemaPath(v string, objectSchema *base.Schema, propertyPath []string) any { + if schemaPreservesStringValue(schemaForCastingPath(objectSchema, propertyPath)) { + return v + } + if len(propertyPath) == 1 { + return castWithSchema(v, objectSchema, propertyPath[0]) + } + return cast(v) +} + +func deepObjectPathHasPrefix(prefix, path []string) bool { + if len(prefix) >= len(path) { + return false + } + for i := range prefix { + if prefix[i] != path[i] { + return false + } + } + return true +} + +// DeepObjectPathConflict reports whether any deepObject property path is also used +// as a prefix for a nested path, such as obj[nested] and obj[nested][child]. +func DeepObjectPathConflict(values []*QueryParam) (prefixParam, nestedParam *QueryParam, ok bool) { + for i := range values { + leftPath := queryParamDeepObjectPath(values[i]) + if len(leftPath) == 0 { + continue + } + for j := i + 1; j < len(values); j++ { + rightPath := queryParamDeepObjectPath(values[j]) + if len(rightPath) == 0 || slices.Equal(leftPath, rightPath) { + continue + } + if deepObjectPathHasPrefix(leftPath, rightPath) { + return values[i], values[j], true + } + if deepObjectPathHasPrefix(rightPath, leftPath) { + return values[j], values[i], true } } } - return cast(v) + return nil, nil, false +} + +func setNestedDeepObjectValue(target map[string]interface{}, propertyPath []string, value any) bool { + if len(propertyPath) == 0 { + target[""] = value + return true + } + current := target + for _, propertyName := range propertyPath[:len(propertyPath)-1] { + next, ok := current[propertyName].(map[string]interface{}) + if !ok { + if existing, exists := current[propertyName]; exists { + current[propertyName] = []interface{}{existing, value} + return false + } + next = make(map[string]interface{}) + current[propertyName] = next + } + current = next + } + + propertyName := propertyPath[len(propertyPath)-1] + if existing, exists := current[propertyName]; exists { + if _, existingIsMap := existing.(map[string]interface{}); existingIsMap { + if _, valueIsMap := value.(map[string]interface{}); !valueIsMap { + current[propertyName] = []interface{}{existing, value} + return false + } + } + } + current[propertyName] = value + return true } // constructKVFromDelimited is the shared implementation for constructing key=value maps @@ -258,56 +452,26 @@ func constructParamMapFromDelimitedEncoding(values []*QueryParam, delimiter stri func ConstructParamMapFromDeepObjectEncoding(values []*QueryParam, sch *base.Schema) map[string]interface{} { decoded := make(map[string]interface{}) for _, v := range values { + propertyPath := queryParamPropertyPath(v) castForProp := func(val string) any { - return castWithSchema(val, sch, v.Property) + return castWithSchemaPath(val, sch, propertyPath) } - if decoded[v.Key] == nil { - props := make(map[string]interface{}) - rawValues := make([]interface{}, len(v.Values)) - for i := range v.Values { - rawValues[i] = castForProp(v.Values[i]) - } - // check if the schema for the param is an array - if sch != nil && slices.Contains(sch.Type, Array) { - props[v.Property] = rawValues - } - // check if schema has additional properties defined as an array - if sch != nil && sch.AdditionalProperties != nil && - sch.AdditionalProperties.IsA() { - s := sch.AdditionalProperties.A.Schema() - if s != nil && - slices.Contains(s.Type, Array) { - props[v.Property] = rawValues - } - } - - if len(props) == 0 { - props[v.Property] = castForProp(v.Values[0]) - } + props, ok := decoded[v.Key].(map[string]interface{}) + if !ok { + props = make(map[string]interface{}) decoded[v.Key] = props - } else { - added := false - rawValues := make([]interface{}, len(v.Values)) - for i := range v.Values { - rawValues[i] = castForProp(v.Values[i]) - } - // check if the schema for the param is an array - if sch != nil && slices.Contains(sch.Type, Array) { - decoded[v.Key].(map[string]interface{})[v.Property] = rawValues - added = true - } - // check if schema has additional properties defined as an array - if sch != nil && sch.AdditionalProperties != nil && - sch.AdditionalProperties.IsA() && - slices.Contains(sch.AdditionalProperties.A.Schema().Type, Array) { - decoded[v.Key].(map[string]interface{})[v.Property] = rawValues - added = true - } - if !added { - decoded[v.Key].(map[string]interface{})[v.Property] = castForProp(v.Values[0]) - } } + + rawValues := make([]interface{}, len(v.Values)) + for i := range v.Values { + rawValues[i] = castForProp(v.Values[i]) + } + if DeepObjectAllowsMultipleValues(sch, v) { + setNestedDeepObjectValue(props, propertyPath, rawValues) + continue + } + setNestedDeepObjectValue(props, propertyPath, castForProp(v.Values[0])) } return decoded } diff --git a/helpers/parameter_utilities_test.go b/helpers/parameter_utilities_test.go index 5944958..e390866 100644 --- a/helpers/parameter_utilities_test.go +++ b/helpers/parameter_utilities_test.go @@ -535,6 +535,284 @@ func TestConstructParamMapFromDeepObjectEncoding_ElseCase(t *testing.T) { require.Equal(t, int64(999), decoded["key1"].(map[string]interface{})["prop3"]) } +func TestParseDeepObjectKey(t *testing.T) { + tests := []struct { + name string + key string + expectedBase string + expectedPath []string + expectedOK bool + }{ + { + name: "flat", + key: "obj[root]", + expectedBase: "obj", + expectedPath: []string{"root"}, + expectedOK: true, + }, + { + name: "nested", + key: "obj[nested][child]", + expectedBase: "obj", + expectedPath: []string{"nested", "child"}, + expectedOK: true, + }, + { + name: "plain key", + key: "obj", + expectedOK: false, + }, + { + name: "empty segment", + key: "obj[]", + expectedOK: false, + }, + { + name: "trailing text", + key: "obj[root]extra", + expectedOK: false, + }, + { + name: "missing closing bracket", + key: "obj[root", + expectedOK: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + baseName, propertyPath, ok := ParseDeepObjectKey(tc.key) + require.Equal(t, tc.expectedOK, ok) + require.Equal(t, tc.expectedBase, baseName) + require.Equal(t, tc.expectedPath, propertyPath) + }) + } +} + +func TestConstructParamMapFromDeepObjectEncoding_NestedObject(t *testing.T) { + sch := &base.Schema{ + Type: []string{"object"}, + Properties: orderedmap.ToOrderedMap(map[string]*base.SchemaProxy{ + "root": base.CreateSchemaProxy(&base.Schema{Type: []string{"string"}}), + "nested": base.CreateSchemaProxy(&base.Schema{ + Type: []string{"object"}, + Properties: orderedmap.ToOrderedMap(map[string]*base.SchemaProxy{ + "child": base.CreateSchemaProxy(&base.Schema{Type: []string{"string"}}), + "count": base.CreateSchemaProxy(&base.Schema{Type: []string{"integer"}}), + }), + }), + }), + } + values := []*QueryParam{ + {Key: "obj", Values: []string{"test1"}, Property: "root", PropertyPath: []string{"root"}}, + {Key: "obj", Values: []string{"10"}, Property: "nested", PropertyPath: []string{"nested", "child"}}, + {Key: "obj", Values: []string{"42"}, Property: "nested", PropertyPath: []string{"nested", "count"}}, + } + + decoded := ConstructParamMapFromDeepObjectEncoding(values, sch) + obj := decoded["obj"].(map[string]interface{}) + nested := obj["nested"].(map[string]interface{}) + + require.Equal(t, "test1", obj["root"]) + require.Equal(t, "10", nested["child"]) + require.Equal(t, int64(42), nested["count"]) +} + +func TestConstructParamMapFromDeepObjectEncoding_NestedArray(t *testing.T) { + sch := &base.Schema{ + Type: []string{"object"}, + Properties: orderedmap.ToOrderedMap(map[string]*base.SchemaProxy{ + "nested": base.CreateSchemaProxy(&base.Schema{ + Type: []string{"object"}, + Properties: orderedmap.ToOrderedMap(map[string]*base.SchemaProxy{ + "tags": base.CreateSchemaProxy(&base.Schema{ + Type: []string{"array"}, + Items: &base.DynamicValue[*base.SchemaProxy, bool]{ + A: base.CreateSchemaProxy(&base.Schema{Type: []string{"string"}}), + }, + }), + }), + }), + }), + } + values := []*QueryParam{ + {Key: "obj", Values: []string{"123", "456"}, Property: "nested", PropertyPath: []string{"nested", "tags"}}, + } + + decoded := ConstructParamMapFromDeepObjectEncoding(values, sch) + obj := decoded["obj"].(map[string]interface{}) + nested := obj["nested"].(map[string]interface{}) + + require.Equal(t, []interface{}{"123", "456"}, nested["tags"]) + require.True(t, DeepObjectAllowsMultipleValues(sch, values[0])) +} + +func TestConstructParamMapFromDeepObjectEncoding_NestedAdditionalPropertiesArray(t *testing.T) { + sch := &base.Schema{ + Type: []string{"object"}, + Properties: orderedmap.ToOrderedMap(map[string]*base.SchemaProxy{ + "filters": base.CreateSchemaProxy(&base.Schema{ + Type: []string{"object"}, + AdditionalProperties: &base.DynamicValue[*base.SchemaProxy, bool]{ + A: base.CreateSchemaProxy(&base.Schema{ + Type: []string{"array"}, + Items: &base.DynamicValue[*base.SchemaProxy, bool]{ + A: base.CreateSchemaProxy(&base.Schema{Type: []string{"string"}}), + }, + }), + }, + }), + }), + } + values := []*QueryParam{ + {Key: "obj", Values: []string{"123", "456"}, Property: "filters", PropertyPath: []string{"filters", "tag"}}, + } + + decoded := ConstructParamMapFromDeepObjectEncoding(values, sch) + obj := decoded["obj"].(map[string]interface{}) + filters := obj["filters"].(map[string]interface{}) + + require.Equal(t, []interface{}{"123", "456"}, filters["tag"]) + require.True(t, DeepObjectAllowsMultipleValues(sch, values[0])) +} + +func TestDeepObjectPathConflict(t *testing.T) { + tests := []struct { + name string + values []*QueryParam + expect bool + prefixPath []string + nestedPath []string + }{ + { + name: "scalar before nested", + values: []*QueryParam{ + {Key: "obj", Values: []string{"bad"}, Property: "nested", PropertyPath: []string{"nested"}}, + {Key: "obj", Values: []string{"ok"}, Property: "nested", PropertyPath: []string{"nested", "child"}}, + }, + expect: true, + prefixPath: []string{"nested"}, + nestedPath: []string{"nested", "child"}, + }, + { + name: "nested before scalar", + values: []*QueryParam{ + {Key: "obj", Values: []string{"ok"}, Property: "nested", PropertyPath: []string{"nested", "child"}}, + {Key: "obj", Values: []string{"bad"}, Property: "nested", PropertyPath: []string{"nested"}}, + }, + expect: true, + prefixPath: []string{"nested"}, + nestedPath: []string{"nested", "child"}, + }, + { + name: "same array path", + values: []*QueryParam{ + {Key: "obj", Values: []string{"alpha"}, Property: "nested", PropertyPath: []string{"nested", "tags"}}, + {Key: "obj", Values: []string{"beta"}, Property: "nested", PropertyPath: []string{"nested", "tags"}}, + }, + }, + { + name: "sibling nested paths", + values: []*QueryParam{ + {Key: "obj", Values: []string{"ok"}, Property: "nested", PropertyPath: []string{"nested", "child"}}, + {Key: "obj", Values: []string{"ok"}, Property: "nested", PropertyPath: []string{"nested", "other"}}, + }, + }, + { + name: "different roots", + values: []*QueryParam{ + {Key: "obj", Values: []string{"ok"}, Property: "nested", PropertyPath: []string{"nested"}}, + {Key: "obj", Values: []string{"ok"}, Property: "other", PropertyPath: []string{"other", "child"}}, + }, + }, + { + name: "empty path is ignored", + values: []*QueryParam{ + {Key: "obj", Values: []string{"ignored"}}, + {Key: "obj", Values: []string{"ok"}, Property: "nested", PropertyPath: []string{"nested", "child"}}, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + prefixParam, nestedParam, ok := DeepObjectPathConflict(tc.values) + require.Equal(t, tc.expect, ok) + if !tc.expect { + return + } + require.Equal(t, tc.prefixPath, prefixParam.PropertyPath) + require.Equal(t, tc.nestedPath, nestedParam.PropertyPath) + }) + } +} + +func TestQueryParamDeepObjectPath(t *testing.T) { + require.Nil(t, queryParamDeepObjectPath(nil)) + require.Nil(t, queryParamDeepObjectPath(&QueryParam{})) + require.Equal(t, []string{"root"}, queryParamDeepObjectPath(&QueryParam{Property: "root"})) + require.Equal(t, []string{"root", "child"}, queryParamDeepObjectPath(&QueryParam{ + Property: "root", + PropertyPath: []string{"root", "child"}, + })) +} + +func TestSetNestedDeepObjectValue_PreservesConflicts(t *testing.T) { + t.Run("empty path", func(t *testing.T) { + target := make(map[string]interface{}) + + require.True(t, setNestedDeepObjectValue(target, nil, "root")) + require.Equal(t, "root", target[""]) + }) + + t.Run("scalar before nested", func(t *testing.T) { + target := make(map[string]interface{}) + + require.True(t, setNestedDeepObjectValue(target, []string{"nested"}, "bad")) + require.False(t, setNestedDeepObjectValue(target, []string{"nested", "child"}, "ok")) + require.IsType(t, []interface{}{}, target["nested"]) + }) + + t.Run("nested before scalar", func(t *testing.T) { + target := make(map[string]interface{}) + + require.True(t, setNestedDeepObjectValue(target, []string{"nested", "child"}, "ok")) + require.False(t, setNestedDeepObjectValue(target, []string{"nested"}, "bad")) + require.IsType(t, []interface{}{}, target["nested"]) + }) +} + +func TestConstructParamMapFromDeepObjectEncoding_NestedPathConflict(t *testing.T) { + tests := []struct { + name string + values []*QueryParam + }{ + { + name: "scalar before nested", + values: []*QueryParam{ + {Key: "obj", Values: []string{"bad"}, Property: "nested", PropertyPath: []string{"nested"}}, + {Key: "obj", Values: []string{"ok"}, Property: "nested", PropertyPath: []string{"nested", "child"}}, + }, + }, + { + name: "nested before scalar", + values: []*QueryParam{ + {Key: "obj", Values: []string{"ok"}, Property: "nested", PropertyPath: []string{"nested", "child"}}, + {Key: "obj", Values: []string{"bad"}, Property: "nested", PropertyPath: []string{"nested"}}, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + decoded := ConstructParamMapFromDeepObjectEncoding(tc.values, nil) + obj := decoded["obj"].(map[string]interface{}) + + require.IsType(t, []interface{}{}, obj["nested"]) + }) + } +} + func TestConstructKVFromLabelEncoding(t *testing.T) { // Test case 1: Empty input string values := "" @@ -924,6 +1202,17 @@ func TestConstructParamMapFromDeepObjectEncoding_WithSchema(t *testing.T) { decoded := ConstructParamMapFromDeepObjectEncoding(values, sch) require.Equal(t, "123", decoded["key1"].(map[string]interface{})["prop1"]) }) + + t.Run("preserves string values when root array items are strings", func(t *testing.T) { + sch := &base.Schema{ + Type: []string{"array"}, + Items: &base.DynamicValue[*base.SchemaProxy, bool]{ + A: base.CreateSchemaProxy(&base.Schema{Type: []string{"string"}}), + }, + } + + require.Equal(t, "123", castWithSchema("123", sch, "unknown")) + }) } func TestConstructParamMapFromPipeEncodingWithSchema(t *testing.T) { diff --git a/helpers/path_finder.go b/helpers/path_finder.go index 370dd63..2915579 100644 --- a/helpers/path_finder.go +++ b/helpers/path_finder.go @@ -27,7 +27,7 @@ func ExtractJSONPathFromValidationError(e *jsonschema.ValidationError) string { for _, seg := range e.InstanceLocation { switch { case isNumeric(seg): - b.WriteString(fmt.Sprintf("[%s]", seg)) + fmt.Fprintf(&b, "[%s]", seg) case isSimpleIdentifier(seg): b.WriteByte('.') @@ -129,7 +129,7 @@ func ExtractJSONPathFromInstanceLocation(instanceLocation []string) string { for _, seg := range instanceLocation { switch { case isNumeric(seg): - b.WriteString(fmt.Sprintf("[%s]", seg)) + fmt.Fprintf(&b, "[%s]", seg) case isSimpleIdentifier(seg): b.WriteByte('.') diff --git a/parameters/query_parameters.go b/parameters/query_parameters.go index 23f5a75..5b24d6d 100644 --- a/parameters/query_parameters.go +++ b/parameters/query_parameters.go @@ -69,14 +69,13 @@ func (v *paramValidator) ValidateQueryParamsWithPathItem(request *http.Request, Key: qKey, Values: qVal, }) - } else if strings.IndexRune(qKey, '[') > 0 && strings.IndexRune(qKey, ']') > 0 { + } else if stripped, propertyPath, ok := helpers.ParseDeepObjectKey(qKey); ok { // check if the param is encoded as a property / deepObject - stripped := qKey[:strings.IndexRune(qKey, '[')] - value := qKey[strings.IndexRune(qKey, '[')+1 : strings.IndexRune(qKey, ']')] queryParams[stripped] = append(queryParams[stripped], &helpers.QueryParam{ - Key: stripped, - Values: qVal, - Property: value, + Key: stripped, + Values: qVal, + Property: propertyPath[0], + PropertyPath: propertyPath, }) } else { queryParams[qKey] = append(queryParams[qKey], &helpers.QueryParam{ diff --git a/parameters/query_parameters_test.go b/parameters/query_parameters_test.go index d9307ee..1bc9ca7 100644 --- a/parameters/query_parameters_test.go +++ b/parameters/query_parameters_test.go @@ -3261,7 +3261,7 @@ paths: } // https://github.com/pb33f/wiretap/issues/82 -func TestNewValidator_QueryParamValidateStyle_DeepObjectAdditionalPropertiesArray_Fail(t *testing.T) { +func TestNewValidator_QueryParamValidateStyle_DeepObjectAdditionalPropertiesArrayNumericStrings(t *testing.T) { spec := `openapi: 3.1.0 paths: /anything/queryParams/deepObject/map: @@ -3292,9 +3292,8 @@ paths: "https://things.com/anything/queryParams/deepObject/map?mapArrParam[test2]=23&mapArrParam[test2]=test4&mapArrParam[test]=test&mapArrParam[test]=test2", nil) valid, errors := v.ValidateQueryParams(request) - assert.False(t, valid) - assert.Len(t, errors, 1) - assert.Equal(t, "got number, want string", errors[0].SchemaValidationErrors[0].Reason) + assert.True(t, valid) + assert.Len(t, errors, 0) } // https://github.com/pb33f/wiretap/issues/83 @@ -3392,6 +3391,251 @@ components: " however it failed to pass a schema validation", errors[0].Reason) } +// https://github.com/pb33f/libopenapi-validator/issues/83 +func TestNewValidator_QueryParamValidateStyle_NestedDeepObject(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /path: + get: + parameters: + - name: obj + in: query + style: deepObject + schema: + type: object + properties: + root: + type: string + nested: + type: object + properties: + child: + type: string + required: true + responses: + "200": + description: OK` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + m, err := doc.BuildV3Model() + require.NoError(t, err) + + v := NewParameterValidator(&m.Model) + request, _ := http.NewRequest(http.MethodGet, + "https://things.com/path?obj[root]=test1&obj[nested][child]=test2", nil) + + valid, errors := v.ValidateQueryParams(request) + assert.True(t, valid) + assert.Len(t, errors, 0) +} + +func TestNewValidator_QueryParamValidateStyle_NestedDeepObjectInvalidLeafType(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /path: + get: + parameters: + - name: obj + in: query + style: deepObject + schema: + type: object + properties: + nested: + type: object + properties: + child: + type: boolean + required: true + responses: + "200": + description: OK` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + m, err := doc.BuildV3Model() + require.NoError(t, err) + + v := NewParameterValidator(&m.Model) + request, _ := http.NewRequest(http.MethodGet, + "https://things.com/path?obj[nested][child]=not-a-bool", nil) + + valid, errors := v.ValidateQueryParams(request) + assert.False(t, valid) + require.Len(t, errors, 1) + require.NotEmpty(t, errors[0].SchemaValidationErrors) + assert.Equal(t, "got string, want boolean", errors[0].SchemaValidationErrors[0].Reason) + assert.Equal(t, "$.nested.child", errors[0].SchemaValidationErrors[0].FieldPath) +} + +func TestNewValidator_QueryParamValidateStyle_NestedDeepObjectRequiredLeaf(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /path: + get: + parameters: + - name: obj + in: query + style: deepObject + schema: + type: object + required: [nested] + properties: + nested: + type: object + required: [child] + properties: + child: + type: string + other: + type: string + required: true + responses: + "200": + description: OK` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + m, err := doc.BuildV3Model() + require.NoError(t, err) + + v := NewParameterValidator(&m.Model) + request, _ := http.NewRequest(http.MethodGet, + "https://things.com/path?obj[nested][other]=value", nil) + + valid, errors := v.ValidateQueryParams(request) + assert.False(t, valid) + require.Len(t, errors, 1) + require.NotEmpty(t, errors[0].SchemaValidationErrors) + assert.Contains(t, errors[0].SchemaValidationErrors[0].Reason, "missing") +} + +func TestNewValidator_QueryParamValidateStyle_NestedDeepObjectArray(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /path: + get: + parameters: + - name: obj + in: query + style: deepObject + schema: + type: object + properties: + nested: + type: object + properties: + tags: + type: array + items: + type: string + required: true + responses: + "200": + description: OK` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + m, err := doc.BuildV3Model() + require.NoError(t, err) + + v := NewParameterValidator(&m.Model) + request, _ := http.NewRequest(http.MethodGet, + "https://things.com/path?obj[nested][tags]=123&obj[nested][tags]=456", nil) + + valid, errors := v.ValidateQueryParams(request) + assert.True(t, valid) + assert.Len(t, errors, 0) +} + +func TestNewValidator_QueryParamValidateStyle_NestedDeepObjectAdditionalPropertiesArray(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /path: + get: + parameters: + - name: obj + in: query + style: deepObject + schema: + type: object + properties: + filters: + type: object + additionalProperties: + type: array + items: + type: string + required: true + responses: + "200": + description: OK` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + m, err := doc.BuildV3Model() + require.NoError(t, err) + + v := NewParameterValidator(&m.Model) + request, _ := http.NewRequest(http.MethodGet, + "https://things.com/path?obj[filters][tag]=123&obj[filters][tag]=456", nil) + + valid, errors := v.ValidateQueryParams(request) + assert.True(t, valid) + assert.Len(t, errors, 0) +} + +func TestNewValidator_QueryParamValidateStyle_NestedDeepObjectPathConflict(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /path: + get: + parameters: + - name: obj + in: query + style: deepObject + schema: + type: object + properties: + nested: + type: object + properties: + child: + type: string + required: true + responses: + "200": + description: OK` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + m, err := doc.BuildV3Model() + require.NoError(t, err) + + v := NewParameterValidator(&m.Model) + request, _ := http.NewRequest(http.MethodGet, + "https://things.com/path?obj[nested]=bad&obj[nested][child]=ok", nil) + + valid, validationErrors := v.ValidateQueryParams(request) + assert.False(t, valid) + require.NotEmpty(t, validationErrors) + assert.Contains(t, validationErrors[0].Reason, "property path 'nested'") + assert.Contains(t, validationErrors[0].Reason, "'nested.child'") +} + func TestNewValidator_ValidateRawMap(t *testing.T) { spec := `openapi: 3.1.0 paths: diff --git a/parameters/validation_functions.go b/parameters/validation_functions.go index ff4749e..069277a 100644 --- a/parameters/validation_functions.go +++ b/parameters/validation_functions.go @@ -6,7 +6,6 @@ package parameters import ( "encoding/json" "fmt" - "slices" "strconv" "strings" @@ -249,26 +248,19 @@ func ValidateQueryArray( // ValidateQueryParamStyle will validate a query parameter by style func ValidateQueryParamStyle(param *v3.Parameter, as []*helpers.QueryParam) []*errors.ValidationError { var validationErrors []*errors.ValidationError + if param.Style == helpers.DeepObject { + if prefixParam, nestedParam, ok := helpers.DeepObjectPathConflict(as); ok { + return []*errors.ValidationError{errors.InvalidDeepObjectPathConflict(param, prefixParam, nestedParam)} + } + } stopValidation: for _, qp := range as { for i := range qp.Values { switch param.Style { case helpers.DeepObject: // check if the object has additional properties defined that treat this as an array - if param.Schema != nil { - pSchema := param.Schema.Schema() - if slices.Contains(pSchema.Type, helpers.Array) { - continue - } - if pSchema.AdditionalProperties != nil && pSchema.AdditionalProperties.IsA() { - addPropSchema := pSchema.AdditionalProperties.A.Schema() - if len(addPropSchema.Type) > 0 { - if slices.Contains(addPropSchema.Type, helpers.Array) { - // an array can have more than one value. - continue - } - } - } + if param.Schema != nil && helpers.DeepObjectAllowsMultipleValues(param.Schema.Schema(), qp) { + continue } if len(qp.Values) > 1 { validationErrors = append(validationErrors, errors.InvalidDeepObject(param, qp))