Skip to content

Commit

Permalink
Add check for phone-context. (#2875)
Browse files Browse the repository at this point in the history 
* Add check for `phone-context`.

Add a check to PhoneNumberUtil that the value of the `phone-context` parameter of the tel URI follows the correct syntax as defined in [RFC3966](https://www.rfc-editor.org/rfc/rfc3966#section-3).

* Add check for `phone-context`.

Add a check to PhoneNumberUtil that the value of the `phone-context` parameter of the tel URI follows the correct syntax as defined in [RFC3966](https://www.rfc-editor.org/rfc/rfc3966#section-3).

* Add check for `phone-context`.

Add a check to PhoneNumberUtil that the value of the `phone-context` parameter of the tel URI follows the correct syntax as defined in [RFC3966](https://www.rfc-editor.org/rfc/rfc3966#section-3).
  • Loading branch information
@fabriceberchtold
fabriceberchtold authored Mar 9, 2023
1 parent cf5b128 commit d5cb012
Show file tree
Hide file tree
Showing 10 changed files with 700 additions and 189 deletions.
114 changes: 98 additions & 16 deletions cpp/src/phonenumbers/phonenumberutil.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,13 @@ const char kRfc3966ExtnPrefix[] = ";ext=";
const char kRfc3966Prefix[] = "tel:";
const char kRfc3966PhoneContext[] = ";phone-context=";
const char kRfc3966IsdnSubaddress[] = ";isub=";
const char kRfc3966VisualSeparator[] = "[\\-\\.\\(\\)]?";

const char kDigits[] = "\\p{Nd}";
// We accept alpha characters in phone numbers, ASCII only. We store lower-case
// here only since our regular expressions are case-insensitive.
const char kValidAlpha[] = "a-z";
const char kValidAlphaInclUppercase[] = "A-Za-z";

// Default extension prefix to use when formatting. This will be put in front of
// any extension component of the number, after the main national number is
Expand Down Expand Up @@ -654,6 +656,13 @@ class PhoneNumberRegExpsAndMappings {
// indicators. When matching, these are hardly ever used to indicate this.
const string extn_patterns_for_parsing_;

// Regular expressions of different parts of the phone-context parameter,
// following the syntax defined in RFC3966.
const std::string rfc3966_phone_digit_;
const std::string alphanum_;
const std::string rfc3966_domainlabel_;
const std::string rfc3966_toplabel_;

public:
scoped_ptr<const AbstractRegExpFactory> regexp_factory_;
scoped_ptr<RegExpCache> regexp_cache_;
Expand Down Expand Up @@ -756,6 +765,14 @@ class PhoneNumberRegExpsAndMappings {

scoped_ptr<const RegExp> plus_chars_pattern_;

// Regular expression of valid global-number-digits for the phone-context
// parameter, following the syntax defined in RFC3966.
std::unique_ptr<const RegExp> rfc3966_global_number_digits_pattern_;

// Regular expression of valid domainname for the phone-context parameter,
// following the syntax defined in RFC3966.
std::unique_ptr<const RegExp> rfc3966_domainname_pattern_;

PhoneNumberRegExpsAndMappings()
: valid_phone_number_(
StrCat(kDigits, "{", PhoneNumberUtil::kMinLengthForNsn, "}|[",
Expand All @@ -764,6 +781,13 @@ class PhoneNumberRegExpsAndMappings {
kDigits, "){3,}[", PhoneNumberUtil::kValidPunctuation,
kStarSign, kValidAlpha, kDigits, "]*")),
extn_patterns_for_parsing_(CreateExtnPattern(/* for_parsing= */ true)),
rfc3966_phone_digit_(
StrCat("(", kDigits, "|", kRfc3966VisualSeparator, ")")),
alphanum_(StrCat(kValidAlphaInclUppercase, kDigits)),
rfc3966_domainlabel_(
StrCat("[", alphanum_, "]+((\\-)*[", alphanum_, "])*")),
rfc3966_toplabel_(StrCat("[", kValidAlphaInclUppercase,
"]+((\\-)*[", alphanum_, "])*")),
regexp_factory_(new RegExpFactory()),
regexp_cache_(new RegExpCache(*regexp_factory_.get(), 128)),
diallable_char_mappings_(),
Expand Down Expand Up @@ -809,7 +833,12 @@ class PhoneNumberRegExpsAndMappings {
regexp_factory_->CreateRegExp("(\\$\\d)")),
carrier_code_pattern_(regexp_factory_->CreateRegExp("\\$CC")),
plus_chars_pattern_(regexp_factory_->CreateRegExp(
StrCat("[", PhoneNumberUtil::kPlusChars, "]+"))) {
StrCat("[", PhoneNumberUtil::kPlusChars, "]+"))),
rfc3966_global_number_digits_pattern_(regexp_factory_->CreateRegExp(
StrCat("^\\", kPlusSign, rfc3966_phone_digit_, "*", kDigits,
rfc3966_phone_digit_, "*$"))),
rfc3966_domainname_pattern_(regexp_factory_->CreateRegExp(StrCat(
"^(", rfc3966_domainlabel_, "\\.)*", rfc3966_toplabel_, "\\.?$"))) {
InitializeMapsAndSets();
}

Expand Down Expand Up @@ -2118,30 +2147,78 @@ bool PhoneNumberUtil::CheckRegionForParsing(
return true;
}

// Extracts the value of the phone-context parameter of number_to_extract_from
// where the index of ";phone-context=" is parameter index_of_phone_context,
// following the syntax defined in RFC3966.
// Returns the extracted string_view (possibly empty), or a nullopt if no
// phone-context parameter is found.
absl::optional<string> PhoneNumberUtil::ExtractPhoneContext(
const string& number_to_extract_from,
const size_t index_of_phone_context) const {
// If no phone-context parameter is present
if (index_of_phone_context == std::string::npos) {
return absl::nullopt;
}

size_t phone_context_start =
index_of_phone_context + strlen(kRfc3966PhoneContext);
// If phone-context parameter is empty
if (phone_context_start >= number_to_extract_from.length()) {
return "";
}

size_t phone_context_end =
number_to_extract_from.find(';', phone_context_start);
// If phone-context is not the last parameter
if (phone_context_end != std::string::npos) {
return number_to_extract_from.substr(
phone_context_start, phone_context_end - phone_context_start);
} else {
return number_to_extract_from.substr(phone_context_start);
}
}

// Returns whether the value of phoneContext follows the syntax defined in
// RFC3966.
bool PhoneNumberUtil::IsPhoneContextValid(
const absl::optional<string> phone_context) const {
if (!phone_context.has_value()) {
return true;
}
if (phone_context.value().empty()) {
return false;
}

// Does phone-context value match pattern of global-number-digits or
// domainname
return reg_exps_->rfc3966_global_number_digits_pattern_->FullMatch(
std::string{phone_context.value()}) ||
reg_exps_->rfc3966_domainname_pattern_->FullMatch(
std::string{phone_context.value()});
}

// Converts number_to_parse to a form that we can parse and write it to
// national_number if it is written in RFC3966; otherwise extract a possible
// number out of it and write to national_number.
void PhoneNumberUtil::BuildNationalNumberForParsing(
PhoneNumberUtil::ErrorType PhoneNumberUtil::BuildNationalNumberForParsing(
const string& number_to_parse, string* national_number) const {
size_t index_of_phone_context = number_to_parse.find(kRfc3966PhoneContext);
if (index_of_phone_context != string::npos) {
size_t phone_context_start =
index_of_phone_context + strlen(kRfc3966PhoneContext);

absl::optional<string> phone_context =
ExtractPhoneContext(number_to_parse, index_of_phone_context);
if (!IsPhoneContextValid(phone_context)) {
VLOG(2) << "The phone-context value is invalid.";
return NOT_A_NUMBER;
}

if (phone_context.has_value()) {
// If the phone context contains a phone number prefix, we need to capture
// it, whereas domains will be ignored.
if (phone_context_start < (number_to_parse.length() - 1) &&
number_to_parse.at(phone_context_start) == kPlusSign[0]) {
if (phone_context.value().at(0) == kPlusSign[0]) {
// Additional parameters might follow the phone context. If so, we will
// remove them here because the parameters after phone context are not
// important for parsing the phone number.
size_t phone_context_end = number_to_parse.find(';', phone_context_start);
if (phone_context_end != string::npos) {
StrAppend(
national_number, number_to_parse.substr(
phone_context_start, phone_context_end - phone_context_start));
} else {
StrAppend(national_number, number_to_parse.substr(phone_context_start));
}
StrAppend(national_number, phone_context.value());
}

// Now append everything between the "tel:" prefix and the phone-context.
Expand Down Expand Up @@ -2175,6 +2252,7 @@ void PhoneNumberUtil::BuildNationalNumberForParsing(
// we are concerned about deleting content from a potential number string
// when there is no strong evidence that the number is actually written in
// RFC3966.
return NO_PARSING_ERROR;
}

// Note if any new field is added to this method that should always be filled
Expand All @@ -2189,7 +2267,11 @@ PhoneNumberUtil::ErrorType PhoneNumberUtil::ParseHelper(
DCHECK(phone_number);

string national_number;
BuildNationalNumberForParsing(number_to_parse, &national_number);
PhoneNumberUtil::ErrorType build_national_number_for_parsing_return =
BuildNationalNumberForParsing(number_to_parse, &national_number);
if (build_national_number_for_parsing_return != NO_PARSING_ERROR) {
return build_national_number_for_parsing_return;
}

if (!IsViablePhoneNumber(national_number)) {
VLOG(2) << "The string supplied did not seem to be a phone number.";
Expand Down
10 changes: 8 additions & 2 deletions cpp/src/phonenumbers/phonenumberutil.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -958,8 +958,14 @@ class PhoneNumberUtil : public Singleton<PhoneNumberUtil> {
bool check_region,
PhoneNumber* phone_number) const;

void BuildNationalNumberForParsing(const string& number_to_parse,
string* national_number) const;
absl::optional<string> ExtractPhoneContext(
const string& number_to_extract_from,
size_t index_of_phone_context) const;

bool IsPhoneContextValid(absl::optional<string> phone_context) const;

ErrorType BuildNationalNumberForParsing(const string& number_to_parse,
string* national_number) const;

bool IsShorterThanPossibleNormalNumber(const PhoneMetadata* country_metadata,
const string& number) const;
Expand Down
107 changes: 99 additions & 8 deletions cpp/test/phonenumbers/phonenumberutil_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,13 @@ class PhoneNumberUtilTest : public testing::Test {
return phone_util_.ContainsOnlyValidDigits(s);
}

void AssertThrowsForInvalidPhoneContext(const string number_to_parse) {
PhoneNumber actual_number;
EXPECT_EQ(
PhoneNumberUtil::NOT_A_NUMBER,
phone_util_.Parse(number_to_parse, RegionCode::ZZ(), &actual_number));
}

const PhoneNumberUtil& phone_util_;

private:
Expand Down Expand Up @@ -3653,13 +3660,6 @@ TEST_F(PhoneNumberUtilTest, ParseNationalNumber) {
"tel:253-0000;isub=12345;phone-context=www.google.com",
RegionCode::US(), &test_number));
EXPECT_EQ(us_local_number, test_number);
// This is invalid because no "+" sign is present as part of phone-context.
// The phone context is simply ignored in this case just as if it contains a
// domain.
EXPECT_EQ(PhoneNumberUtil::NO_PARSING_ERROR,
phone_util_.Parse("tel:2530000;isub=12345;phone-context=1-650",
RegionCode::US(), &test_number));
EXPECT_EQ(us_local_number, test_number);
EXPECT_EQ(PhoneNumberUtil::NO_PARSING_ERROR,
phone_util_.Parse("tel:2530000;isub=12345;phone-context=1234.com",
RegionCode::US(), &test_number));
Expand Down Expand Up @@ -4085,7 +4085,7 @@ TEST_F(PhoneNumberUtilTest, FailedParseOnInvalidNumbers) {
EXPECT_EQ(PhoneNumber::default_instance(), test_number);
// This is invalid because no "+" sign is present as part of phone-context.
// This should not succeed in being parsed.
EXPECT_EQ(PhoneNumberUtil::INVALID_COUNTRY_CODE_ERROR,
EXPECT_EQ(PhoneNumberUtil::NOT_A_NUMBER,
phone_util_.Parse("tel:555-1234;phone-context=1-331",
RegionCode::ZZ(), &test_number));
EXPECT_EQ(PhoneNumber::default_instance(), test_number);
Expand Down Expand Up @@ -4703,6 +4703,97 @@ TEST_F(PhoneNumberUtilTest, ParseItalianLeadingZeros) {
EXPECT_EQ(zeros_number, test_number);
}

TEST_F(PhoneNumberUtilTest, ParseWithPhoneContext) {
PhoneNumber expected_number;
expected_number.set_country_code(64);
expected_number.set_national_number(33316005L);
PhoneNumber actual_number;

// context = ";phone-context=" descriptor
// descriptor = domainname / global-number-digits

// Valid global-phone-digits
EXPECT_EQ(PhoneNumberUtil::NO_PARSING_ERROR,
phone_util_.Parse("tel:033316005;phone-context=+64",
RegionCode::ZZ(), &actual_number));
EXPECT_EQ(expected_number, actual_number);
actual_number.Clear();

EXPECT_EQ(PhoneNumberUtil::NO_PARSING_ERROR,
phone_util_.Parse("tel:033316005;phone-context=+64;{this isn't "
"part of phone-context anymore!}",
RegionCode::ZZ(), &actual_number));
EXPECT_EQ(expected_number, actual_number);
actual_number.Clear();

expected_number.set_national_number(3033316005L);
EXPECT_EQ(PhoneNumberUtil::NO_PARSING_ERROR,
phone_util_.Parse("tel:033316005;phone-context=+64-3",
RegionCode::ZZ(), &actual_number));
EXPECT_EQ(expected_number, actual_number);
actual_number.Clear();

expected_number.set_country_code(55);
expected_number.set_national_number(5033316005L);
EXPECT_EQ(PhoneNumberUtil::NO_PARSING_ERROR,
phone_util_.Parse("tel:033316005;phone-context=+(555)",
RegionCode::ZZ(), &actual_number));
EXPECT_EQ(expected_number, actual_number);
actual_number.Clear();

expected_number.set_country_code(1);
expected_number.set_national_number(23033316005L);
EXPECT_EQ(PhoneNumberUtil::NO_PARSING_ERROR,
phone_util_.Parse("tel:033316005;phone-context=+-1-2.3()",
RegionCode::ZZ(), &actual_number));
EXPECT_EQ(expected_number, actual_number);
actual_number.Clear();

// Valid domainname
expected_number.set_country_code(64);
expected_number.set_national_number(33316005L);
EXPECT_EQ(PhoneNumberUtil::NO_PARSING_ERROR,
phone_util_.Parse("tel:033316005;phone-context=abc.nz",
RegionCode::NZ(), &actual_number));
EXPECT_EQ(expected_number, actual_number);
actual_number.Clear();

EXPECT_EQ(
PhoneNumberUtil::NO_PARSING_ERROR,
phone_util_.Parse("tel:033316005;phone-context=www.PHONE-numb3r.com",
RegionCode::NZ(), &actual_number));
EXPECT_EQ(expected_number, actual_number);
actual_number.Clear();

EXPECT_EQ(PhoneNumberUtil::NO_PARSING_ERROR,
phone_util_.Parse("tel:033316005;phone-context=a", RegionCode::NZ(),
&actual_number));
EXPECT_EQ(expected_number, actual_number);
actual_number.Clear();

EXPECT_EQ(PhoneNumberUtil::NO_PARSING_ERROR,
phone_util_.Parse("tel:033316005;phone-context=3phone.J.",
RegionCode::NZ(), &actual_number));
EXPECT_EQ(expected_number, actual_number);
actual_number.Clear();

EXPECT_EQ(PhoneNumberUtil::NO_PARSING_ERROR,
phone_util_.Parse("tel:033316005;phone-context=a--z",
RegionCode::NZ(), &actual_number));
EXPECT_EQ(expected_number, actual_number);

// Invalid descriptor
AssertThrowsForInvalidPhoneContext("tel:033316005;phone-context=");
AssertThrowsForInvalidPhoneContext("tel:033316005;phone-context=+");
AssertThrowsForInvalidPhoneContext("tel:033316005;phone-context=64");
AssertThrowsForInvalidPhoneContext("tel:033316005;phone-context=++64");
AssertThrowsForInvalidPhoneContext("tel:033316005;phone-context=+abc");
AssertThrowsForInvalidPhoneContext("tel:033316005;phone-context=.");
AssertThrowsForInvalidPhoneContext("tel:033316005;phone-context=3phone");
AssertThrowsForInvalidPhoneContext("tel:033316005;phone-context=a-.nz");
AssertThrowsForInvalidPhoneContext("tel:033316005;phone-context=a{b}c");
}

TEST_F(PhoneNumberUtilTest, CanBeInternationallyDialled) {
PhoneNumber test_number;
test_number.set_country_code(1);
Expand Down
7 changes: 4 additions & 3 deletions java/libphonenumber/src/com/google/i18n/phonenumbers/NumberParseException.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,10 @@ public enum ErrorType {
*/
INVALID_COUNTRY_CODE,
/**
* This generally indicates the string passed in had less than 3 digits in it. More
* specifically, the number failed to match the regular expression VALID_PHONE_NUMBER in
* PhoneNumberUtil.java.
* This indicates the string passed is not a valid number. Either the string had less than 3
* digits in it or had an invalid phone-context parameter. More specifically, the number failed
* to match the regular expression VALID_PHONE_NUMBER, RFC3966_GLOBAL_NUMBER_DIGITS, or
* RFC3966_DOMAINNAME in PhoneNumberUtil.java.
*/
NOT_A_NUMBER,
/**
Expand Down
Loading

0 comments on commit d5cb012

Please sign in to comment.