Fix stateful phone-context validation regexes (drop the g flag) - #498
Open
spokodev wants to merge 1 commit into
Open
Fix stateful phone-context validation regexes (drop the g flag)#498spokodev wants to merge 1 commit into
g flag)#498spokodev wants to merge 1 commit into
Conversation
`RFC3966_GLOBAL_NUMBER_DIGITS_PATTERN_` and `RFC3966_DOMAINNAME_PATTERN_`
are module-level regexes created with the `g` flag and used only via
`RegExp#test()` in `isPhoneContextValid`. `test()` on a global regex
advances that regex's `lastIndex`, and since both patterns are anchored
(`^...$`, no `m` flag), the next call starts mid-string, `^` no longer
matches, and it returns `false` before resetting `lastIndex` to 0.
Because the regex objects persist across every parse, this makes
phone-context validation alternate valid/invalid for the same input and
leak state between unrelated numbers:
parsePhoneNumber('tel:12133734253;phone-context=+1') // +12133734253
parsePhoneNumber('tel:12133734253;phone-context=+1') // undefined (!)
The `g` flag serves no purpose for a full-string anchored `test()`, so
remove it from both patterns. Validation is now stateless and returns the
same result on every call.
Owner
|
Hi,
Thanks for the second PR.
I didn’t get to review the first one yet but I did confirm that there is
that bug.
It looks like you found yet another one.
I’ll eventually review your PRs but at the current moment I’m a but busy
wrapping up other libraries — read-excel-file and write-excel-file — so
this one will have to wait too.
…On Thu, 23 Jul 2026 at 15:31, spokodev ***@***.***> wrote:
What
isPhoneContextValid() validates a tel: URI's phone-context parameter with
two module-level regexes:
const RFC3966_GLOBAL_NUMBER_DIGITS_PATTERN_ = new RegExp(RFC3966_GLOBAL_NUMBER_DIGITS_, 'g')const RFC3966_DOMAINNAME_PATTERN_ = new RegExp(RFC3966_DOMAINNAME_, 'g')// ...return RFC3966_GLOBAL_NUMBER_DIGITS_PATTERN_.test(phoneContext) ||
RFC3966_DOMAINNAME_PATTERN_.test(phoneContext)
Both are created with the g flag but used only via RegExp#test(). On a
global regex, test() advances the regex's lastIndex and resumes from it
next time. Since both patterns are anchored (^…$, no m flag), the
following call starts mid-string, ^ can't match, so it returns false and
resets lastIndex to 0.
The regex objects persist across every parse, so phone-context validation *alternates
valid/invalid for the same input*, and leaks state between unrelated
numbers:
parsePhoneNumber('tel:12133734253;phone-context=+1').number // '+12133734253'parsePhoneNumber('tel:12133734253;phone-context=+1') // undefined ← same valid input
parsePhoneNumber('tel:12133734253;phone-context=+1').number // '+12133734253'parsePhoneNumber('tel:2079460958;phone-context=+44') // undefined ← poisoned by the previous call
Numbers without a phone-context are unaffected (they short-circuit before
these regexes), which is why it hides in normal use and surfaces in tel:
URI / batch scenarios.
Fix
The g flag serves no purpose for a full-string anchored test(). Removing
it from both patterns makes validation stateless and deterministic.
Test
Added a case asserting isPhoneContextValid returns the same result across
repeated calls — '+1' exercises the global-number-digits pattern and 'a'
the domainname pattern. It fails on the current code (the second call
returns false) and passes with the fix.
------------------------------
You can view, comment on, or merge this pull request online at:
#498
Commit Summary
- f1fc713
<f1fc713>
Fix stateful phone-context validation regexes (drop the `g` flag)
File Changes
(2 files
<https://github.com/catamphetamine/libphonenumber-js/pull/498/files>)
- *M* source/helpers/extractPhoneContext.js
<https://github.com/catamphetamine/libphonenumber-js/pull/498/files#diff-c64e83e9366851487b1f76816ed9c2b99175693808ec8767ee48279080e01f88>
(4)
- *M* source/helpers/extractPhoneContext.test.js
<https://github.com/catamphetamine/libphonenumber-js/pull/498/files#diff-7f651c45d27886c728e7c321ac76fcefa76d9e4b961a23dc535b837d9ee04fcb>
(13)
Patch Links:
- https://github.com/catamphetamine/libphonenumber-js/pull/498.patch
- https://github.com/catamphetamine/libphonenumber-js/pull/498.diff
—
Reply to this email directly, view it on GitHub
<#498?email_source=notifications&email_token=AADUP3Y76IHODCRRT6NYPLT5GIATPA5CNFSNUABEM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UF42DCMJWGUZDIOJWGSTHEZLBONXW5KTTOVRHGY3SNFRGKZFFMV3GK3TUVRTG633UMVZF6Y3MNFRWW>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADUP347L4A2Q5IULG6OFQL5GIATPAVCNFSNUABEKJSXA33TNF2G64TZHM3TIMZZGIYDSOB3JFZXG5LFHM2DSNJYHAZDGNZRGSQXMAQ>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
isPhoneContextValid()validates atel:URI'sphone-contextparameter with two module-level regexes:Both are created with the
gflag but used only viaRegExp#test(). On a global regex,test()advances the regex'slastIndexand resumes from it next time. Since both patterns are anchored (^…$, nomflag), the following call starts mid-string,^can't match, so it returnsfalseand resetslastIndexto0.The regex objects persist across every parse, so phone-context validation alternates valid/invalid for the same input, and leaks state between unrelated numbers:
Numbers without a
phone-contextare unaffected (they short-circuit before these regexes), which is why it hides in normal use and surfaces intel:URI / batch scenarios.Fix
The
gflag serves no purpose for a full-string anchoredtest(). Removing it from both patterns makes validation stateless and deterministic.Test
Added a case asserting
isPhoneContextValidreturns the same result across repeated calls —'+1'exercises the global-number-digits pattern and'a'the domainname pattern. It fails on the current code (the second call returnsfalse) and passes with the fix.