fix(isLatLong): fix bug on isLatLang #1074
Conversation
ezkemboi
commented
Jul 29, 2019
ezkemboi
commented
- when lat starts with (, it makes it a true latitude.
- Closes isLatLong returns true #929
- when lat starts with (, it makes it a true latitude. - Closes validatorjs#929
src/lib/isLatLong.js
Outdated
| if ( | ||
| (pair[0].charAt(0) === '(' && pair[1].charAt(pair[1].length - 1) !== ')') || | ||
| (pair[1].charAt(pair[1].length - 1) === ')' && pair[0].charAt(0) !== '(') | ||
| ) return false; |
There was a problem hiding this comment.
This will fail if str has length of zero:
> const str = ""
> const pair = str.split(',')
> pair
[ '' ]
> pair[0].charAt(0)
''
> pair[1].charAt(pair[1].length - 1)
Thrown:
TypeError: Cannot read property 'charAt' of undefined
There was a problem hiding this comment.
@chriso, will it mean that we have to catch empty validations, for instance when a user passes an empty value for either latitude or longitude?
Or we should look on ways to integrate this validation in the regex path?
Maybe you can help me get some bigger picture...
There was a problem hiding this comment.
Sorry, disregard my point. I see there's an if (!str.includes(',')) return false; guard above which disallows an empty string.
There was a problem hiding this comment.
The regex ^\(.*\) might do the same validation, but I don't know if it will throw an error.
I am open to possibilities/opinions on the same.
lib/isLatLong.js
Outdated
| @@ -16,6 +16,7 @@ function _default(str) { | |||
| (0, _assertString.default)(str); | |||
| if (!str.includes(',')) return false; | |||
| var pair = str.split(','); | |||
| if (pair[0].charAt(0) === '(' && pair[1].charAt(pair[1].length - 1) !== ')' || pair[1].charAt(pair[1].length - 1) === ')' && pair[0].charAt(0) !== '(') return false; | |||
There was a problem hiding this comment.
Can re-format/write this for better readability?
if (pair[0].startsWith('(') && !pair[1].endsWith(')')
|| pair[1].endsWith(')') && !pair[0].startsWith('(') return false;There was a problem hiding this comment.
I will work on that @profnandaa.
Thank you.
ezkemboi
force-pushed
the
fix-lat-long-validation
branch
from
1a5fe9a to
b684474
Compare
