diff --git a/cpp/src/phonenumbers/phonenumberutil.cc b/cpp/src/phonenumbers/phonenumberutil.cc index 984ff86ea..69e6ce562 100644 --- a/cpp/src/phonenumbers/phonenumberutil.cc +++ b/cpp/src/phonenumbers/phonenumberutil.cc @@ -316,9 +316,6 @@ void GetSupportedTypesForMetadata( // Helper method to check a number against possible lengths for this number // type, and determine whether it matches, or is too short or too long. -// Currently, if a number pattern suggests that numbers of length 7 and 10 are -// possible, and a number in between these possible lengths is entered, such as -// of length 8, this will return TOO_LONG. PhoneNumberUtil::ValidationResult TestNumberLength( const string& number, const PhoneMetadata& metadata, PhoneNumberUtil::PhoneNumberType type) { @@ -397,9 +394,7 @@ PhoneNumberUtil::ValidationResult TestNumberLength( // Helper method to check a number against possible lengths for this region, // based on the metadata being passed in, and determine whether it matches, or -// is too short or too long. Currently, if a number pattern suggests that -// numbers of length 7 and 10 are possible, and a number in between these -// possible lengths is entered, such as of length 8, this will return TOO_LONG. +// is too short or too long. PhoneNumberUtil::ValidationResult TestNumberLength( const string& number, const PhoneMetadata& metadata) { return TestNumberLength(number, metadata, PhoneNumberUtil::UNKNOWN); @@ -2184,8 +2179,11 @@ PhoneNumberUtil::ErrorType PhoneNumberUtil::ParseHelper( // and carrier code be long enough to be a possible length for the region. // Otherwise, we don't do the stripping, since the original number could be // a valid short number. - if (TestNumberLength(potential_national_number, *country_metadata) != - TOO_SHORT) { + ValidationResult validation_result = + TestNumberLength(potential_national_number, *country_metadata); + if (validation_result != TOO_SHORT && + validation_result != IS_POSSIBLE_LOCAL_ONLY && + validation_result != INVALID_LENGTH) { normalized_national_number.assign(potential_national_number); if (keep_raw_input && !carrier_code.empty()) { temp_number.set_preferred_domestic_carrier_code(carrier_code); diff --git a/cpp/test/phonenumbers/phonenumberutil_test.cc b/cpp/test/phonenumbers/phonenumberutil_test.cc index 84632c7ab..4eb7a3ac4 100644 --- a/cpp/test/phonenumbers/phonenumberutil_test.cc +++ b/cpp/test/phonenumbers/phonenumberutil_test.cc @@ -3662,6 +3662,17 @@ TEST_F(PhoneNumberUtilTest, ParseNationalNumber) { EXPECT_EQ(PhoneNumberUtil::NO_PARSING_ERROR, phone_util_.Parse("12", RegionCode::NZ(), &test_number)); EXPECT_EQ(short_number, test_number); + + // Test for short-code with leading zero for a country which has 0 as + // national prefix. Ensure it's not interpreted as national prefix if the + // remaining number length is local-only in terms of length. Example: In GB, + // length 6-7 are only possible local-only. + short_number.set_country_code(44); + short_number.set_national_number(123456); + short_number.set_italian_leading_zero(true); + EXPECT_EQ(PhoneNumberUtil::NO_PARSING_ERROR, + phone_util_.Parse("0123456", RegionCode::GB(), &test_number)); + EXPECT_EQ(short_number, test_number); } TEST_F(PhoneNumberUtilTest, ParseNumberWithAlphaCharacters) { diff --git a/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java b/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java index 766d0279c..bc8a53eb0 100644 --- a/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java +++ b/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java @@ -2478,9 +2478,7 @@ public class PhoneNumberUtil { /** * Helper method to check a number against possible lengths for this region, based on the metadata - * being passed in, and determine whether it matches, or is too short or too long. Currently, if a - * number pattern suggests that numbers of length 7 and 10 are possible, and a number in between - * these possible lengths is entered, such as of length 8, this will return TOO_LONG. + * being passed in, and determine whether it matches, or is too short or too long. */ private ValidationResult testNumberLength(CharSequence number, PhoneMetadata metadata) { return testNumberLength(number, metadata, PhoneNumberType.UNKNOWN); @@ -2488,9 +2486,7 @@ public class PhoneNumberUtil { /** * Helper method to check a number against possible lengths for this number type, and determine - * whether it matches, or is too short or too long. Currently, if a number pattern suggests that - * numbers of length 7 and 10 are possible, and a number in between these possible lengths is - * entered, such as of length 8, this will return TOO_LONG. + * whether it matches, or is too short or too long. */ private ValidationResult testNumberLength( CharSequence number, PhoneMetadata metadata, PhoneNumberType type) { @@ -3222,7 +3218,10 @@ public class PhoneNumberUtil { // We require that the NSN remaining after stripping the national prefix and carrier code be // long enough to be a possible length for the region. Otherwise, we don't do the stripping, // since the original number could be a valid short number. - if (testNumberLength(potentialNationalNumber, regionMetadata) != ValidationResult.TOO_SHORT) { + ValidationResult validationResult = testNumberLength(potentialNationalNumber, regionMetadata); + if (validationResult != ValidationResult.TOO_SHORT + && validationResult != ValidationResult.IS_POSSIBLE_LOCAL_ONLY + && validationResult != ValidationResult.INVALID_LENGTH) { normalizedNationalNumber = potentialNationalNumber; if (keepRawInput && carrierCode.length() > 0) { phoneNumber.setPreferredDomesticCarrierCode(carrierCode.toString()); diff --git a/java/libphonenumber/test/com/google/i18n/phonenumbers/PhoneNumberUtilTest.java b/java/libphonenumber/test/com/google/i18n/phonenumbers/PhoneNumberUtilTest.java index cc5e325a0..5da7e9c95 100644 --- a/java/libphonenumber/test/com/google/i18n/phonenumbers/PhoneNumberUtilTest.java +++ b/java/libphonenumber/test/com/google/i18n/phonenumbers/PhoneNumberUtilTest.java @@ -2121,6 +2121,13 @@ public class PhoneNumberUtilTest extends TestMetadataTestCase { PhoneNumber shortNumber = new PhoneNumber(); shortNumber.setCountryCode(64).setNationalNumber(12L); assertEquals(shortNumber, phoneUtil.parse("12", RegionCode.NZ)); + + // Test for short-code with leading zero for a country which has 0 as national prefix. Ensure + // it's not interpreted as national prefix if the remaining number length is local-only in + // terms of length. Example: In GB, length 6-7 are only possible local-only. + shortNumber.setCountryCode(44).setNationalNumber(123456) + .setItalianLeadingZero(true); + assertEquals(shortNumber, phoneUtil.parse("0123456", RegionCode.GB)); } public void testParseNumberWithAlphaCharacters() throws Exception { diff --git a/javascript/i18n/phonenumbers/phonenumberutil.js b/javascript/i18n/phonenumbers/phonenumberutil.js index adef74b79..8ab26796d 100644 --- a/javascript/i18n/phonenumbers/phonenumberutil.js +++ b/javascript/i18n/phonenumbers/phonenumberutil.js @@ -3313,9 +3313,7 @@ i18n.phonenumbers.PhoneNumberUtil.prototype.isPossibleNumberForType = /** * Helper method to check a number against possible lengths for this region, * based on the metadata being passed in, and determine whether it matches, or - * is too short or too long. Currently, if a number pattern suggests that - * numbers of length 7 and 10 are possible, and a number in between these - * possible lengths is entered, such as of length 8, this will return TOO_LONG. + * is too short or too long. * * @param {string} number * @param {i18n.phonenumbers.PhoneMetadata} metadata @@ -3331,10 +3329,7 @@ i18n.phonenumbers.PhoneNumberUtil.prototype.testNumberLength_ = /** * Helper method to check a number against a particular pattern and determine - * whether it matches, or is too short or too long. Currently, if a number - * pattern suggests that numbers of length 7 and 10 are possible, and a number - * in between these possible lengths is entered, such as of length 8, this will - * return TOO_LONG. + * whether it matches, or is too short or too long. * * @param {string} number * @param {i18n.phonenumbers.PhoneMetadata} metadata @@ -4215,9 +4210,14 @@ i18n.phonenumbers.PhoneNumberUtil.prototype.parseHelper_ = // carrier code be long enough to be a possible length for the region. // Otherwise, we don't do the stripping, since the original number could be // a valid short number. - if (this.testNumberLength_(potentialNationalNumber.toString(), - regionMetadata) != - i18n.phonenumbers.PhoneNumberUtil.ValidationResult.TOO_SHORT) { + var validationResult = this.testNumberLength_( + potentialNationalNumber.toString(), regionMetadata); + if (validationResult != + i18n.phonenumbers.PhoneNumberUtil.ValidationResult.TOO_SHORT && + validationResult != + i18n.phonenumbers.PhoneNumberUtil.ValidationResult.IS_POSSIBLE_LOCAL_ONLY && + validationResult != + i18n.phonenumbers.PhoneNumberUtil.ValidationResult.INVALID_LENGTH) { normalizedNationalNumber = potentialNationalNumber; if (keepRawInput && carrierCode.toString().length > 0) { phoneNumber.setPreferredDomesticCarrierCode(carrierCode.toString()); diff --git a/javascript/i18n/phonenumbers/phonenumberutil_test.js b/javascript/i18n/phonenumbers/phonenumberutil_test.js index 89cd8e513..44a43a485 100644 --- a/javascript/i18n/phonenumbers/phonenumberutil_test.js +++ b/javascript/i18n/phonenumbers/phonenumberutil_test.js @@ -2759,6 +2759,15 @@ function testParseNationalNumber() { shortNumber.setCountryCode(64); shortNumber.setNationalNumber(12); assertTrue(shortNumber.equals(phoneUtil.parse('12', RegionCode.NZ))); + + // Test for short-code with leading zero for a country which has 0 as + // national prefix. Ensure it's not interpreted as national prefix if the + // remaining number length is local-only in terms of length. Example: In GB, + // length 6-7 are only possible local-only. + shortNumber.setCountryCode(44); + shortNumber.setNationalNumber(123456); + shortNumber.setItalianLeadingZero(true); + assertTrue(shortNumber.equals(phoneUtil.parse('0123456', RegionCode.GB))); } function testParseNumberWithAlphaCharacters() { diff --git a/pending_code_changes.txt b/pending_code_changes.txt index bdb3e52ea..bdc86680c 100644 --- a/pending_code_changes.txt +++ b/pending_code_changes.txt @@ -1,2 +1,6 @@ Code changes: + - Improve parsing logic to be smarter about national-prefix detection & + stripping based on possible-lengths (IS_POSSIBLE_LOCAL_ONLY and + INVALID_LENGTH). Enables e.g. adding Iran short-codes starting with "096" + without the need to hack IR's national prefix parsing config. - Logging changes: Don't log client-provided phone numbers.