Yup: Any value that starts with a digit is validated as number

Created on 21 Feb 2018  路  25Comments  路  Source: jquense/yup

The number validator validates any number that starts with a digit, e.g. 1a is accepted. This seems to be a problem of the value to number conversion:

let parsed = parseFloat(value);
if (this.isType(parsed)) return parsed;

return NaN;

From https://www.ecma-international.org/ecma-262/6.0/index.html#sec-parsefloat-string:

parseFloat may interpret only a leading portion of _string_ as a Number value; it ignores any characters that cannot be interpreted as part of the notation of an decimal literal, and no indication is given that any such characters were ignored.

What about changing the code to this:

let parsed = parseFloat(value);
if (parsed == value) return parsed;

return NaN;

Most helpful comment

This bug should be threated with high severity. Such a basic check must not fail in a validation library.

All 25 comments

hey there, I'd be concerned that a change like this will have effects where strings that should be coerced aren't such as ' 100 ' but maybe that is an unreasonable worry. I agree that the current behavior is a bit looser than would be expected :/

To allow leading/trailing whitespace value could be trimmed before if it's a string:

if (typeof value === 'string') value = value.trim();

Facing the same issue.
In my concrete use case, user enters '4,6' (German decimal syntax), which will incorrectly be parsed to just 4. It would definitely be great if there would maybe be a nice way to inject your own casting logic.

My current workaround is:

yup.addMethod(yup.number, 'delocalize', function () {
  return this.transform(function (currentValue, originalValue) {
    return parseFloat(originalValue.replace(',', '.'))
  })
})

yup.number().delocalize()

So, far it seems to be working for me.

Another option could be to add a regex (e.g. /^\d+(?:[.,]\d?)$/) to the library that ensures the user actually entered a really valid number and not only something that can "somehow" be cast to to a number.

Something like yup.number().ensureNumberFormat()

What do you think?

It would definitely be great if there would maybe be a nice way to inject your own casting logic.

it seems like you've hit on the intended api for doing that already. I'm not sure what would be nicer? If you want it to be the default I usually do something like yup.localizedNumber = () => number().delocalize()

I'm also happy to add a regex based transform if someone want's to PR, but it should maintain the style and structure of the existing api, which perfers methods returning new schema, over setting schema wide defaults

Just to add another voice - I was very shocked to see a string (for example: '12') pass the number check. I was also very confused as to why the string '12F' would pass my check of:

var number = yup.number().required().positive().integer();

I honestly thought I had an error in my code and spent a while trying different things.

  • I did not expect a string to be allowed to be a number at all
  • and I was extremely confused on why a number that also has a letter in it would pass!

@paintedbicycle parsing values, mostly from strings, is half of what yup does. if you don't want any parsing logic (ie just validation) though you can pass the strict option.

@jquense Right, ok, understood on my first point then.

The OP's issue of 'Any value that starts with a digit is validated as number' is actually a little deeper.

It also appears that if a .matches() matches the first bit of the string, anything after does not matter.

I created a Zip .matches() and found:

'11221' // valid
'11R21' // invalid
'11221WERWERWERW' //valid

`

const zip = new RegExp("[0-9]{5}(-[0-9]{4})?");
const schema = yup.object().shape({
  zipCode: yup.string().matches(zip)
})

schema.isValid({
  zipCode: "33333sddssss"
}, { strict: true })
  .then(function (valid) {
    console.log(valid); // This returns valid when it should not
  });

`

Matches isn't doing anything special it's checking against your regex, in this case your regex allows letters after the numbers.

Let's try and keep this issue focused on the particular issue of how the number schema uses parseFloat to unexpected ends thanks y'all

Why does it use parseFloat?

What is wrong with the Number(value)

@kongdigital Number has it's own problems unfortunately, like:

  • Number('') === 0
  • Number(null) === 0
  • Number(undefined) === 0

so when in javascript i need to convert a string to a number, i have to go with something like

function strToNum(text) {
    return text ? Number(text) : NaN;
}

@gabor I understand there is no simple solution but this is totally unexpected that a value like 12fdkfjlsdjfadklaj is a valid number. A user would never expect this to be a number.

parseFloat on its own will never pick it up.

Everyone agrees it's unexpected, I don't think there is a lot of value in pointing that out again :) if someone would like send a PR addressing the issue in a perfomantbway I'd happily take that thanks!

@jquense I know I was asking why we don't use Number.

Anyway I looked at the problem it seems pretty straight forward fix. I'll make a PR but essentially

      let parsed = parseFloat(value);
      if (this.isType(parsed) && isFinite(value)) return parsed;
      return NaN;

The only concern is Number.POSITIVE_INFINITY and Number.NEGATIVE_INFINITY. I didn't notice any tests for infinity either so how should we treat it?

I propose we allow infinity as a valid value:

const isInfinite = (value) => Math.abs(value)===Number.POSITIVE_INFINITY
.
.
.
let parsed = parseFloat(value);
 if ((this.isType(parsed) && isFinite(value)) || isInfinite(value)) return parsed;
return NaN;

Before I write any tests any feedback for Number.POSITIVE_INFINITY

@jquense Any opinions on how "0xFF" should be handled? The solution would be something like:

      let coerced = +value;
      if (this.isType(cooerced)) return coerced;

Looks like there is a PR that addresses both of the issues mentioned here. Any reason to hold back on accepting it?

This working for me Yup.string().matches(/^[0-9]*$/, 'Must be a number')

Any plans for a release with this fix soon-ish?

fix: parsing non numeric strings as numbers issue #177
@jquense : When can we expect the next version in npm with this fix?

This bug should be threated with high severity. Such a basic check must not fail in a validation library.

Why was this closed?

ya'll this is fixed

ya'll this is fixed

Unfortunately, it is not in Yup v^0.28.1

If the input value is "1." (without the quotes), it passes next validation Schema:

amount: yup
  .number('A number is enough')
  .integer('A number is enough')
  .required('Amount is required!')

The one that actually works for me in this case, is @ivan-ivory 's solution:

This working for me Yup.string().matches(/^[0-9]*$/, 'Must be a number')

Didn't have any problem using this validation yup.number().integer().required() in a field with input type="number".

For example:
12. is considered valid but the value is submitted as 12 (without the dot) because of the type="number".

Was this page helpful?
0 / 5 - 0 ratings