Passing an array containing an object with .toString set to a string will break all methods of validator.js

[kamagatos]

The following will break :

var myObj = [{ toString: 'randomstring' }]
validator.isURL(myObj)

The reason is because calling .toString() here will natively call toString on every index of the array and since in our case, toString isn't a function, it will simply throw an exception.

[chriso]

Have you tried the latest version (4.6.1)? The if (input.toString) { input = input.toString() } was changed to if (typeof input.toString === 'function') { input = input.toString() }.

Your example myObj will now be converted to [object Object] as per the coercion rules.

[kamagatos]

Yes, I'm using the latest version. typeof input.toString === 'function' will return true since myObj is an Array, but then when the function is actually called, it will throw a TypeError because myObj[0].toString is a string and not a function.

[chriso]

Ah I see that it's the native Array.toString() that calls toString() on each of the elements:

> validator.toString([{toString: 'foo'}])
TypeError: Cannot convert object to primitive value
    at Array.toString (native)
    at Object.validator.toString (/Users/chris/Documents/personal/validator.js/validator.js:132:31)

Note the same thing happens when the object is deeply nested:

> validator.toString([[[[[[[[[{toString: 'foo'}]]]]]]]]])
TypeError: Cannot convert object to primitive value
    at Array.toString (native)
    at Array.toString (native)
    at Array.toString (native)
    at Array.toString (native)
    at Array.toString (native)
    at Array.toString (native)
    at Array.toString (native)
    at Array.toString (native)
    at Array.toString (native)
    at Object.validator.toString (/Users/chris/Documents/personal/validator.js/validator.js:132:31)

What do you expect to happen here? The only way to avoid an error like this is to do a deep scan of all array elements. The library is for validating strings so really this is outside the scope of the library.

The alternative is just to drop calling toString() entirely and replace any object or array with [object Object]:

diff --git i/validator.js w/validator.js
index 52ba176..7cc1a95 100644
--- i/validator.js
+++ w/validator.js
@@ -127,13 +127,7 @@
     };

     validator.toString = function (input) {
-        if (typeof input === 'object' && input !== null) {
-            if (typeof input.toString === 'function') {
-                input = input.toString();
-            } else {
-                input = '[object Object]';
-            }
-        } else if (input === null || typeof input === 'undefined' || (isNaN(input) && !input.length)) {
+        if (input === null || typeof input === 'undefined' || (isNaN(input) && !input.length)) {
             input = '';
         }
         return '' + input;

This is a reasonably large breaking change though.

[chriso]

Just a note that passing an object containing toString() set to a string works:

> validator.toString({toString: 'foo'})
'[object Object]'

The object has to be nested in an array for the error to occur:

> validator.toString([{toString: 'foo'}])
TypeError: Cannot convert object to primitive value
    at Array.toString (native)
    at Object.validator.toString (/Users/chris/Documents/personal/validator.js/validator.js:132:31)
    at repl:1:11
    at REPLServer.defaultEval (repl.js:252:27)
    at bound (domain.js:287:14)
    at REPLServer.runBound [as eval] (domain.js:300:12)
    at REPLServer.<anonymous> (repl.js:417:12)
    at emitOne (events.js:95:20)
    at REPLServer.emit (events.js:182:7)
    at REPLServer.Interface._onLine (readline.js:211:10)

Also note that validator.toString() is called on the first argument of each validator.

[chriso]

Yeah I'm inclined to say that guarding against this is outside the scope of the library. Messing with toString() causes the language built-ins (Array.toString()) to break - this is not a problem with this library but rather a problem with your input.

The following also breaks the library:

validator.isEmail({toString: function () { throw new Error(); }});

We obviously shouldn't have to guard against that or suppress the error by wrapping the toString() call in a try..catch. This is a string validation library.

If you want to check untrusted input then you could coerce it yourself:

try {
  // coerce to a string
  untrusted_input += '';
} catch (err) {
  // input is invalid
}

[kamagatos]

Indeed dropping toString for objects altogether is a large breaking change and not necessarily a good one.
Try/catch-ing my input is my current fallback.
But what you mentioned earlier raises another issue.

The following code will always return true, which is technically not accurate.

validator.isURL(['http://google.com'])

Don't know if it's intentional or not, but again, thanks for this great library!

[chriso]

Agreed that it's not ideal. I've tried to make the fact that the library validates strings only very clear.

In v5 I might look at removing string coercion altogether, since it seems to be a major source of confusion/bugs:

if (typeof input !== 'string') {
  throw new Error('this library validates strings only');
}

That'd be a bit of a breaking change 😉

[kamagatos]

Good Idea! A deprecation notice would be a great way to start.

[pesho]

I see no logic in printing the deprecation warning when calling validator.toString(...). I call this method exactly in order to convert the input to a string. Isn't this its intended usage? If not, the documentation should be updated.

[chriso]

@pesho toString() will be removed soon. I'll remove it from the README.