Skip to content

honour Date object passed as legacy 2nd arg to isAfter / isBefore#2783

Open
HrachShah wants to merge 1 commit into
validatorjs:masterfrom
HrachShah:fix/isAfter-isBefore-ignore-date-arg-when-object-passed-as-comparisonDate
Open

honour Date object passed as legacy 2nd arg to isAfter / isBefore#2783
HrachShah wants to merge 1 commit into
validatorjs:masterfrom
HrachShah:fix/isAfter-isBefore-ignore-date-arg-when-object-passed-as-comparisonDate

Conversation

@HrachShah

Copy link
Copy Markdown

What this PR fixes

isAfter and isBefore silently ignore a Date object that callers pass as
the legacy second argument. The disambiguation between "this is the legacy date
arg" and "this is the new options object" was a typeof options === 'object'
check, which is true for any Date as well, so the new-options branch always
won. isAfter('2010-01-02', new Date('2010-01-01')) returned false even
though 2010-01-02 is plainly after 2010-01-01, because the code then did
options.comparisonDate (which is undefined on a Date), fell back to
Date().toString() for "now", and compared against the current moment.

Repro

import isAfter from 'validator/lib/isAfter';

const pivot = new Date('2010-01-01T00:00:00Z');
isAfter('2010-01-02', pivot); // false on master, should be true
isAfter('2009-12-31', pivot); // true on master, should be false

Fix

Hoist the disambiguation into a small resolveComparisonDate helper that
recognises a Date instance directly, and short-circuit toDate() when the
comparison date is already a Date. The typeof === 'object' branch is also
guarded against null for the same reason the rest of the codebase already
guards null options.

function resolveComparisonDate(options) {
  if (options instanceof Date) {
    return options;
  }
  if (typeof options === 'object' && options !== null) {
    return options.comparisonDate;
  }
  return options;
}

const comparisonDate = resolveComparisonDate(options) || Date().toString();
const comparison = comparisonDate instanceof Date
  ? comparisonDate
  : toDate(comparisonDate);

Tests

The shared test helper in test/testFunctions.js serialises args via
JSON.stringify for error messages, which would strip the Date identity. The
new cases are written as direct assertions against the validator instead.

  • test/validators/isAfter.test.js — new '(legacy syntax with Date object)'
    describe block:
    • isAfter('2010-01-02', new Date('2010-01-01')) is true
    • isAfter('2009-12-31', new Date('2010-01-01')) is false
    • isAfter('2010-01-02', new Date('not a date')) is false
  • test/validators/isBefore.test.js — same three cases for isBefore.

Both new test blocks fail on master (Stashed the source changes and ran the suite: 2 failing) and pass on the fix (320 passing).

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

ESLint passes on the modified files.

…and isBefore

isAfter and isBefore support a legacy two-argument form `isAfter(str, date)` where the
second argument is a date, plus a modern one-argument form that takes an options
object with `comparisonDate`. The legacy/object disambiguation was done with
`typeof options === 'object'`, which is true for any Date object. So when a caller
wrote `isAfter('2010-01-02', new Date('2010-01-01'))` the legacy branch fired, the
code looked up `options.comparisonDate` on the Date (which is `undefined`), fell
back to `Date().toString()` for "now", and silently compared against the current
moment instead of the supplied Date. `isAfter('2010-01-02', new Date('2010-01-01'))`
returned `false` even though 2010-01-02 is after 2010-01-01, and `isBefore` had
the symmetric problem.

Hoist the disambiguation into a small `resolveComparisonDate` helper that
recognises a `Date` instance directly, and short-circuit `toDate()` when the
comparison date is already a `Date`. The `typeof === 'object'` branch is also
guarded against `null` for the same reason the rest of the codebase already
guards null options.

tests/testFunctions.js serialises args via `JSON.stringify` for error messages,
so I exercised the new path with a hand-built assertion rather than through the
shared `test` helper, which round-trips args through JSON and would lose the
Date identity. The new cases fail on master and pass on the fix.

- test/validators/isAfter.test.js: add a '(legacy syntax with Date object)' describe
  block that asserts `isAfter('2010-01-02', new Date('2010-01-01'))` is true,
  `isAfter('2009-12-31', new Date('2010-01-01'))` is false, and that an invalid
  Date falls through to `false`.
- test/validators/isBefore.test.js: mirror the same three cases for isBefore.
@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (e36b279) to head (f331d12).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #2783   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          114       114           
  Lines         2586      2599   +13     
  Branches       656       660    +4     
=========================================
+ Hits          2586      2599   +13     

☔ 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.

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.

1 participant