diff --git a/csharp/PhoneNumbers.Test/TestAsYouTypeFormatter.cs b/csharp/PhoneNumbers.Test/TestAsYouTypeFormatter.cs index 030d25c6..e0c43abb 100644 --- a/csharp/PhoneNumbers.Test/TestAsYouTypeFormatter.cs +++ b/csharp/PhoneNumbers.Test/TestAsYouTypeFormatter.cs @@ -953,5 +953,42 @@ public void TestAYTFLongNDD_SG() Assert.Equal("777777 9876 789", formatter.InputDigit('9')); Assert.Equal("777777 9876 7890", formatter.InputDigit('0')); } + + [Fact] + public void TestInputDigit_KeepsEchoingWithinMaxInputStringLength() + { + // A plausible, if unusually long, real-world entry (national number, verbose extension, + // several pause characters) must not be silently truncated/frozen. This regression would + // previously trip once accrued input passed 50 characters: every InputDigit call from then + // on returned the same frozen snapshot no matter what was typed next. 250 is + // PhoneNumberUtil.MAX_INPUT_STRING_LENGTH - the same bound Parse() itself uses for "is this + // even plausibly a phone number" - so nothing here should ever hit the formatter's cost cap. + var formatter = phoneUtil.GetAsYouTypeFormatter("US"); + const string input = "12345678901 ext. 123456789,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,"; + Assert.True(input.Length < 250, "test input should stay well under the cap for this test to be meaningful"); + Assert.True(input.Length > 50, "test input should exceed the old, too-tight cap for this test to be meaningful"); + string previous = ""; + foreach (var c in input) + { + var result = formatter.InputDigit(c); + Assert.NotEqual(previous, result); + previous = result; + } + } + + [Fact] + public void TestInputDigit_CapsPathologicallyLongInput() + { + // Once input exceeds PhoneNumberUtil.MAX_INPUT_STRING_LENGTH, Parse() itself would already + // refuse to treat it as a phone number, so the formatter is free to stop growing/formatting + // it - this is what actually bounds AsYouTypeFormatter's cost for pathological input. + var formatter = phoneUtil.GetAsYouTypeFormatter("US"); + string result = ""; + for (var i = 0; i < 300; i++) + { + result = formatter.InputDigit('1'); + } + Assert.Equal(250, result.Length); + } } } diff --git a/csharp/PhoneNumbers/AsYouTypeFormatter.cs b/csharp/PhoneNumbers/AsYouTypeFormatter.cs index 6081c95f..796da74c 100644 --- a/csharp/PhoneNumbers/AsYouTypeFormatter.cs +++ b/csharp/PhoneNumbers/AsYouTypeFormatter.cs @@ -59,6 +59,27 @@ public partial class AsYouTypeFormatter // Character used when appropriate to separate a prefix, such as a long NDD or a country calling // code, from the national number. private const char SeparatorBeforeNationalNumber = ' '; + + // Matches PhoneNumberUtil.MAX_INPUT_STRING_LENGTH - Parse() itself refuses anything longer than + // this as not a phone number, so there is no legitimate as-you-type input (number, extension, + // and pause/wait characters included) that should ever reach this cap; only pathological input + // does. Every InputDigit call re-derives its return value from scratch off the accrued buffers - + // including a regex match against the whole accumulated nationalNumber in + // AttemptToFormatAccruedDigits, and accruedInput.ToString() itself on every bail-out path - so + // cost grows worse than quadratically with digits typed if the buffers are left to grow + // unbounded. Past this many characters we stop growing accruedInput and freeze the output into + // cappedOutput, so every further call is O(1) instead of re-copying an ever-longer buffer. + private const int MaxAccruedCharsForFormatting = PhoneNumberUtil.MAX_INPUT_STRING_LENGTH; + + // Set once accruedInput.Length reaches MaxAccruedCharsForFormatting; null until then. See + // MaxAccruedCharsForFormatting for why this exists. + // + // Scoped #nullable enable/restore rather than a file-level pragma: this file's nullable context + // is otherwise oblivious on netstandard2.0 (Nullable isn't set for that TFM), and enabling it + // for the whole file would surface a pile of never-checked nullable warnings elsewhere in it. +#nullable enable + private string? cappedOutput; +#nullable restore private static readonly PhoneMetadata EmptyMetadata = new() { InternationalPrefix = "NA" }; private readonly PhoneMetadata defaultMetaData; private PhoneMetadata currentMetadata; @@ -268,6 +289,7 @@ private string GetFormattingTemplate(string numberPattern, string numberFormat) public void Clear() { currentOutput = ""; + cappedOutput = null; accruedInput.Length = 0; accruedInputWithoutFormatting.Length = 0; formattingTemplate.Length = 0; @@ -320,6 +342,15 @@ public string InputDigitAndRememberPosition(char nextChar) private string InputDigitWithOptionToRememberPosition(char nextChar, bool rememberPosition) { + if (cappedOutput != null) + { + return cappedOutput; + } + if (accruedInput.Length >= MaxAccruedCharsForFormatting) + { + cappedOutput = accruedInput.ToString(); + return cappedOutput; + } accruedInput.Append(nextChar); if (rememberPosition) { diff --git a/csharp/PhoneNumbers/PhoneNumberUtil.cs b/csharp/PhoneNumbers/PhoneNumberUtil.cs index 238af0ab..c0bcbe27 100644 --- a/csharp/PhoneNumbers/PhoneNumberUtil.cs +++ b/csharp/PhoneNumbers/PhoneNumberUtil.cs @@ -62,7 +62,7 @@ public partial class PhoneNumberUtil // We don't allow input strings for parsing to be longer than this. This prevents malicious // input from overflowing the regular-expression engine. - private const int MAX_INPUT_STRING_LENGTH = 250; + internal const int MAX_INPUT_STRING_LENGTH = 250; // Region-code for the unknown region. private const string UNKNOWN_REGION = "ZZ"; @@ -673,6 +673,8 @@ public static string ExtractPossibleNumber(string number) /// True if the number could be a phone number of some sort, otherwise false. public static bool IsViablePhoneNumber(string number) { + if (number == null) + throw new ArgumentNullException(nameof(number)); if (number.Length < MIN_LENGTH_FOR_NSN) return false; return ValidPhoneNumber().IsMatch(number); @@ -1828,6 +1830,8 @@ private bool IsNumberMatchingDesc(string nationalNumber, PhoneNumberDesc numberD /// A bool that indicates whether the number is of a valid pattern. public bool IsValidNumber(PhoneNumber number) { + if (number == null) + throw new ArgumentNullException(nameof(number)); // Inlined to share the national significant number string between region lookup and // validation in the case where the country calling code maps to multiple regions // (e.g. NANPA). For single-region calling codes the NSN is only computed once anyway. @@ -1898,6 +1902,8 @@ private bool IsValidNumberForRegion(PhoneNumber number, string regionCode, strin /// code. public string GetRegionCodeForNumber(PhoneNumber number) { + if (number == null) + throw new ArgumentNullException(nameof(number)); countryCallingCodeToRegionCodeMap.TryGetValue(number.CountryCode, out List regions); if (regions == null) { @@ -2063,6 +2069,8 @@ public bool IsAlphaNumber(string number) /// True if the number is possible. public bool IsPossibleNumber(PhoneNumber number) { + if (number == null) + throw new ArgumentNullException(nameof(number)); var result = IsPossibleNumberWithReason(number); return result == ValidationResult.IS_POSSIBLE || result == ValidationResult.IS_POSSIBLE_LOCAL_ONLY; } @@ -3049,6 +3057,10 @@ private static void BuildNationalNumberForParsing(string numberToParse, StringBu /// of the two numbers, described in the method definition. public MatchType IsNumberMatch(PhoneNumber firstNumberIn, PhoneNumber secondNumberIn) { + if (firstNumberIn == null) + throw new ArgumentNullException(nameof(firstNumberIn)); + if (secondNumberIn == null) + throw new ArgumentNullException(nameof(secondNumberIn)); // Early exit if both had extensions and these are different. if (firstNumberIn.HasExtension && secondNumberIn.HasExtension && firstNumberIn.Extension != secondNumberIn.Extension) return MatchType.NO_MATCH; diff --git a/csharp/PhoneNumbers/PhoneNumberUtil.net.cs b/csharp/PhoneNumbers/PhoneNumberUtil.net.cs index 76d546cd..0f99934a 100644 --- a/csharp/PhoneNumbers/PhoneNumberUtil.net.cs +++ b/csharp/PhoneNumbers/PhoneNumberUtil.net.cs @@ -142,6 +142,8 @@ private static string NormalizeToString(string number, NormalizeMode mode) /// The formatted phone number. public string Format(PhoneNumber number, PhoneNumberFormat numberFormat) { + if (number == null) + throw new ArgumentNullException(nameof(number)); if (number.NationalNumber == 0) { // Unparseable numbers that kept their raw input just use that, unless default country was diff --git a/csharp/PhoneNumbers/PhoneNumberUtil.netstandard.cs b/csharp/PhoneNumbers/PhoneNumberUtil.netstandard.cs index 8d42d3d6..044640b9 100644 --- a/csharp/PhoneNumbers/PhoneNumberUtil.netstandard.cs +++ b/csharp/PhoneNumbers/PhoneNumberUtil.netstandard.cs @@ -74,6 +74,8 @@ public static string ConvertAlphaCharactersInNumber(string number) => /// The formatted phone number. public string Format(PhoneNumber number, PhoneNumberFormat numberFormat) { + if (number == null) + throw new ArgumentNullException(nameof(number)); // Unparseable numbers that kept their raw input just use that, unless default country was // specified and the format is E164. In that case, we prepend the raw input with the country // code.