Skip to content

fix(isISO8601): route signed ordinal dates to the day-of-year check - #2873

Open
yfwmaniish wants to merge 1 commit into
validatorjs:masterfrom
yfwmaniish:fix/iso8601-signed-ordinal-date
Open

fix(isISO8601): route signed ordinal dates to the day-of-year check#2873
yfwmaniish wants to merge 1 commit into
validatorjs:masterfrom
yfwmaniish:fix/iso8601-signed-ordinal-date

Conversation

@yfwmaniish

Copy link
Copy Markdown

Fixes #2860.

What

isISO8601(str, { strict: true }) misroutes signed ordinal dates (+YYYY-DDD / -YYYY-DDD).

isValidDate()'s ordinal branch matches:

const ordinalMatch = str.match(/^(\d{4})-?(\d{3})([ T]{1}\.*|$)/);

This has no sign prefix, even though the top-level iso8601 regex explicitly accepts a leading +/- on the year ([\+-]?\d{4}). A signed ordinal date like "+2009-145" fails this ^(\d{4}) match (the string starts with +, not a digit) and falls through to the generic calendar-date split:

const match = str.match(/(\d{4})-?(\d{0,2})-?(\d*)/).map(Number);

This regex has no ^ anchor, so it matches starting one character in, cutting the 3-digit day-of-year into a 2-digit "month" and 1-digit "day". Two things follow:

  • "+2009-145" → month=14, day=5 → new Date('2009-14-05') normalizes the out-of-range month, the equality check fails, and a valid date (day 145 of 2009 exists) is rejected.
  • "+2009-130" → month=13, day=0day is falsy, so if (month && day) is skipped entirely and the function unconditionally returns true — the day-of-year is never actually validated, only coincidentally correct here.

Fix

Add [+-]? to the ordinal pattern, so signed ordinal dates route to the branch that already has the correct leap-year-aware day-of-year check (unaffected — the capture group indices don't change, so the existing oYear/oDay extraction is unchanged).

Testing

Added '+2009-145', '+2009-130', '-2009-145' to the strict = true valid cases in test/validators.test.js.

  • Negative control: reverted just the regex change and reran — +2009-145 fails with exactly the reported symptom (validator.isISO8601("+2009-145", {"strict":true}) failed but should have passed); restored.
  • Full suite: npx mocha --require @babel/register --reporter dot --recursive — 323/323 passing.
  • eslint on both changed files — clean.

I didn't add an invalid case: the only way to reach an "impossible day-of-year" here is day 366 in a non-leap year, and the top-level iso8601 regex's own day-of-year alternative (3([0-5]\d|6[0-6])) already constrains matches to 001-366, so a genuinely out-of-range day like 999 never reaches this code at all regardless of the fix (rejected earlier, by the outer regex). There's no black-box example where this specific bug flips a wrong accept into a rejection — only wrong rejections (+2009-145) and an unvalidated-but-coincidentally-correct pass-through (+2009-130), both covered above.

Copilot AI lite review requested due to automatic review settings September 1, 2026 04:53
@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (a79ff98) to head (4cf7de6).

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #2873   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          114       114           
  Lines         2599      2599           
  Branches       658       658           
=========================================
  Hits          2599      2599           

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

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.

Pull request overview

This PR fixes isISO8601(str, { strict: true }) rejecting signed ordinal dates (+YYYY-DDD / -YYYY-DDD) by ensuring those inputs route through the ordinal (day-of-year) validation path in src/lib/isISO8601.js, instead of falling through to the generic calendar-date parsing.

Changes:

  • Updated the ordinal-date detection regex in isValidDate() to allow an optional leading +/- sign before the year.
  • Added strict-mode regression cases for signed ordinal dates to the validator test suite.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/lib/isISO8601.js Allows optional leading sign in the ordinal-date match so signed ordinal inputs use the correct leap-year-aware day-of-year validation.
test/validators.test.js Adds strict-mode valid test cases covering signed ordinal dates to prevent regressions.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

isValidDate()'s ordinal-date branch matched /^(\d{4})-?(\d{3}).../,
with no sign prefix, even though the top-level iso8601 regex accepts
a leading +/- on the year. A signed ordinal date like "+2009-145"
therefore fell through to the generic calendar-date split
(\d{4})-?(\d{0,2})-?(\d*), which cuts the 3-digit day-of-year into a
2-digit "month" and 1-digit "day" -- misvalidating or, when the split
leaves day as "0" (falsy), skipping validation entirely and returning
true unconditionally.

Add [+-]? to the ordinal pattern so signed ordinal dates route to the
branch that already has the correct leap-year-aware day-of-year check.

Fixes validatorjs#2860
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.

isISO8601 misroutes signed ordinal dates

2 participants