Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: validating cookie parameter for req-validator #225

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: validating cookie parameter for req-validator #225

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9bbc21c
fix: validating cookie parameter for req-validator
Prashansa-K Nov 11, 2024
b67debf
fix: validating cookie parameter for req-validator
Prashansa-K Nov 13, 2024
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
8 changes: 0 additions & 8 deletions openapi2kong/oas3_testfiles/13-request-validator-plugin.expected.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,14 +190,6 @@
"schema": "{\"type\":\"integer\"}",
"style": "simple"
},
{
"explode": true,
"in": "cookie",
"name": "cookieid",
"required": true,
"schema": "{\"type\":\"integer\"}",
"style": "form"
},
{
"explode": false,
"in": "path",
Expand Down
3 changes: 3 additions & 0 deletions openapi2kong/oas3_testfiles/13-request-validator-plugin.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ paths:
schema:
type: integer
required: true
# This would not be added to the req-validator plugin config
# as cookie type is not supported yet.
# A warning would be logged and this parameter would be ignored.
- in: cookie
name: cookieid
schema:
Expand Down
15 changes: 14 additions & 1 deletion openapi2kong/validator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,19 @@ func generateParameterSchema(operation *v3.Operation, path *v3.PathItem,

result := make([]map[string]interface{}, len(combinedParameters))
i := 0
invalidParamCounts := 0

for _, parameter := range combinedParameters {
if parameter != nil {
if parameter.In == "cookie" {
logbasics.Info(`cookie parameters are not supported for request-validator plugin;
choose either path, query or header`)
Comment on lines +73 to +74
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these backticks will preserve the whitespace and newline for the log message, which is not what we want. Also the message can be a bit clearer about the consequences I think, how about "cookie parameters are not supported by the request-validator plugin; validation will be skipped"


invalidParamCounts++

continue
}

style := getDefaultParamStyle(parameter.Style, parameter.In)

var explode bool
Expand All @@ -81,6 +91,7 @@ func generateParameterSchema(operation *v3.Operation, path *v3.PathItem,
paramConf["style"] = style
paramConf["explode"] = explode
paramConf["in"] = parameter.In

if parameter.In == "path" {
paramConf["name"] = sanitizeRegexCapture(parameter.Name, insoCompat)
} else {
Expand Down Expand Up @@ -109,7 +120,9 @@ func generateParameterSchema(operation *v3.Operation, path *v3.PathItem,
}
}

return result, nil
// This ensures that we don't return nulls in the map, in case of invalid parameters
// indexing makes sure that order is maintained and nulls are in the end
return result[:len(result)-invalidParamCounts], nil
}

func parseMediaType(mediaType string) (string, string, error) {
Expand Down
Loading