diff --git a/go.mod b/go.mod index 1223db4..decccd0 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/nyaruka/gocommon go 1.21 require ( - github.com/aws/aws-sdk-go v1.52.3 + github.com/aws/aws-sdk-go v1.52.4 github.com/gabriel-vasile/mimetype v1.4.3 github.com/go-chi/chi/v5 v5.0.12 github.com/go-playground/validator/v10 v10.20.0 @@ -14,7 +14,7 @@ require ( github.com/lib/pq v1.10.9 github.com/nyaruka/librato v1.1.1 github.com/nyaruka/null/v2 v2.0.3 - github.com/nyaruka/phonenumbers v1.3.4 + github.com/nyaruka/phonenumbers v1.3.5 github.com/pkg/errors v0.9.1 github.com/shopspring/decimal v1.4.0 github.com/stretchr/testify v1.9.0 diff --git a/go.sum b/go.sum index 61cc752..5830e79 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,7 @@ filippo.io/edwards25519 v1.1.0 h1:FNf4tywRC1HmFuKW5xopWpigGjJKiJSV0Cqo0cJWDaA= filippo.io/edwards25519 v1.1.0/go.mod h1:BxyFTGdWcka3PhytdK4V28tE5sGfRvvvRV7EaN4VDT4= -github.com/aws/aws-sdk-go v1.52.3 h1:BNPJmHOXNoM/iBWJKrvaQvJOweRcp3KLpzdb65CfQwU= -github.com/aws/aws-sdk-go v1.52.3/go.mod h1:LF8svs817+Nz+DmiMQKTO3ubZ/6IaTpq3TjupRn3Eqk= +github.com/aws/aws-sdk-go v1.52.4 h1:9VsBVJ2TKf8xPP3+yIPGSYcEBIEymXsJzQoFgQuyvA0= +github.com/aws/aws-sdk-go v1.52.4/go.mod h1:LF8svs817+Nz+DmiMQKTO3ubZ/6IaTpq3TjupRn3Eqk= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -43,8 +43,8 @@ github.com/nyaruka/librato v1.1.1 h1:0nTYtJLl3Sn7lX3CuHsLf+nXy1k/tGV0OjVxLy3Et4s github.com/nyaruka/librato v1.1.1/go.mod h1:fme1Fu1PT2qvkaBZyw8WW+SrnFe2qeeCWpvqmAaKAKE= github.com/nyaruka/null/v2 v2.0.3 h1:rdmMRQyVzrOF3Jff/gpU/7BDR9mQX0lcLl4yImsA3kw= github.com/nyaruka/null/v2 v2.0.3/go.mod h1:OCVeCkCXwrg5/qE6RU0c1oUVZBy+ZDrT+xYg1XSaIWA= -github.com/nyaruka/phonenumbers v1.3.4 h1:bF1Wdh++fxw09s3surhVeBhXEcUKG07pHeP8HQXqjn8= -github.com/nyaruka/phonenumbers v1.3.4/go.mod h1:Ut+eFwikULbmCenH6InMKL9csUNLyxHuBLyfkpum11s= +github.com/nyaruka/phonenumbers v1.3.5 h1:WZLbQn61j2E1OFnvpUTYbK/6hViUgl6tppJ55/E2iQM= +github.com/nyaruka/phonenumbers v1.3.5/go.mod h1:Ut+eFwikULbmCenH6InMKL9csUNLyxHuBLyfkpum11s= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= @@ -68,8 +68,6 @@ golang.org/x/sys v0.20.0 h1:Od9JTbYCk261bKm4M/mw7AklTlFYIa0bIp9BgSm1S8Y= golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/text v0.15.0 h1:h1V/4gjBv8v9cjcR6+AR5+/cIYK5N/WAgiv4xlsEtAk= golang.org/x/text v0.15.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= -google.golang.org/protobuf v1.33.0 h1:uNO2rsAINq/JlFpSdYEKIZ0uKD/R9cpdv0T+yoGwGmI= -google.golang.org/protobuf v1.33.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= google.golang.org/protobuf v1.34.1 h1:9ddQBjfCyZPOHPUiPxpYESBLc+T8P3E+Vo4IbKZgFWg= google.golang.org/protobuf v1.34.1/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= diff --git a/urns/phone.go b/urns/phone.go index 9358b4c..8ebd7e8 100644 --- a/urns/phone.go +++ b/urns/phone.go @@ -11,11 +11,21 @@ import ( ) var nonTelCharsRegex = regexp.MustCompile(`[^0-9A-Za-z]`) +var altShortCodeRegex = regexp.MustCompile(`^[1-9][0-9]{2,5}$`) -// ParsePhone returns a validated phone URN. If it can parse a possible number then that is used.. otherwise any value -// that validates as a phone URN is used. +// ParsePhone returns a validated phone URN or an error. func ParsePhone(raw string, country i18n.Country) (URN, error) { - // strip all non-tel characters.. only preserving an optional leading + + number, err := ParseNumber(raw, country) + if err != nil { + return "", err + } + + return NewFromParts(Phone.Prefix, number, "", "") +} + +// 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) { + // strip all non-alphanumeric characters.. only preserving an optional leading + raw = strings.TrimSpace(raw) hasPlus := strings.HasPrefix(raw, "+") raw = nonTelCharsRegex.ReplaceAllString(raw, "") @@ -23,26 +33,22 @@ func ParsePhone(raw string, country i18n.Country) (URN, error) { raw = "+" + raw } - // if we're sufficienly long and don't start with a 0 then add a + + // 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 := parsePhoneOrShortcode(raw, country) if err != nil { - if err == phonenumbers.ErrInvalidCountryCode { - return NilURN, errors.New("invalid country code") - } - - return NewFromParts(Phone.Prefix, raw, "", "") + return "", err } - return NewFromParts(Phone.Prefix, number, "", "") + return number, nil } // tries to extract a valid phone number or shortcode from the given string -func parsePhoneOrShortcode(raw string, country i18n.Country) (string, error) { - parsed, err := phonenumbers.Parse(raw, string(country)) +func parsePhoneOrShortcode(input string, country i18n.Country) (string, error) { + parsed, err := phonenumbers.Parse(input, string(country)) if err != nil { return "", err } @@ -55,7 +61,13 @@ func parsePhoneOrShortcode(raw string, country i18n.Country) (string, error) { return phonenumbers.Format(parsed, phonenumbers.NATIONAL), nil } - return "", errors.New("unable to parse phone number or shortcode") + // it seems libphonenumber's metadata regarding shortcodes is lacking so we also accept any sequence of 3-6 digits + // that doesn't start with a zero as a shortcode + if altShortCodeRegex.MatchString(input) { + return input, nil + } + + return "", errors.New("not a possible number or shortcode") } // 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 0bae043..e9edd6e 100644 --- a/urns/phone_test.go +++ b/urns/phone_test.go @@ -1,83 +1,81 @@ -package urns +package urns_test import ( "testing" "github.com/nyaruka/gocommon/i18n" + "github.com/nyaruka/gocommon/urns" "github.com/stretchr/testify/assert" ) func TestParsePhone(t *testing.T) { testCases := []struct { - input string - country i18n.Country - expected URN + input string + country i18n.Country + expectedURN urns.URN + expectedErr string }{ - {" 0788383383 ", "RW", "tel:+250788383383"}, - {"+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"}, - {"800-CABBAGE", "US", "tel:+18002222243"}, - {"+62877747666", "ID", "tel:+62877747666"}, - {"0877747666", "ID", "tel:+62877747666"}, - {"07531669965", "GB", "tel:+447531669965"}, - {"263780821000", "ZW", "tel:+263780821000"}, - {"254791541111", "US", "tel:+254791541111"}, // international but missing + and wrong country - - {"1", "RW", "tel:1"}, - {"123456", "RW", "tel:123456"}, - {"mtn", "RW", "tel:mtn"}, - {"!mtn!", "RW", "tel:mtn"}, // non tel chars stripped + {"+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", "ZZ", NilURN}, // invalid country code - {"1234567890123456789012345678901234567890123456789012345678901234567890123456789", "RW", NilURN}, // too long - } - - for i, tc := range testCases { - urn, err := ParsePhone(tc.input, tc.country) + {" 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", ""}, + {"0877747666", "ID", "tel:+62877747666", ""}, + {"07531669965", "GB", "tel:+447531669965", ""}, + {"263780821000", "ZW", "tel:+263780821000", ""}, + {"254791541111", "US", "tel:+254791541111", ""}, // international but missing + and wrong country - if tc.expected == NilURN { - assert.Error(t, err, "%d: expected error for %s, %s", i, tc.input, tc.country) - } else { - assert.NoError(t, err, "%d: unexpected error for %s, %s", i, tc.input, tc.country) - assert.Equal(t, tc.expected, urn, "%d: created URN mismatch for %s, %s", i, tc.input, tc.country) - } - } -} + {"1234", "US", "tel:1234", ""}, + {"12345", "US", "tel:12345", ""}, + {"123", "RW", "tel:123", ""}, + {"8080", "EC", "tel:8080", ""}, -func TestParsePhoneOrShortcode(t *testing.T) { - tcs := []struct { - input string - country i18n.Country - expected string - }{ - {"+250788123123", "", "+250788123123"}, // international number fine without country - {"+250 788 123-123", "", "+250788123123"}, // still fine if not E164 formatted + // 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"}, - {"0788123123", "RW", "+250788123123"}, // country code added - {" (206)555-1212 ", "US", "+12065551212"}, // punctiation removed - {"800-CABBAGE", "US", "+18002222243"}, // letters converted to numbers - {"12065551212", "US", "+12065551212"}, // country code but no + - {"10000", "US", "10000"}, // valid short code for US + // inputa that fail checking for possible number or shortcode + {"99", "EC", "", "not a possible number or shortcode"}, + {"567-1234", "US", "", "not a possible number or shortcode"}, // only dialable locally - {"5912705", "US", ""}, // is only possible as a local number so ignored + {"0788383383", "ZZ", "", "invalid country code"}, // invalid country code + {"1234567890123456789012345678901234567890123456789012345678901234567890123456789", "RW", "", "the string supplied is too long to be a phone number"}, // too long } - for _, tc := range tcs { - parsed, err := parsePhoneOrShortcode(tc.input, tc.country) + for i, tc := range testCases { + urn, err := urns.ParsePhone(tc.input, tc.country) + + if tc.expectedErr != "" { + if assert.EqualError(t, err, tc.expectedErr, "%d: expected error for %s, %s", i, tc.input, tc.country) { + assert.Equal(t, urns.NilURN, urn) - if tc.expected != "" { - assert.NoError(t, err, "unexpected error for '%s'", tc.input) - assert.Equal(t, tc.expected, parsed, "result mismatch for '%s'", tc.input) + // check parsing as just a number rather than a phone URN + num, err := urns.ParseNumber(tc.input, tc.country) + assert.EqualError(t, err, tc.expectedErr) + assert.Equal(t, "", num) + } } else { - assert.Error(t, err, "expected error for '%s'", tc.input) + 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 parsing as just a number rather than a phone URN + num, err := urns.ParseNumber(tc.input, tc.country) + assert.NoError(t, err) + assert.Equal(t, tc.expectedURN.Path(), num) + } } } } + func TestToLocalPhone(t *testing.T) { tcs := []struct { - urn URN + urn urns.URN country i18n.Country expected string }{ @@ -86,6 +84,6 @@ func TestToLocalPhone(t *testing.T) { } for _, tc := range tcs { - assert.Equal(t, tc.expected, ToLocalPhone(tc.urn, tc.country), "local mismatch for '%s'", tc.urn) + assert.Equal(t, tc.expected, urns.ToLocalPhone(tc.urn, tc.country), "local mismatch for '%s'", tc.urn) } } diff --git a/urns/schemes.go b/urns/schemes.go index 8a17422..495b5be 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 telRegex = regexp.MustCompile(`^\+?[a-zA-Z0-9]{1,64}$`) +var phoneRegex = regexp.MustCompile(`^\+?\d{1,64}$`) var twitterHandleRegex = regexp.MustCompile(`^[a-zA-Z0-9_]{1,15}$`) var webchatRegex = regexp.MustCompile(`^[a-zA-Z0-9]{24}(:[^\s@]+@[^\s@]+)?$`) @@ -129,7 +129,7 @@ var Phone = &Scheme{ // might have alpha characters in it return strings.ToUpper(path) }, - Validate: func(path string) bool { return telRegex.MatchString(path) }, + Validate: func(path string) bool { return phoneRegex.MatchString(path) }, Format: func(path string) string { parsed, err := phonenumbers.Parse(path, "") if err != nil { diff --git a/urns/urns_test.go b/urns/urns_test.go index 7d1a00b..c534a32 100644 --- a/urns/urns_test.go +++ b/urns/urns_test.go @@ -161,12 +161,11 @@ func TestValidate(t *testing.T) { {"tel:+250123", ""}, {"tel:1337", ""}, {"tel:1", ""}, // one digit shortcodes are a thing - {"tel:PRIZES", ""}, - {"tel:cellbroadcastchannel50", ""}, // invalid tel numbers - {"tel:07883 83383", "invalid path component"}, // can't have spaces {"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 // twitter handles {"twitter:jimmyjo", ""},