Joi: Dangerously relying on the order of Object.keys() in Joi.extend

[vam]

Context

• _joi version_: 9.0.4

The functionality of Joi.extend(), more specifically the definition params for a rule, depends on the order of Object.keys(). That could potentially lead to problems, as the order is generally _not_ guaranteed.

The relevant part of the code: https://github.com/hapijs/joi/blob/1ea86e6ad16c218714b39209bac3be99a0c598dd/lib/index.js#L262-L282

[DavidTPate]

I'm not seeing how in the context of its use here that this would be an issue. Object.keys(params) is used to retrieve each of the keys within the object so they can be checked, the order of those parameters doesn't matter as it checks each one.

Are you able to provide a test case or example where this breaks?

[vam]

The order of the Object.keys() matters because of this line, where the arguments of the rule functions are mapped on to the arg object and then passed to the rule's validate function (L292: rule.validate.call(this, arg, value, state, options);)

However, after reading this article, I have the impression that the arbitrary order of the keys function is rather a theoretical problem (at least in ES6) and that the order is actually rather stable. In general, as long as Object.keys() returns the keys in the order of their definition, everything's fine.

For the sake of argument, I can give you a rather eccentric definition of a custom schema, that actually leads to a problem:

var customJoi = Joi.extend({
    name: 'number',
    base: Joi.number(),
    language: { inRange: 'Number not in range!' },
    rules: [{
        name: 'inRange',
        params: { a: Joi.number(), 2: Joi.number() },
        validate(params, value, state, options) {
            if (value < params['a'] || value > params[2]) {
                return this.createError('number.inRange', {}, state, options);
            }
            return value
        }
    }],
})
customJoi.number().inRange(0, 100).validate(10)

This won't validate. Whereas this definition would work:

...
params: { a: Joi.number(), b: Joi.number() },
validate(params, value, state, options) {
    if (value < params.a || value > params.b) {
        return this.createError('number.inRange', {}, state, options);
    }
    return value
}
...

[Marsup]

I knowingly did that, because yolo ¯_(ツ)_/¯

I'll assume vendors are responsible and it won't ever change, because it would presumably break many things across the web.

That makes for a way better API to rely on that rather than have an array of who knows what.

[vam]

Cool. (Hmm.. that kind of opens new coding dimensions for me..)

[Marsup]

If anyone is uncomfortable with that, go with a single object param, so you won't have to rely on ordering, like :

var customJoi = Joi.extend({
    name: 'number',
    base: Joi.number(),
    language: { inRange: 'Number not in range!' },
    rules: [{
        name: 'inRange',
        params: { options: Joi.object({ a: Joi.number(), 2: Joi.number() }).required() },
        validate(params, value, state, options) {
            if (value < params.options['a'] || value > params.options[2]) {
                return this.createError('number.inRange', {}, state, options);
            }
            return value
        }
    }],
})
customJoi.number().inRange({ a: 0, 2: 100 }).validate(10)

[kirillgroshkov]

Was also confused by that. Reading http://2ality.com/2015/10/property-traversal-order-es6.html didn't convince me, it looks more like a proposal rather than an established standard.

Anyway, I suggest to maybe improve documentation to show what to do in case of 2 parameters passes, which will make it immediately more clear.

ps: Joi is a joy to work with so far!

[Marsup]

@kirillgroshkov Added a justification, let me know if it's understandable and covers your concerns.

[kirillgroshkov]

@Marsup great! Perfectly understandable now. IMHO, the best documentation is example, and before it lacked example with more than 1 parameter.

[lock[bot]]

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.