fix(checkSchema): check all locations if in is not specified.

[nikli2009]

Description

When using checkSchema, I notice that the default option doesn't work as expected when the field is in req.query and in is not specified. (if in is empty, by default we should check all locations as official doc suggested here

Related issues:

• fixes Default sanitizer does not work with checkSchema when checking query #1071
• probably fixes buildCheckFunction default added only to req.body #1010

To-do list

• I have added tests for what I changed.

• This pull request is ready to merge.

[coveralls]

Coverage remained the same at 100.0% when pulling 989f0bb on nikli2009:master into 690cd63 on express-validator:master.

[fedeci]

Hey @nikli2009 can you add a few more tests to check if #1010 is fixed by this?

[nikli2009]

Hey @nikli2009 can you add a few more tests to check if #1010 is fixed by this?

hey man, gladly ! checking today

[nikli2009]

Hey @nikli2009 can you add a few more tests to check if #1010 is fixed by this?

Test added, could you help review and let me know if need to add more

[fedeci]

Some nitty comments but look fine. Thanks!
/cc @gustavohenke patch or minor? It actually corrects a bug(?) but may break something🤔

[nikli2009]

Tests updated + commits squashed 🥪🥪 .

[gustavohenke]

This is a breaking change.
For everybody using check() instead of the specialised functions, this can be a bit wild...

Test code:

const req = {};
check('foo').isNumeric().run(req).then(() => {
  console.log(validationResult(req).array());
});

Output:

[
  {
    value: undefined,
    msg: 'Invalid value',
    param: 'foo',
    location: 'body'
  },
  {
    value: undefined,
    msg: 'Invalid value',
    param: 'foo',
    location: 'cookies'
  },
  {
    value: undefined,
    msg: 'Invalid value',
    param: 'foo',
    location: 'headers'
  },
  {
    value: undefined,
    msg: 'Invalid value',
    param: 'foo',
    location: 'params'
  },
  {
    value: undefined,
    msg: 'Invalid value',
    param: 'foo',
    location: 'query'
  }
]

---

Looking at #1071, it's conceptually wrong that you don't pass in to the schema, and expect a certain request location to have the value you wanted.
You should probably use matchedData() instead.

But then, a few things I'm wondering:

• would it makes sense to differentiate between "all/multiple locations" vs "any location", so to appease to those that want the convenience of check?
• should we verify if the user actually tried to pass the validated field somewhere -- e.g. /some?query vs /some -- and prefer handling req.query instead of req.body?

[nikli2009]

Thanks for taking time to review this PR and sharing all the detail of API design @gustavohenke

• should we verify if the user actually tried to pass the validated field somewhere -- e.g. /some?query vs /some -- and prefer handling req.query instead of req.body?

Totally agree 👍 , we can check query if query isn't empty.
Also wondering for validator exists, scenarios like below ⬇️, should we check all locations if user doesn't specify in? 🤔

// when use check or checkSchema
check('anyKey').exists()
checkSchema({
   'anyKey': {
       exists: true
   }
})


// req is simply `/helloworld`
{
   body: {},
   query: {},
   params: {},
   headers: { ... stuff },
   cookies: { ...stuff }
}
// expected validation result 
// Option 1: [ notInBody ]
// Option 2: [ notInBody, notInQuery, notInParams ]
// Option 3: [ notInBody, notInQuery, notInParams, notInHeaders, notInCookies ]