Handling possible out-of-bounds exception when parsing RFC3966 format (#1690)

* Handling possible out-of-bounds exception that would be thrown in C++
and Java if a number with a phone-context tag is entered (RFC3966
format) but no actual phone context.

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.

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) {

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.

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 {

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

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() {

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.