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

URI-encoded path parameters get broken #256

Closed
TorstenForth opened this issue Mar 6, 2020 · 7 comments
Closed

URI-encoded path parameters get broken #256

TorstenForth opened this issue Mar 6, 2020 · 7 comments

Comments

@TorstenForth
Copy link

Hi,

I submit a string containing the '/' character with Swagger UI in the browser (as a path parameter).
When I receive the string in req.params, the string contains the URI-encoded '%2F'.

I was debugging into the source code and found out that the req.params (which are URI-decoded by Express) are overwritten with the original (not URI-decoded) values.
The params are overwritten in the file src/index.ts, line 140 of Commit 00273f7

``

  if (openapi?.pathParams) {
    const { pathParams } = openapi;
    // override path params
    req.params[name] = pathParams[name] || req.params[name];
  }

``

@cdimascio
Copy link
Owner

@TorstenForth thanks for the issue. perhaps the validator (like express) must also decode the path param prior to overriding it. is this effectively what you are suggesting? also if you feel so inclined, feel free to make the change and submit a PR. i'm happy to review

@TorstenForth
Copy link
Author

@cdimascio Thanks for the quick reply. Decoding the path params would work for me. I'm not familiar with the internals of the validator. Why are the parameters actually overwritten?

Is it ok to change line 140 to
req.params[name] = decodeURIComponent(pathParams[name]) || req.params[name];?

I tried to push a branch with the fix for the pull request but I gut a 'Permission denied' error.

@cdimascio
Copy link
Owner

cdimascio commented Mar 6, 2020

Sure, please give that a try. If all 200+ tests pass, we should be good shape, but i'll review to be sure.

To create the PR
tldr

  • Find a project you want to contribute to
  • Fork it
  • Clone it to your local system
  • Make a new branch
  • Make your changes
  • Push it back to your repo
  • From the Github UI, Click the Compare & pull request button (NOTE: this button will be present for some window of time after you pushed. If the button no longer appears, click pull request button and choose branches manually)
  • From the Github UI, Click Create pull request to open a new pull request

Detailed steps with example here:

(I'll add this process to CONTRIBUTING.md as other have also asked)

it's done to keep the params synced with the spec schema, code

@cdimascio
Copy link
Owner

Also, if you don't, please provide the relevant aspects of your spec, your express handler function, and the http call you are making. this will help me to repro

@cdimascio
Copy link
Owner

cdimascio commented Mar 6, 2020

If your change does not work, another thing to try might be to modify
openapi.metadata.ts line 55. This is where the path param value is extracted. It may need to be decoded there as it's much earlier in the process. the override occurs very late.

something like

const paramsVals = matchedRoute.slice(1).map(decodeURIComponent);

cdimascio pushed a commit that referenced this issue Mar 7, 2020
cdimascio added a commit that referenced this issue Mar 7, 2020
@cdimascio
Copy link
Owner

@TorstenForth i had a look at this issue. it should be resolved now. see v3.9.4. let me know how it goes. thanks!

@TorstenForth
Copy link
Author

@cdimascio Thank you very much. It works well now.

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