diff --git a/cpp/src/phonenumbers/phonenumberutil.cc b/cpp/src/phonenumbers/phonenumberutil.cc index 15a808d33..6b3299e87 100644 --- a/cpp/src/phonenumbers/phonenumberutil.cc +++ b/cpp/src/phonenumbers/phonenumberutil.cc @@ -2031,11 +2031,12 @@ void 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) { - int phone_context_start = + size_t phone_context_start = index_of_phone_context + strlen(kRfc3966PhoneContext); // If the phone context contains a phone number prefix, we need to capture // it, whereas domains will be ignored. - if (number_to_parse.at(phone_context_start) == kPlusSign[0]) { + if (phone_context_start < (number_to_parse.length() - 1) && + number_to_parse.at(phone_context_start) == 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. diff --git a/cpp/test/phonenumbers/phonenumberutil_test.cc b/cpp/test/phonenumbers/phonenumberutil_test.cc index f71bfddee..aee8fa659 100644 --- a/cpp/test/phonenumbers/phonenumberutil_test.cc +++ b/cpp/test/phonenumbers/phonenumberutil_test.cc @@ -4023,6 +4023,12 @@ TEST_F(PhoneNumberUtilTest, FailedParseOnInvalidNumbers) { phone_util_.Parse("tel:555-1234;phone-context=1-331", RegionCode::ZZ(), &test_number)); EXPECT_EQ(PhoneNumber::default_instance(), test_number); + + // Only the phone-context symbol is present, but no data. + EXPECT_EQ(PhoneNumberUtil::NOT_A_NUMBER, + phone_util_.Parse(";phone-context=", + RegionCode::ZZ(), &test_number)); + EXPECT_EQ(PhoneNumber::default_instance(), test_number); } TEST_F(PhoneNumberUtilTest, ParseNumbersWithPlusWithNoRegion) { diff --git a/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java b/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java index c403759bd..7cda4029c 100644 --- a/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java +++ b/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java @@ -3270,11 +3270,12 @@ public class PhoneNumberUtil { */ private void buildNationalNumberForParsing(String numberToParse, StringBuilder nationalNumber) { int indexOfPhoneContext = numberToParse.indexOf(RFC3966_PHONE_CONTEXT); - if (indexOfPhoneContext > 0) { + if (indexOfPhoneContext >= 0) { int phoneContextStart = indexOfPhoneContext + RFC3966_PHONE_CONTEXT.length(); // If the phone context contains a phone number prefix, we need to capture it, whereas domains // will be ignored. - if (numberToParse.charAt(phoneContextStart) == PLUS_SIGN) { + if (phoneContextStart < (numberToParse.length() - 1) + && numberToParse.charAt(phoneContextStart) == PLUS_SIGN) { // 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. diff --git a/java/libphonenumber/test/com/google/i18n/phonenumbers/PhoneNumberUtilTest.java b/java/libphonenumber/test/com/google/i18n/phonenumbers/PhoneNumberUtilTest.java index 3975e6f03..13a59ad82 100644 --- a/java/libphonenumber/test/com/google/i18n/phonenumbers/PhoneNumberUtilTest.java +++ b/java/libphonenumber/test/com/google/i18n/phonenumbers/PhoneNumberUtilTest.java @@ -2508,6 +2508,17 @@ public class PhoneNumberUtilTest extends TestMetadataTestCase { NumberParseException.ErrorType.INVALID_COUNTRY_CODE, e.getErrorType()); } + try { + // Only the phone-context symbol is present, but no data. + String invalidRfcPhoneContext = ";phone-context="; + phoneUtil.parse(invalidRfcPhoneContext, RegionCode.ZZ); + fail("No number is present: should fail."); + } catch (NumberParseException e) { + // Expected this exception. + assertEquals("Wrong error type stored in exception.", + NumberParseException.ErrorType.NOT_A_NUMBER, + e.getErrorType()); + } } public void testParseNumbersWithPlusWithNoRegion() throws Exception { diff --git a/javascript/i18n/phonenumbers/phonenumberutil.js b/javascript/i18n/phonenumbers/phonenumberutil.js index bb9eefc22..e2086dba4 100644 --- a/javascript/i18n/phonenumbers/phonenumberutil.js +++ b/javascript/i18n/phonenumbers/phonenumberutil.js @@ -4271,11 +4271,13 @@ i18n.phonenumbers.PhoneNumberUtil.prototype.buildNationalNumberForParsing_ = /** @type {number} */ var indexOfPhoneContext = numberToParse.indexOf( i18n.phonenumbers.PhoneNumberUtil.RFC3966_PHONE_CONTEXT_); - if (indexOfPhoneContext > 0) { + if (indexOfPhoneContext >= 0) { var phoneContextStart = indexOfPhoneContext + i18n.phonenumbers.PhoneNumberUtil.RFC3966_PHONE_CONTEXT_.length; // If the phone context contains a phone number prefix, we need to capture // it, whereas domains will be ignored. + // No length check is necessary, as per C++ or Java, since out-of-bounds + // requests to charAt return an empty string. if (numberToParse.charAt(phoneContextStart) == i18n.phonenumbers.PhoneNumberUtil.PLUS_SIGN) { // Additional parameters might follow the phone context. If so, we will diff --git a/javascript/i18n/phonenumbers/phonenumberutil_test.js b/javascript/i18n/phonenumbers/phonenumberutil_test.js index b359faf39..1cca95ed9 100644 --- a/javascript/i18n/phonenumbers/phonenumberutil_test.js +++ b/javascript/i18n/phonenumbers/phonenumberutil_test.js @@ -3217,6 +3217,19 @@ function testFailedParseOnInvalidNumbers() { i18n.phonenumbers.Error.INVALID_COUNTRY_CODE, e.message); } + try { + // Only the phone-context symbol is present, but no data. + /** @type {string} */ + var invalidRfcPhoneContext = ';phone-context='; + phoneUtil.parse(invalidRfcPhoneContext, RegionCode.ZZ); + fail('Should have thrown an exception, no valid country calling code ' + + 'present.'); + } catch (e) { + // Expected. + assertEquals('Wrong error type stored in exception.', + i18n.phonenumbers.Error.NOT_A_NUMBER, + e.message); + } } function testParseNumbersWithPlusWithNoRegion() { diff --git a/pending_code_changes.txt b/pending_code_changes.txt index 8b1378917..4f695b299 100644 --- a/pending_code_changes.txt +++ b/pending_code_changes.txt @@ -1 +1,3 @@ - +Code changes: + - Small fix for possible out-of-bounds exception on RFC3966 input where no + phone context was actually provided.