Coerced types are thrown away

[henhal]

When a request is handled, parseRequest() is called to parse the request using an operation. Then, the validator is called to validate the request. However, the parsed request is not passed to the validator; instead the validator parses the request again. It then validates the local parsed request using coerceTypes, which means AJV mutates that local parsed request only. This is then thrown away, meaning that the parsed request kept in the context and passed to the operation handlers does not have coerced types etc.

Example: An operation has a header with schema: {type: number}. On a raw request, of course all headers are strings. However, inside validateRequest(), the header is coerced so that foo: '42' becomes foo: 42, but this coercion happens on a local copy of the parsed request. The result is that once the parsed request reaches the operation handler, the headers are again all strings.

Shouldn't the validator take a parsed request instead of a raw request (which it currently internally parses one more time, then validates and potentially mutates it, before throwing it away), so that a parsed request could actually have coerced headers etc?

Edit: I just realized that ParsedRequest also actually only supports string params, so I guess this is somewhat deliberate? However, there are instances of ParsedRequest inside the validator which momentarily contain non-strings, since ajv coerces in-place, so that's not very robust. But again, that object is thrown away.
Expanding ParsedRequest into headers and other parameters of type string | number | boolean would be a breaking change, but it would allow parameters to be coerced properly. Let me know if PRs are welcome in this area, and if so whether you have any specific thoughts or constraints to keep in mind.

[mdebarros]

+1 for this.

The "coersion" was working on 3.6.3, but no longer in 3.9.2.

I have also confirmed that this impacts all parameters, i.e. query, path, headers body, etc.

[anttiviljami]

Yeah definitely sounds like an oversight on my part. Any chance to provide a PR for a fix? Would gladly merge :)

…

On Fri 4. Jun 2021 at 13.27, Miguel de Barros ***@***.***> wrote: +1 for this. The "cohesion" was working on 3.6.3, but no longer in 3.9.2. I have also confirmed that this impacts all parameters, i.e. query, path, headers body, etc. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#144 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABOSUMQ5HZ73H6TBJ2KQRZ3TRC2AVANCNFSM4Y5X2A5Q> .

[nklisch]

This seems to still be an open issue, as I am seeing this problem as well

[nklisch]

@anttiviljami The second PR on this issue seems rather stale, would you accept a new one to parse, and if the user choses, coerce the types via AJV coerceTypes option?

[nklisch]

Also, I'd like to update the Context object type so it can take in a type for the params, allowing typescript intellisense, like how Express requests work, since we are placing the parsed requests in that object.

[anttiviljami]

@nklisch Another PR would be appreciated indeed. 🙏 I like both ideas. 👍

[sabeslamidze]

Could this be a fix #571? Will be glad to collaborate on this and get it merge asap. Thank you!