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

useRequestUrl: true breaks query parameters #941

Closed
g-radam opened this issue Jul 4, 2024 · 0 comments · Fixed by #942
Closed

useRequestUrl: true breaks query parameters #941

g-radam opened this issue Jul 4, 2024 · 0 comments · Fixed by #942

Comments

@g-radam
Copy link
Contributor

g-radam commented Jul 4, 2024

Describe the bug
The useRequestUrl option implemented #857 has a bug in that when set to true, schemas with query parameters defined will cause the validator to fail to find the route where the parameters are defined. The root cause seems to be within the openapi.metadata.xx file, in the lookupRoute function. See exerp below:

function lookupRoute(req, useRequestUrl) {
        const path = useRequestUrl ? req.url : req.originalUrl.split('?')[0]; ////////// Bug here

        // .. snip ..

        const regexp = (0, path_to_regexp_1.pathToRegexp)(expressRoute, keys, pathOpts);
        const matchedRoute = regexp.exec(path);
        if (matchedRoute) {
           // Need to get here to successfully validate the route
        }
}

Note the the code compares the path against a regex, and that regex ASUMES there is no ?parameters at the end of the path. When useRequestUrl is false, the ?parameters are stripped via: req.originalUrl.split('?')[0],
However when useRequestUrl is true, it uses req.url which does NOT strip the ?parameters, failing the regex checks.

I confirmed this by console logging the variables:
useRequestUrl: false => /some/path/here
useRequestUrl: true => /some/path/here?interval=600

And the comparing Regex: /^/some/path/here[/#?]?$/i

The solution I believe is updating the path declaration:

const path = useRequestUrl ? req.url.split('?')[0] : req.originalUrl.split('?')[0];

To Reproduce

paths:
  /some/path/here:
    get:
      summary: Does some work
      parameters:
        - $ref: '#/components/parameters/QueryParamterInterval'   # ?interval=600
      responses:
        '200':
          $ref: '#/components/responses/FooBarResponse'
router.use(
    OpenApiValidator.middleware({
        apiSpec: ....
        useRequestUrl: true, // Breaks when true 
    }),
);

Actual behavior

<pre>Not Found: not found<br> &nbsp; &nbsp;at /<path>/node_modules/express-openapi-validator/src/middlewares/openapi.metadata.ts:62:13<br> &nbsp; &nbsp;at /<path>/node_modules/express-openapi-validator/src/openapi.validator.ts:158:18<br> &nbsp; &nbsp;at processTicksAndRejections (node:internal/process/task_queues:95:5)</pre>

Expected behavior
The requests is validated / accepted and not erroneously blocked with a "Not Found" error.

Examples and context
An example or relevant context e.g. an OpenAPI snippet, an Express handler function snippet

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 a pull request may close this issue.

1 participant