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

unexpected result of matchedData() when using oneOf() #482

Closed
pkyeck opened this issue Dec 18, 2017 · 5 comments · Fixed by #506
Closed

unexpected result of matchedData() when using oneOf() #482

pkyeck opened this issue Dec 18, 2017 · 5 comments · Fixed by #506
Labels

Comments

@pkyeck
Copy link

pkyeck commented Dec 18, 2017

I'm trying to validate the token I pass to my REST endpoint. It could be passed in the HTTP headers , a cookie or as a POST parameter. So I'm checking like this:

oneOf(
  [
    header('Authorization')
      .trim()
      .matches(/Token([abcdef0-9]{40})/i),

    body('token')
      .trim()
      .matches(/[abcdef0-9]{40}/i),

    cookie('cookiename')
      .trim()
      .matches(/[abcdef0-9]{40}/i),
  ],
  'token is missing'
),

Now I was expecting the result of matchedData(req) to only include the one value that was validated correctly but the other two are in there as well (as undefined). Is this expected?

And it would be great if we could expose the result under the same name. I don't want to get authorization, token or cookiename but just e.g. token no matter which of these was set.

@gustavohenke
Copy link
Member

Hello @pkyeck! Thanks for using express-validator.
You have two different questions, so let's break them down into smaller pieces:

I was expecting the result of matchedData(req) to only include the one value that was validated correctly but the other two are in there as well (as undefined). Is this expected?

Currently, the tests don't say a thing about optional data (at least that's what I got from quickly going thru it), what is a shame!
But can you try using onlyValidData option? It should work.

And it would be great if we could expose the result under the same name. I don't want to get authorization, token or cookiename but just e.g. token no matter which of these was set.

Given those are within oneOf, this is probably acceptable, but I'm not sure how I would go about it.
Would probably have to add a few more options to make it work with most scenarios.

Otherwise, this is quite easy to achieve in user-land.

@pkyeck
Copy link
Author

pkyeck commented Dec 22, 2017

I'm already using the onlyValidData option - the undefined values are in there nonetheless.

For the second part: yep, did this in userland.

@gustavohenke
Copy link
Member

Hey @pkyeck, I would like to fix this for the next release (there's a good backlog of changes for a v5.0.0).

Are you up for a challenge? I began by adding a failing test in #506 -- if you have ideas, or would like to tackle this yourself, feel free to let me know!

@pkyeck
Copy link
Author

pkyeck commented Feb 7, 2018

was away from the computer the last couple of days. but thanks for fixing this!!

@lock
Copy link

lock bot commented May 31, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators May 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants