Make phone parsing stricter #122
Conversation
| } | ||
|
|
||
| // ParseNumber tries to extact a possible number or shortcode from the given string, returning an error if it can't. | ||
| func ParseNumber(raw string, country i18n.Country) (string, error) { |
There was a problem hiding this comment.
making this exported because it might be useful elsewhere like goflow
urns/phone_test.go
Outdated
| for i, tc := range testCases { | ||
| urn, err := ParsePhone(tc.input, tc.country) | ||
| {"1", "RW", "", "the phone number supplied is not a number"}, | ||
| {"1234", "RW", "", "not a possible number or shortcode"}, // RW short codes are 3 digits |
There was a problem hiding this comment.
wrong, there are short code with 4 digits
There was a problem hiding this comment.
are those dialable numbers ?
There was a problem hiding this comment.
I notice here in Ecuador I get messages from numbers like 8080 which libphonenumber doesn't recognize but I thought maybe they're only one way
There was a problem hiding this comment.
@norkans7 updated to accept short codes that libphonenumber recognizes OR anything that matches ^[1-9][0-9]{2,4}$, e.g. 3-5 digits, not starting with a zero
There was a problem hiding this comment.
There was a problem hiding this comment.
I'm assuming that libphonenumber mostly recognizes valid short codes so this is just a fallback.. and I'm trying to be careful to not start accepting local numbers as valid.. but we can make it 6 I guess.... updating...
There was a problem hiding this comment.
I'd like to move all phone number validation into go eventually so that logic is consistent and in one place.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #122 +/- ##
==========================================
+ Coverage 89.54% 89.57% +0.03%
==========================================
Files 43 43
Lines 1979 1986 +7
==========================================
+ Hits 1772 1779 +7
+ Misses 168 167 -1
- Partials 39 40 +1 ☔ View full report in Codecov by Sentry. |
rowanseymour
force-pushed
the
stricter_phone
branch
from
6f0dcb6 to
c9bf887
Compare
We currently allow phone URNs to be any garbage that is up to 64 chars of alphanumeric characters... and then in courier we have to make it stricter in
StrictTelForCountryhttps://github.com/nyaruka/courier/blob/29ce7cd76c13081efb5e61be45f32770c7a2722a/handlers/utils.go#L99I don't see why we can't be stricter here..