utils - fix: reject malformed numbers in shorthandToMilliseconds

[rudycelekli]

Please check if the PR fulfills these requirements

• Followed the Contributing guidelines and Code of Conduct
• Tests for the changes have been added (for bug fixes/features) with 100% code coverage.

What kind of change does this PR introduce? Bug fix (@cacheable/utils)

shorthandToMilliseconds now rejects malformed numbers with valid time units. The old regex accepts multiple decimal points or returns NaN for a number without digits.

Input	Before this PR	After this PR
"1.2.3s"	1200	Throws an error
"1..5s"	1000	Throws an error
"..s", ".s", ".m"	NaN	Throws an error
".5s", "5.s"	500, 5000	500, 5000

The decimal part stays in one optional group to prevent polynomial backtracking:

const match = /^(\d+(?:\.\d*)?|\.\d+)\s*(ms|s|m|h|hr|d)$/i.exec(shorthand);

Verification

• pnpm build passed.
• node scripts/test-build.mjs passed for all packages.
• pnpm test passed with 1,390 tests and coverage. Redis and the HTTP service supplied the test services.
• All 254 tests in @cacheable/utils passed. The changed shorthand-time.ts file has 100% statement, branch, function, and line coverage.
• The workspace has 100% line coverage. Some unchanged files have branch or function coverage below 100%, so the full-coverage box stays clear.
• All five malformed-input cases failed against main at 3a37f54 with expected [Function] to throw an error.
• Both regexes produced the same matches and captures for all 597,870 nonempty strings of length six or less over [0 1 . s m h r d space].
• A local check used 40,000 digits followed by x. The previous PR regex took 15,088 ms; the new regex took 0.509 ms.
• Test audit: the malformed-input table protects the error contract for numbers with multiple dots or no digits. The old regex accepts all five inputs. Existing tests only reject an unsupported unit. These cases call the exported function without a test hook.
• Test audit: the decimal table protects .5s and 5.s. A stricter regex can reject these valid values. Existing decimal tests use digits on both sides of the dot. These cases call the exported function without a test hook.
• Test rewrite: send in string with malformed number and valid unit throws became reject a malformed number in "%s". The table retains all five inputs and identifies each case in a failure.
• Test rewrite: send in string with decimal edge cases became convert "%s" to milliseconds. The table retains both original assertions and identifies each case in a failure.

The original contributor used Claude Code. Codex prepared the follow-up change and ran the checks above.

[jaredwray]

Thank you for this fix. The new regex accepts and rejects the correct inputs. But it adds a performance problem.

Problem: polynomial backtracking (ReDoS) in the new regex

In \d+\.?\d*, the dot is optional. So \d+ and \d* can divide one run of digits in many ways. When the end of the string does not match, the engine tries each division. The time increases with the square of the input length. The old [\d.]+ did not have this problem.

Input	main	This PR	Suggested regex
40,000 digits + "x"	0.5 ms	2,092 ms	0.5 ms

Cacheable.set, @cacheable/memory, and @cacheable/node-cache send string TTLs to this function without validation. A long TTL string from user input can block the event loop.

Requested change

Put the decimal part in one optional group at packages/utils/src/shorthand-time.ts:33:

const match = /^(\d+(?:\.\d*)?|\.\d+)\s*(ms|s|m|h|hr|d)$/i.exec(shorthand);

This regex gives the same captures as the regex in this PR. A comparison on all 597,870 strings of length 6 or less over [0 1 . s m h r d space] found 0 differences.

Optional

• Use test.each for the malformed inputs. Then a failure shows which input failed.
• Add the four test-audit answers from AGENTS.md to the verification list in the PR body.

[codecov]

Codecov Report

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

Additional details and impacted files

@@            Coverage Diff            @@
##              main     #1744   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           29        29           
  Lines         3620      3620           
  Branches       824       812   -12     
=========================================
  Hits          3620      3620           

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

[jaredwray]

Commit 4a6e545 applies the requested regex change. The decimal part stays in one optional group to prevent polynomial backtracking.

Both test groups now use test.each. The PR description includes the four test-audit answers for each group and the reasons for both test rewrites.

pnpm build, node scripts/test-build.mjs, and pnpm test pass. All 1,390 tests pass. The changed parser has 100% statement, branch, function, and line coverage.

[jaredwray]

@rudycelekli - this is going to get merged today and I did the fix I asked for. Should be released in the next couple weeks. Thanks!

I did the fix