feat: keep original headers after validation

[mrnagydavid]

In this PR, I made it so that we do not replace the req.headers after validation.
This feature can be controlled by the opt.keepOriginal flag.

We decided to do so, because we want to validate only a subset of headers in NCB3, and replacing them on the Request object would break many parts of the application, meanwhile we do not want to validate every header at a single location.

[github-actions]

🎉 This PR is included in version 5.11.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[kirillgroshkov]

1. I was expecting that we'll need to pass convert: false to getValidationResult, so Joi would not mutate the object that we're passing to it. Maybe you checked with tests that it does not mutate anything? (I usually check so by doing _deepFreeze(obj), which would throw if it's attempted to be mutated)

2. I'm questioning the need for keepOriginal option, and its default, and if same default is applicable to the body/query/params vs headers.

Why?
a. body/query/params didn't have an issue with "mutation by default", so, new option is not strictly important.
b. the headers mutation you now default to true, right? (if I'm not mistaken) I find it dangerous, as "default should be safe" (non-mutating)

So, I thought maybe we just hard-code the behavior that headers validation is always non-mutating, while everything else is always mutating?
Or, if you don't like it, we may do "different default for different properties" (which I don't like much, as it'll less clear/obvious to the consumer)

[kirillgroshkov]

p3: I have a habit to explicitly document what is a default value of every option, e.g here I'd write like Defaults to false, which means it would **convert**/mutate the original value. Set to true to force non-mutating behavior.

Your description is good, but I like the emphasis on "what is default, and what happens when you flip the default".

[mrnagydavid]

was expecting that we'll need to pass convert: false to getValidationResult

The issue is not with conversion.
The issue is with 1] the removal of unknown (not validated) properties and 2] replacing req.headers with it.
If we would want to solve this via JOI config, then stripUnknown looks like a better candidate.

Having stripUnknown: false and convert: false would result in the desired effect.
But since this is a generic library, I would want to provide an overriding option.
Having a flag that overrides stripUnknown and convert would lead to more complicated code than what we have now.

=> backend-lib caused the issue by replacing the req.body/req.headers unconfigurably
~ it feels wrong to solve this via new JOI configurations when it can be solved in backend-lib

So, I thought maybe we just hard-code the behavior that headers...

I am not sure I understand or I think you misunderstand the current flow:

• validateRequest.body/params/query => they are unchanged
• validateRequest.header => has the keepOriginal flag set to true by default, and its overridable

p3: I have a habit to explicitly document what is a default value of every option

👍 You are right.
I deliberately didn't comment the default, since it is not "generally" true. But I could have done it.
PR
Will merge this if we settle the above points.