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 query coercion issue #571

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

Conversation

sabeslamidze
Copy link

@sabeslamidze sabeslamidze commented May 12, 2023

Fixed #554

Note that query parameters are coerced just fine by Ajv validators. But parameters.query is never assigned back to original request that is passed down to handlers.

However, this might be not desirable behaviour by default. Maybe it is better to add additional option on OpenAPIBackend initialization to allow modifying data during validation like this:

  const api = new OpenAPIBackend({
    definition: "./open-api-spec.json",
    validate: true,
    **modifyReqData: true**,
    ajvOpts: {
      coerceTypes: true
    }
    ...
  })

Pls let me know what you think.

@anttiviljami
Copy link
Member

@sabeslamidze I agree it'd be better to put this behind an option flag. 👍 Generally the less processing we make to the passed request object the better IMO.

@ziflex
Copy link

ziflex commented Dec 1, 2023

@anttiviljami don't you think if your schema says it must be a number, your code should receive it as a number? Why would I need to parse numbers (and to other types) manually in every handler?

@@ -307,6 +307,10 @@ export class OpenAPIValidator<D extends Document = Document> {
if (validate.errors) {
result.errors.push(...validate.errors);
}
else {
// set Ajv validator coerced query to request
req.query = parameters.query
Copy link

Choose a reason for hiding this comment

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

Path params are needed to be copied too.

@ydayagi
Copy link

ydayagi commented Dec 5, 2023

@sabeslamidze @anttiviljami @ziflex
I understand that you want to keep original request as is.
However, having a field of type number in code and then in run-time it is a string is not ok.
AJV has an option called 'coerceTypes'. You can support that option and that will solve the issue, no?
When can I expect this to be merged?

@ziflex
Copy link

ziflex commented Dec 7, 2023

@ydayagi no, I meant that I want this library to parse numbers for me. I do not want to parse params in every handler.

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

Successfully merging this pull request may close these issues.

Parameter query is not converted to correct type
4 participants