Bug report: Poor empty value handling for default format validator?

[mondwan]

Description

Given I need to validate value is not empty, for example

• '': Invalid
• ' ': Valid

  Problems

Option presence explicitly states whitespaces only is not a valid string. So I head to format.

However, default format validator always allows empty value (empty defined above) at the beginner which ignores my regex object.

Reproduce the problem

Here is the fiddle to demonstrate the problem: https://jsfiddle.net/8okbtbuz/

Given I have a regex ^.{1,64}$' which ensures there is at least one character, validator.single() always treat the empty value is a valid one.

Problematic codes spotted

https://github.com/ansman/validate.js/blob/master/validate.js#L948

It always return success for empty value. This is not good.....

[ansman]

This is by design, you have to use both presence and format to achieve what you want.

[mondwan]

Nope. Both of them does not work for me

By code,

https://github.com/ansman/validate.js/blob/master/validate.js#L759

For option presence, It always return false for empty string. (a string contains spaces only)

Code example,

var a = ['', ' ', '  '];

a.forEach(function (v) {
    console.log('value |%s|', v);

  // Presence on
    console.log('presesence on' , validate.single(v, {
    presence: true,
    format: {
        pattern: "^.{1,64}$",
      message: 'haha',
    },
  }));

  // presence off
    console.log('presence off', validate.single(v, {
    presence: false,
    format: {
        pattern: "^.{1,64}$",
      message: 'haha',
    },
  }));
 });

Output

value ||
presesence on ["can't be blank"]
presence off undefined
value | |
presesence on ["can't be blank"]
presence off undefined
value |  |
presesence on ["can't be blank"]
presence off undefined

Fiddler

https://jsfiddle.net/8okbtbuz/1/

Expected result

'' -> invalid, empty string
' ' -> valid, string with at least one space

[ansman]

Ah, I see your dilemma. I'll draft a solution.

[ansman]

@mondwan, @jpbufe3, @sckogi I've drafted a solution in ecde1177f02b79a4a3b0eb28432703426edac82d, how does it look? I see two main use cases, one is user inputted data such a form on a web page. It's easy to put an extra space somewhere which doesn't really qualify as having entered something.

The other is a server which should have strict parsing in which a whitespace string is very much defined.

[mondwan]

Hello @ansman , glad to see this patch. In my opinions, changes on format suits my need and the introduction of disallowEmpty on presence is quite good.

However, I have not checked out the changes on others as there are no real usages on my side. I cannot judge whether they are good or not

2 suggestions at last:

• Comments like here should be removed or updated. Otherwise, it cause confusion.
• disallowEmpty should be allowEmpty? This provides better backward compatibility?

Thanks for your patch :D

[sckogi]

Hi @ansman, I agree with @mondwan about allowEmpty. And it seems in spec is missing some cases like datetime([], {}) and so.

Thanks ;D !

[ansman]

@sckogi Datetime was changed here: https://github.com/ansman/validate.js/commit/75ec791d1e55f87d97588d87d034aa184e23445a#diff-ed60d8ad61ec5449dd72bc2b36d651e1L47 https://github.com/ansman/validate.js/commit/75ec791d1e55f87d97588d87d034aa184e23445a#diff-de099361a37e0c86a01873666f829014L918

I've made the changes here: 75ec791d1e55f87d97588d87d034aa184e23445a

[mondwan]

Hello @ansman , may I know the date for releasing this patch?

[ansman]

Released in 0.11.0

[mondwan]

Hello @ansman , after using the 0.11.0 in my production environment, I believe value '' should be included in isDefined().

In line 275

# It should rewrite this
return obj !== null && obj !== undefined;
# to this
return obj !== null && obj !== undefined && obj !== '';

Here is a JSfiddler to illustrate the reasons:

Goal of my program

• Allow string if value is ''
• Allow strings for example ' ', ' ' if they obey regex ^.{1,5}$

Current result

# Case1
value ||
allowEmpty true ["Invalid from format"]
allowEmpty false ["Invalid from presence", "Invalid from format"]

# Case2
value | |
allowEmpty true undefined
allowEmpty false ["Invalid from presence"]

# Case3
value |  |
allowEmpty true undefined
allowEmpty false ["Invalid from presence"]

# Case 4
value |012345|
allowEmpty true ["Invalid from format"]
allowEmpty false ["Invalid from format"]

Summary

Version before 0.11.1 failure for #Case 1, 2, 3 and version 0.11.1 failures only on #Case 1.

In version 0.11.1, rule format will always be executed even string is "not defined" which is ''. Therefore, #Case 1 is invalid. For #Case2, 3, they are valid as values "are defined".

Below is the expected result I hope to get.

Expected result

# Case1
value ||
allowEmpty true undefined
allowEmpty false ["Invalid from presence", "Invalid from format"]

# Case2
value | |
allowEmpty true undefined
allowEmpty false ["Invalid from presence"]

# Case3
value |  |
allowEmpty true undefined
allowEmpty false ["Invalid from presence"]

# Case 4
value |012345|
allowEmpty true ["Invalid from format"]
allowEmpty false ["Invalid from format"]

[alolis]

I am trying to figure out if this should now return error or not:

validate.single("", {presence: false, { length : { minimum : 5 } }});

I am expecting to NOT return error since presence: false but I am getting a length error. Is there something I am missing here? I am using 0.11.1

[mondwan]

Before 0.11.1, it will not raise error from length as value '' is treated as empty. No further validations will be made if value is empty

At 0.11.1, value '' is treated as defined, validations will always be made it value is defined.

That's explain the result you got.

So, I suggest to define '' to be a kind of not defined value which same as null and undefined to fix the problem

[alolis]

@mondwan , thanks for the answers for starters. Redefining isDefined and include "" as an undefined value, might cause other issues though in different cases where it does make sense.

My case for the validator is a form which has optional fields but if the user does enter a value, it should be validated (specifically it's an optional "New Password" field)

I guess i need to re-define isUndefined or just use a regex validator. The presence: {value: false, allowEmpty: true} would have been a nice solution to this, without messing around with anything else.

[mondwan]

Yes. I know. It is just my suggestion.

Actually, I need the similar mechanism you mentioned.

I haven't studied usage of isUndefined. However, if you are trying to solve this by regex, I guess regex like this ^.{0,5}$ should play well on existing mechanism.

Regex is still possible to achieve what I need, but I don't know how to do if using rules like numericality with avoiding empty value as discussed.

[alolis]

However, if you are trying to solve this by regex, I guess regex like this ^.{0,5}$ should play well on existing mechanism.

That's what I did, but I really think it should be fixed via the presence validator.

[obsidianart]

agree with alolis (I have a similar scenario) it would be nice to specify value:false. For the time being I'll downgrade validatejs. I didn't expect a breaking change in a minor release either...

[mondwan]

Hello @ansman , any comment for this issue?

[airtonix]

easiest way to handle this is by using the regexp or operator:

before:

        format: {
            pattern: '[0-9]{6,10}',
            flags: 'ig',
            message: '^Valid Account numbers are between 6 and 10 digits'
        },

after

        format: {
            pattern: '^([\s]+)?$|[0-9]{6,10}',
            flags: 'ig',
            message: '^Leave blank or provide a valid account number containing between 6 and 10 digits'
        },