From ccd41ba93489072eebeb05c42d2aa107668e6eca Mon Sep 17 00:00:00 2001 From: Rowan Seymour Date: Thu, 9 May 2024 15:17:14 -0500 Subject: [PATCH] Allow sender id phone URNs --- urns/phone.go | 53 ++++++++++++++-------- urns/phone_test.go | 109 ++++++++++++++++++++++++++------------------- urns/schemes.go | 4 +- urns/urns_test.go | 9 ++-- 4 files changed, 105 insertions(+), 70 deletions(-) diff --git a/urns/phone.go b/urns/phone.go index 703b8e6..51a6dc0 100644 --- a/urns/phone.go +++ b/urns/phone.go @@ -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 } @@ -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, "+") @@ -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 } @@ -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 diff --git a/urns/phone_test.go b/urns/phone_test.go index 9bff19e..2062031 100644 --- a/urns/phone_test.go +++ b/urns/phone_test.go @@ -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) { @@ -60,6 +64,9 @@ 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()) } } } @@ -67,22 +74,32 @@ func TestParsePhone(t *testing.T) { 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) { diff --git a/urns/schemes.go b/urns/schemes.go index 4189f83..379e431 100644 --- a/urns/schemes.go +++ b/urns/schemes.go @@ -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@]+)?$`) @@ -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 } diff --git a/urns/urns_test.go b/urns/urns_test.go index b1a8df1..80fe493 100644 --- a/urns/urns_test.go +++ b/urns/urns_test.go @@ -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", ""},