Sanitizing arrays loses data except first item

[bincc]

Version: express-validator 6.2.0

Send POST data:

{
title: "test",
genre: ["aaa", "bbb", "ccc"]
}

After sanitizeBody("*").escape()
POST data changed to:

{
title: "test",
genre: "aaa"
}

And array data of genre lost!

[JustinChow]

Seeing the same issue in v6.14.5.

[fedeci]

Although sanitizeBody() is deprecated it also happens with new chain builders.
Related to #931 and #883
@gustavohenke do you think that this behaviour should be changed?

[favph]

@fedeci again, if it isn't changed, could you add it to the documentation? The behavior is a matter of taste, but it's surprising and nowhere mentioned. The documentation for .notEmpty says it checks for the value to be "a string with a length of 1 or bigger". This is misleading since you would assume the test fails if the value is an array and so not a string.

[fedeci]

@favph Sure I will, however to require a minimum array length you can use isArray({ min: 1 })

[favph]

@fedeci another thing regarding the Validator.js adapter:

// this is the function used to convert values to string
export function toString(value: any, deep = true): string {
  if (Array.isArray(value) && value.length && deep) {
    return toString(value[0], false);
  } else if (value instanceof Date) {
    return value.toISOString();
  } else if (value && typeof value === 'object' && value.toString) {
    return value.toString();
  } else if (value == null || (isNaN(value) && !value.length)) {
    return '';
  }

  return String(value);
}

I don't know if this warrants an issue, but I find it suboptimal that a user can easily (by supplying their own toString property with the query) trigger an exception (truthy => line 8, falsy => attempt to call by String(value) at the end), that has to be caught by express (if extended parsing is active, which is the default).

E.g. use the example here and:

curl 'localhost:8081?username\[0\]=foo&username\[toString\]=bar'

[fedeci]

@favph This is the error returned to me:

<pre>TypeError: value.toString is not a function<br>

And can be easily be fixed by checking for the type of toString.

} else if (value && typeof value === 'object' && typeof value.toString === 'function') {

edit: definitely it's not only that, I'm working on this!

---

edit: I have carefully analyzed this behavior and I honestly don't think it needs to be changed because it is not a validation error but a "user error". I mean, if we don't catch errors thrown from validator.js when options are incorrect and let the user to handle them from express I don't see why we should catch this one.
/cc @gustavohenke

[favph]

@fedeci

edit: definitely it's not only that, I'm working on this!

String(value) also attempts to call value.toString(), so it must be changed, too. Object.getPrototypeOf(value).toString.call(value) ?

I mean, if we don't catch errors thrown from validator.js when options are incorrect and let the user to handle them from express I don't see why we should catch this one.

OTOH this exception is caused by an internal call to an untrusted toString and so affects everyone. Not just those who introduced exceptions by their own code.

It's not that express-validator should catch exceptions, but that it should – if possible – avoid this call.

Again, thanks for looking into it!

[gustavohenke]

Hi hi, https://github.com/express-validator/express-validator/releases/tag/v7.0.0 is out with a fix for this 🙂
Please give it a try!