Skip to content

fix(isFloat): reject sign-only + decimal-separator combinations#2786

Open
JSap0914 wants to merge 1 commit into
validatorjs:masterfrom
JSap0914:fix/2090-isfloat-sign-decimal-combo
Open

fix(isFloat): reject sign-only + decimal-separator combinations#2786
JSap0914 wants to merge 1 commit into
validatorjs:masterfrom
JSap0914:fix/2090-isfloat-sign-decimal-combo

Conversation

@JSap0914

Copy link
Copy Markdown
Contributor

Problem

isFloat('+.') and isFloat('-.') return true but should return false. The same false positive appears with comma-decimal locales (+,/-, for de-DE) and with the Arabic decimal separator (/ for ar-JO).

Closes #2090

Root cause

The early-return blacklist in src/lib/isFloat.js rejects each problematic token individually:

if (str === '' || str === '.' || str === ',' || str === '-' || str === '+') {
  return false;
}

But the combined form — a sign immediately followed by the decimal separator and nothing else — slips through because it is not in the list. The float regex then matches it (the digit groups are all optional) and returns true, while parseFloat('+.') returns NaN, so the result is semantically wrong.

Fix

Extend the early-return to also reject the two combined forms (+<sep> and -<sep>) for the dot and comma separators, and derive the locale's actual separator (e.g. ٫ for Arabic locales) to cover non-ASCII decimal characters too:

+ const decimalSep = options.locale ? decimal[options.locale] : '.';
- if (str === '' || str === '.' || str === ',' || str === '-' || str === '+') {
+ if (str === '' || str === '.' || str === ',' || str === '-' || str === '+'
+   || str === '+.' || str === '-.' || str === '+,' || str === '-,'
+   || str === `+${decimalSep}` || str === `-${decimalSep}`) {

Verification

$ npx mocha --reporter dot --require @babel/register --recursive test
  318 passing (596ms)

Six new invalid fixtures were added (two per locale: default ., de-DE comma, ar-JO Arabic separator). All previously-valid inputs (+.123, -.5, +,123 for de-DE, etc.) remain valid. No existing tests were modified.

This fix was developed with AI assistance.

isFloat('+.') and isFloat('-.') returned true because the early-return
blacklist only checked for the sign and separator individually, missing
the combined form.  Same false positive occurs for the comma-decimal
locales ('+,'/'-,' with e.g. de-DE) and for the Arabic decimal
separator ('+٫'/'-٫' with ar-JO).

Extend the guard to also reject '+<sep>' and '-<sep>' for the dot and
comma separators (existing behavior for the characters in isolation is
kept) and derive the locale's separator to cover non-latin separators.

Closes validatorjs#2090
Copilot AI review requested due to automatic review settings June 27, 2026 09:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@codecov

codecov Bot commented Jun 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (3d2f4b3) to head (d646e37).

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #2786   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          114       114           
  Lines         2587      2588    +1     
  Branches       656       657    +1     
=========================================
+ Hits          2587      2588    +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread src/lib/isFloat.js
Comment on lines +10 to +12
if (str === '' || str === '.' || str === ',' || str === '-' || str === '+'
|| str === '+.' || str === '-.' || str === '+,' || str === '-,'
|| str === `+${decimalSep}` || str === `-${decimalSep}`) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not a regular expression as suggested in the issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

isFloat() returns true when a combination of sign and decimal separator is passed

3 participants