Skip to content

Commit

Permalink
Allow sender id phone URNs
Browse files Browse the repository at this point in the history
  • Loading branch information
@rowanseymour
rowanseymour committed May 9, 2024
1 parent 9d5c82b commit ccd41ba
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 70 deletions.
53 changes: 34 additions & 19 deletions urns/phone.go
View file
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
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
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
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
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

0 comments on commit ccd41ba

Please sign in to comment.