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

Request validator middleware throws for valid requests (query parsing) #369

Closed
dystopiandev opened this issue Sep 14, 2020 · 3 comments
Closed

Comments

@dystopiandev
Copy link
Contributor

Describe the bug
I use swagger-routes-express to parse and mount OAS3 paths as express routes. I mount the routes after using:

middleware({
  apiSpec: (validatedDefinition as unknown) as string,
  validateRequests: { allowUnknownQueryParameters: false },
  validateResponses: true,
  validateFormats: "full",
  $refParser: { mode: "dereference" },
  validateSecurity: true,
  operationHandlers: false
})

...but I get errors on all routes, whether or not they require any params.

Actual behavior
Throws Bad Request: request should have required property 'query'

Expected behavior
Shouldn't throw, because all is right by the spec.

I solved it temporarily by changing:

const valid = validator(Object.assign(Object.assign({}, req), { cookies }));

...to:

const valid = validator(Object.assign(Object.assign({query: {}}, req), { cookies }));

...in node_modules/express-openapi-validator/dist/middlewares/openapi.request.validator.js

...so I reckon changing this line to

const valid = validator({ query: {}, ...req, cookies });

...should resolve this.

@cdimascio
Copy link
Owner

cdimascio commented Sep 14, 2020

@dystopiandev interesting. this almost seems as if some middleware in the stack is nulling out the req.query. typically, its {} when not present. would you be willing to submit a PR with your proposed change? if so, check out CONTRIBUTING.md to get started

@dystopiandev
Copy link
Contributor Author

@dystopiandev interesting. this almost seems as if some middleware in the stack is nulling out the req.query. typically, its {} when not present. would you be willing to submit a PR with your proposed change? if so, check out CONTRIBUTING.md to get started

I'll send a PR shortly.

@dystopiandev
Copy link
Contributor Author

@cdimascio #371

cdimascio added a commit that referenced this issue Sep 15, 2020
cdimascio added a commit that referenced this issue Sep 28, 2020
* implement as standard express middleware

* conver to standard express middleware

* require options only

* update codacy

* update ncyrc

* update nyc

* update readme

* update README

* update README

* cleanup specs

* remove sync test

* update examples

* namspace errors and provide a default export

* alpha.2

* fix test

* publish changelog

* improve response type handling

* increment patch version

* improve async init with connect middleware

* update packages

* update deps

* fix operation handlers when installed as router middleware

* update README

* do not pass routes to path params

* update patch version

* add default resolver cache

* update alpha version

* update README

* fix for issue #369

* add test no query params

* increment alpha version

* increment alpha to beta

* update README

* Update README.md

* Update README.md

* increment beta version

* remove default export

* update import

* example lock

* update README

* ignore sav file

* add publish target

* add publish target

Co-authored-by: dystopiandev <redhart@dualsight.org>
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

No branches or pull requests

2 participants