Joi: Chaining string.insensitive() will not convert values with lowercase/uppercase modifiers

Created on 18 May 2017  路  21Comments  路  Source: sideway/joi

Context

  • node version: v6.10.2
  • joi version: v10.4.2
  • environment (node, browser): node
  • used with (hapi, standalone, ...): standalone
  • any other relevant information: macOS 10.12.14

What are you trying to achieve or the steps to reproduce ?

I thought it was fairly confusing that these options do not work together

const schema = Joi.string().valid(['A', 'B', 'C']).insensitive().lowercase()
schema.validate('A')
// { error: null, value: 'A' }
// or for uppercase
const anotherSchema = Joi.string().valid(['a', 'b', 'c']).insensitive().uppercase()
anotherSchema.validate('b')
// { error: null, value: 'b' }

Which result you had ?

After validation with schema, the value was 'A'
After validation with anotherSchema, the value was 'b'

What did you expect ?

After validation with schema, the value should be 'a'
After validation with anotherSchema, the value should be 'B'

Now its fine that there is a workaround to just convert the array to the correct casing and use:

Joi.string().valid(['a', 'b', 'c']).lowercase();
schema.validate('A')
// { error: null, value: 'a' }

but seems like odd behavior for this to not work with both. I can submit a PR if you agree.

Thanks for Joi!

breaking changes bug

All 21 comments

So essentially you want the validation portion to be insensitive() but you want to leverage .lowercase() as a data transformation step?

Yes, I would expect them both to work together. The fact that insensitive() and lowercase() do produce the same validation, but different transformations when used together is weird and awkward.

So, from looking at Joi primarily as a validator it seems weird to me to expect them both to be respected equally. When validating a string, you're simultaneously saying "I don't care what case it is" and "it must be lowercase".

That said, I think there is room for improvement on the coercion step. If the validation rule is insensitive I don't see any reason for either lowercase or uppercase to be applied to the value as well. I imagine a PR along those lines would be up for review :)

I agree with you that its a _little_ strange from a validation perspective. But the statement

"I don't care what case it is" and "it must be lowercase".

are not mutually exclusive. The lowercase() is a direct subset of the insensitive(), and I would expect that the most specific rule wins out if you chain them ( like an override ).

But like you said, this is really just about the conversion, and perhaps we need to look at what happens if the conversion doesn't happen? Does this make any sense if you have convert: false?

schema = Joi.string().valid(['a', 'b', 'c']).insensitive().lowercase();
schema.validate('A', { convert: false })
/// ???

This does seem really strange...because I'm not sure what the behavior should be.

Like you said

I would expect that the most specific rule wins out if you chain them

so the current validation behavior matches my personal expectation:

Joi.string().insensitive().lowercase().options({convert: false}).validate('Ay No');
// ValidationError: "value" must only contain lowercase characters

That's primarily why I could get behind a PR to apply the same idea to the data coercion/conversion. :)

Hey wait...

Joi.string().insensitive().lowercase().validate('Ay No')
// { error: null, value: 'ay no' }

It looks like it's already doing the thing.

Ah, yeah this doesn't work when you specify a whitelist 馃槃

And more to the point, the validation against the whitelist is what is the concern. If you have a whitelist in mixed case ( perhaps dynamically generated ) you may want many things:

  • to validate against that whitelist insensitively
  • but convert the result to upper/lowercase

I just went back and read your original issue, and I apologize for not catching the .valid specifically.

I think that's actually a different question altogether on whether or not the values provided to valid should be coerced by the additional schema options. And I actually think probably not at this point.

I think this is actually just a documentation problem.

If the validation convert option is on (enabled by default), a string will be converted using the specified modifiers for string.lowercase(), string.uppercase(), string.trim(), and each replacement specified with string.replace()

and paired with

string.insensitive()
Allows the value to match any whitelist of blacklist item in a case insensitive comparison.

The documentation suggests that lowercase(), uppercase(), trim() and replace() are convert options only, and not validation directives.

Documentation PRs are always welcome ;)

Yeah, basically does removing that whole paragraph make it correct? Because each one of those is separately documented.

I'll gladly send a PR 馃憤

I think so, yes, since there is a note on the convert behavior on the individual methods.

I think this is still a bug at the valid() whitelist level because these two statements behave differently

Joi.string().valid(['c', 'b', 'a']).insensitive().lowercase().validate('A', { convert: false })
// { error: null, value: 'A' }
Joi.string().insensitive().lowercase().validate('A', { convert: false })
// { error: ValidationError: "value" must only contain lowercase characters ...

I'll let others with more experience in managing Joi make the final call, but my first instinct is to say that valid values should not be coerced. I'm honestly a bit surprised that valid(['A']).insensitive() is a thing, haha. To me, valid is a strict equality and adding coerced values ~breaks~ hurts my confidence in cases where I really do want it to be a strict list of valid values.

That being said, I don't have strong feelings about it and could be convinced otherwise :)

Yeah, I agree with you there @WesTyler. That said, I'd definitely be willing to take a look at a PR to address your concerns @rooftopsparrow.

Re-qualifying as a bug. valid should win (as-is), always, and uppercase or lowercase are indeed omitted, it would be different for allow. The missing part here is the "as-is".

Just so I'm 100% clear here (sorry this issue kind of went all over). This behavior is wrong:

Joi.string().valid(['c', 'b', 'a']).insensitive().lowercase().validate('A', { convert: false })
// { error: null, value: 'A' }

and should instead return the validation message from the .valid() chain?

From my perspective yes, it should return 'a' not matter the casing, lowercase being superfluous.

Remember valid is a strict whitelisting, you could also have Joi.string().uppercase().valid(true, false, 42), insensitive only influencing the matching on strings, but otherwise I consider it should act as an enumeration and return the list you provided if a match is found, consistency being your problem.

And to answer about the convert (which I totally missed), good question, I'd say probably not, when convert is false the input shouldn't be transformed.

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.

Was this page helpful?
0 / 5 - 0 ratings