Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow sender id phone URNs #124

Merged
merged 1 commit into from
May 9, 2024
Merged

Allow sender id phone URNs #124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 34 additions & 19 deletions urns/phone.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@ import (

var nonTelCharsRegex = regexp.MustCompile(`[^0-9A-Za-z]`)
var altShortCodeRegex = regexp.MustCompile(`^[1-9][0-9]{2,5}$`)
var senderIDRegex = regexp.MustCompile(`^[0-9A-Za-z]{3,64}$`)

var ErrNotNumber = errors.New("not a possible number")

// ParsePhone returns a validated phone URN or an error.
func ParsePhone(raw string, country i18n.Country) (URN, error) {
number, err := ParseNumber(raw, country, true)
func ParsePhone(raw string, country i18n.Country, allowShort, allowSenderID bool) (URN, error) {
number, err := ParseNumber(raw, country, allowShort, allowSenderID)
if err != nil {
return "", err
}
Expand All @@ -24,7 +27,7 @@ func ParsePhone(raw string, country i18n.Country) (URN, error) {
}

// 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, allowShort bool) (string, error) {
func ParseNumber(raw string, country i18n.Country, allowShort, allowSenderID bool) (string, error) {
// strip all non-alphanumeric characters.. only preserving an optional leading +
raw = strings.TrimSpace(raw)
hasPlus := strings.HasPrefix(raw, "+")
Expand All @@ -33,34 +36,41 @@ func ParseNumber(raw string, country i18n.Country, allowShort bool) (string, err
raw = "+" + raw
}

number, err := parsePossibleNumber(raw, country, allowShort)
number, err := parsePossibleNumber(raw, country, allowShort, allowSenderID)
if err != nil {
// if we're sufficiently long and don't start with a 0 then add a +
if len(raw) >= 11 && !strings.HasPrefix(raw, "0") {
raw = "+" + raw
}

number, err = parsePossibleNumber(raw, country, allowShort)
if err != nil {
return "", err
}
return "", err
}

return number, nil
}

// tries to extract a valid phone number or shortcode from the given string
func parsePossibleNumber(input string, country i18n.Country, allowShort bool) (string, error) {
func parsePossibleNumber(input string, country i18n.Country, allowShort, allowSenderID bool) (string, error) {
// try parsing as is, only bailing if we have a junk country code
parsed, err := phonenumbers.Parse(input, string(country))
if err != nil {
if country != "" && err == phonenumbers.ErrInvalidCountryCode {
return "", err
}

if phonenumbers.IsPossibleNumberWithReason(parsed) == phonenumbers.IS_POSSIBLE {
return phonenumbers.Format(parsed, phonenumbers.E164), nil
// check to see if we have a possible number
if err == nil {
if phonenumbers.IsPossibleNumberWithReason(parsed) == phonenumbers.IS_POSSIBLE {
return phonenumbers.Format(parsed, phonenumbers.E164), nil
}
}

// if we're sufficiently long and don't start with a 0, try adding a + prefix and re-parsing
if len(input) >= 11 && !strings.HasPrefix(input, "0") {
parsedWithPlus, err := phonenumbers.Parse("+"+input, string(country))
if err == nil {
if phonenumbers.IsPossibleNumberWithReason(parsedWithPlus) == phonenumbers.IS_POSSIBLE {
return phonenumbers.Format(parsedWithPlus, phonenumbers.E164), nil
}
}
}

if allowShort {
// if we allow short codes and we have a country.. check for one
if parsed != nil && country != i18n.NilCountry && allowShort {
if phonenumbers.IsPossibleShortNumberForRegion(parsed, string(country)) {
return phonenumbers.Format(parsed, phonenumbers.NATIONAL), nil
}
Expand All @@ -72,7 +82,12 @@ func parsePossibleNumber(input string, country i18n.Country, allowShort bool) (s
}
}

return "", errors.New("not a possible number")
// carriers send all sorts of junk, so if we're being very lenient...
if allowSenderID && senderIDRegex.MatchString(input) {
return strings.ToLower(input), nil
}

return "", ErrNotNumber
}

// ToLocalPhone converts a phone URN to a local phone number.. without any leading zeros. Kinda weird but used by
Expand Down
109 changes: 63 additions & 46 deletions urns/phone_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,48 +10,52 @@ import (

func TestParsePhone(t *testing.T) {
testCases := []struct {
input string
country i18n.Country
expectedURN urns.URN
expectedErr string
input string
country i18n.Country
allowShort bool
allowSenderID bool
expectedURN urns.URN
expectedErr string
}{
{"+250788123123", "", "tel:+250788123123", ""}, // international number fine without country
{"+250 788 123-123", "", "tel:+250788123123", ""}, // still fine if not E164 formatted
{"250788123123", "", "tel:+250788123123", ""}, // still fine without leading + because it's long enough

{" 0788383383 ", "RW", "tel:+250788383383", ""}, // country code added
{"+250788383383 ", "RW", "tel:+250788383383", ""}, // already has country code and leading +
{"250788383383 ", "RW", "tel:+250788383383", ""}, // already has country code and no leading +
{"+250788383383 ", "KE", "tel:+250788383383", ""}, // already has a different country code
{"(917)992-5253", "US", "tel:+19179925253", ""}, // punctuation removed
{"800-CABBAGE", "US", "tel:+18002222243", ""}, // vanity numbers converted to digits
{"+62877747666", "ID", "tel:+62877747666", ""},
{"812111005611", "ID", "tel:+62812111005611", ""},
{"0877747666", "ID", "tel:+62877747666", ""},
{"07531669965", "GB", "tel:+447531669965", ""},
{"263780821000", "ZW", "tel:+263780821000", ""},
{"254791541111", "US", "tel:+254791541111", ""}, // international but missing + and wrong country

{"1234", "US", "tel:1234", ""},
{"12345", "US", "tel:12345", ""},
{"123", "RW", "tel:123", ""},
{"8080", "EC", "tel:8080", ""},
{"+250788123123", "", true, true, "tel:+250788123123", ""}, // international number fine without country
{"+250 788 123-123", "", true, true, "tel:+250788123123", ""}, // still fine if not E164 formatted
{"250788123123", "", true, true, "tel:+250788123123", ""}, // still fine without leading + because it's long enough
{" 0788383383 ", "RW", true, true, "tel:+250788383383", ""}, // country code added
{"+250788383383 ", "RW", true, true, "tel:+250788383383", ""}, // already has country code and leading +
{"250788383383 ", "RW", true, true, "tel:+250788383383", ""}, // already has country code and no leading +
{"+250788383383 ", "KE", true, true, "tel:+250788383383", ""}, // already has a different country code
{"(917)992-5253", "US", true, true, "tel:+19179925253", ""}, // punctuation removed
{"800-CABBAGE", "US", true, true, "tel:+18002222243", ""}, // vanity numbers converted to digits
{"+62877747666", "ID", true, true, "tel:+62877747666", ""},
{"812111005611", "ID", true, true, "tel:+62812111005611", ""},
{"0877747666", "ID", true, true, "tel:+62877747666", ""},
{"07531669965", "GB", true, true, "tel:+447531669965", ""},
{"263780821000", "ZW", true, true, "tel:+263780821000", ""},
{"254791541111", "US", true, true, "tel:+254791541111", ""}, // international but missing + and wrong country

{"123456", "US", true, false, "tel:123456", ""},
{"12345", "US", true, false, "tel:12345", ""},
{"1234", "US", true, false, "tel:1234", ""},
{"1234", "US", false, false, "", "not a possible number"},
{"1234", "", true, false, "", "not a possible number"}, // can't parse short without country
{"123", "RW", true, false, "tel:123", ""},
{"8080", "EC", true, false, "tel:8080", ""},

{"PRIZES", "RW", true, false, "", "not a possible number"},

// inputs that fail parsing by libphonenumber
{"1", "RW", "", "the phone number supplied is not a number"},
{"mtn", "RW", "", "the phone number supplied is not a number"},
{"1234", "", "", "invalid country code"}, // can't parse short without country
{"1", "RW", true, false, "", "not a possible number"},

// inputa that fail checking for possible number or shortcode
{"99", "EC", "", "not a possible number"},
{"567-1234", "US", "", "not a possible number"}, // only dialable locally
// input that fails checking for possible number or shortcode
{"99", "EC", true, false, "", "not a possible number"},
{"567-1234", "US", true, false, "", "not a possible number"}, // only dialable locally

{"0788383383", "ZZ", "", "invalid country code"}, // invalid country code
{"1234567890123456789012345678901234567890123456789012345678901234567890123456789", "RW", "", "the string supplied is too long to be a phone number"}, // too long
{"0788383383", "ZZ", true, false, "", "invalid country code"}, // invalid country code
{"1234567890123456789012345678901234567890123456789012345678901234567890123456789", "RW", true, false, "", "not a possible number"}, // too long
}

for i, tc := range testCases {
urn, err := urns.ParsePhone(tc.input, tc.country)
urn, err := urns.ParsePhone(tc.input, tc.country, tc.allowShort, tc.allowSenderID)

if tc.expectedErr != "" {
if assert.EqualError(t, err, tc.expectedErr, "%d: expected error for %s, %s", i, tc.input, tc.country) {
Expand All @@ -60,29 +64,42 @@ func TestParsePhone(t *testing.T) {
} else {
if assert.NoError(t, err, "%d: unexpected error for %s, %s", i, tc.input, tc.country) {
assert.Equal(t, tc.expectedURN, urn, "%d: URN mismatch for %s, %s", i, tc.input, tc.country)

// check the returned URN is valid
assert.Nil(t, urn.Validate())
}
}
}
}

func TestParseNumber(t *testing.T) {
testCases := []struct {
input string
country i18n.Country
allowShort bool
expectedNum string
expectedErr string
input string
country i18n.Country
allowShort bool
allowSenderID bool
expectedNum string
expectedErr string
}{
{"+250788123123", "", true, "+250788123123", ""},
{"+250788123123", "", false, "+250788123123", ""},
{"0788123123", "RW", true, "+250788123123", ""},
{"0788123123", "RW", false, "+250788123123", ""},
{"123", "RW", true, "123", ""},
{"123", "RW", false, "", "not a possible number"},
{"+250788123123", "", false, false, "+250788123123", ""},
{"+250788123123", "", true, false, "+250788123123", ""},
{"+250788123123", "", true, true, "+250788123123", ""},

{"0788123123", "RW", false, false, "+250788123123", ""},
{"0788123123", "RW", true, false, "+250788123123", ""},
{"0788123123", "RW", true, true, "+250788123123", ""},

{"123", "RW", false, false, "", "not a possible number"},
{"123", "RW", true, false, "123", ""},
{"123", "RW", true, true, "123", ""},

{"PRIZES", "RW", false, false, "", "not a possible number"},
{"PRIZES", "RW", true, false, "", "not a possible number"},
{"PRIZES", "RW", true, true, "prizes", ""},
}

for i, tc := range testCases {
num, err := urns.ParseNumber(tc.input, tc.country, tc.allowShort)
num, err := urns.ParseNumber(tc.input, tc.country, tc.allowShort, tc.allowSenderID)

if tc.expectedErr != "" {
if assert.EqualError(t, err, tc.expectedErr, "%d: expected error for %s, %s", i, tc.input, tc.country) {
Expand Down
4 changes: 2 additions & 2 deletions urns/schemes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ var emailRegex = regexp.MustCompile(`^[^\s@]+@[^\s@]+$`)
var freshchatRegex = regexp.MustCompile(`^[0-9a-fA-F]{8}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{12}/[0-9a-fA-F]{8}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{12}$`)
var viberRegex = regexp.MustCompile(`^[a-zA-Z0-9_=/+]{1,24}$`)
var lineRegex = regexp.MustCompile(`^[a-zA-Z0-9_]{1,36}$`)
var phoneRegex = regexp.MustCompile(`^\+?\d{1,64}$`)
var phoneRegex = regexp.MustCompile(`^((\+[0-9]{7,15})|([a-z0-9]{1,64}))$`) // E164 or short code or sender ID
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://www.oreilly.com/library/view/regular-expressions-cookbook/9781449327453/ch04s03.html#:~:text=Thanks%20to%20the%20international%20phone,in%20use%20contain%20seven%20digits.

var twitterHandleRegex = regexp.MustCompile(`^[a-zA-Z0-9_]{1,15}$`)
var webchatRegex = regexp.MustCompile(`^[a-zA-Z0-9]{24}(:[^\s@]+@[^\s@]+)?$`)

Expand Down Expand Up @@ -127,7 +127,7 @@ var Phone = &Scheme{
Name: "Phone",
Normalize: func(path string) string {
// try to parse again
norm, err := ParseNumber(path, "", true)
norm, err := ParseNumber(path, "", true, true)
if err != nil {
return path
}
Expand Down
9 changes: 6 additions & 3 deletions urns/urns_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,14 +170,17 @@ func TestValidate(t *testing.T) {
// valid tel numbers
{"tel:+250788383383", ""},
{"tel:+250788383383", ""},
{"tel:+250123", ""},
{"tel:250123", ""},
{"tel:1337", ""},
{"tel:1", ""}, // one digit shortcodes are a thing
{"tel:1", ""},
{"tel:prizes", ""},

// invalid tel numbers
{"tel:", "cannot be empty"}, // need a path
{"tel:07883 83383", "invalid path component"}, // can't have spaces
{"tel:PRIZES", "invalid path component"}, // vanity numbers should be parsed into real digits
{"tel:PRIZES", "invalid path component"}, // we allow letters but we always lowercase
{"tel:+123", "invalid path component"}, // too short to have a +
{"tel:+prizes", "invalid path component"}, // sender ids don't have +

// twitter handles
{"twitter:jimmyjo", ""},
Expand Down
Loading