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.

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

fix(oas3): request body validation #9698

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(oas3): request body validation #9698

wants to merge 1 commit into from

Conversation

glowcloud
Copy link
Contributor

Refs #9673

Request body with content type application/json should now correctly validate missing required values and incorrect value types, and show errors.

Screenshot 2024-03-14 at 12 19 33

I had an issue with making it work for application/x-www-form-urlencoded. I could validate types, which was missing before, but empty values would incorrectly show errors if the parameter type was different than string. This is because the value would be an empty string then, and the method would see that as an error. As such, I went back to the version that worked for application/json for now.

There is also one e2e test that will not pass, because it expects that a request body with a single space " " will be validated as correct. With my changes it's as invalid JSON:

Screenshot 2024-03-14 at 12 32 15

I'm not sure if I should update the test or if the previous behaviour was correct, and I should change the validation.

@glowcloud
Copy link
Contributor Author

glowcloud commented Mar 14, 2024

Here's where we set the error because of which the test fails:

if(typeof value === "string") {
try {
objectVal = JSON.parse(value)
} catch (e) {
errors.push("Parameter string value must be valid JSON")
return errors
}
}

Looks like we can't parse " ".

The same thing happens for OpenAPI 2.0 (without changes in this PR)

Screenshot 2024-03-14 at 16 21 53

Apart from that, even if that JSON error wasn't set, the test shouldn't pass because the schema has required properties:

Screenshot 2024-03-14 at 16 07 19

which we don't specify in the test.

We can change to the current behaviour by changing this

export const validateValues = (
state,
{
oas3RequestBody,
oas3RequestContentType,
oas3RequestBodyValue
}
) => {
if (oas3RequestContentType !== "application/json") {
return []
}
return validateParam(oas3RequestBody, oas3RequestBodyValue, {
bypassRequiredCheck: false,
isOAS3: true,
})
}

to something like this

export const validateValues = (
  state,
  { 
    oas3RequestBody, 
    oas3RequestContentType, 
    oas3RequestBodyValue 
  }
) => {
  if (oas3RequestContentType === "application/json") {
    try {
      JSON.parse(oas3RequestBodyValue)
      return validateParam(oas3RequestBody, oas3RequestBodyValue, {
        bypassRequiredCheck: false,
        isOAS3: true,
      })
    } catch (e) {
      return []
    }
  }
  return []
}

but if we're missing the required properties, or the JSON is malformed, ex. doing something like this

{
  "name": 123
}12345

the input won't be invalid. This is the current behaviour for 3.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant