From 3da6a699d5434ad79831c7ac057f3cf7046a964a Mon Sep 17 00:00:00 2001 From: Sami Daniel Santos Silva Date: Wed, 25 Sep 2024 05:00:57 -0300 Subject: [PATCH 1/4] Refactor PrefixContainerTest to include a new test case for checking if the prefix matches exactly with keys --- .../test/ModelBinding/PrefixContainerTest.cs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/PrefixContainerTest.cs b/src/Mvc/Mvc.Core/test/ModelBinding/PrefixContainerTest.cs index 21fdd6c446ce..d42cd384cec5 100644 --- a/src/Mvc/Mvc.Core/test/ModelBinding/PrefixContainerTest.cs +++ b/src/Mvc/Mvc.Core/test/ModelBinding/PrefixContainerTest.cs @@ -180,6 +180,23 @@ public void ContainsPrefix_HasEntries_PartialAndPrefixMatch_WithSquareBrace(int Assert.True(result); } + [Theory] + [InlineData("parameter")] + [InlineData("foo")] + [InlineData("bar")] + public void ContainsPrefix_ReturnFalse_WhenPrefixMatchesExactly_WithKeys(string prefix) + { + // Arrange + var keys = new string[] { "parameter", "foo", "bar" }; + var container = new PrefixContainer(keys); + + // Act + var result = container.ContainsPrefix(prefix); + + // Assert + Assert.False(result); + } + [Theory] [InlineData("")] [InlineData("foo")] From 34a42e473db07ece78dde3847d5b147ccea57687 Mon Sep 17 00:00:00 2001 From: SAMI_DANIEL_SANTOS_SILVA <22201491@ALFAF52.FLORESTA> Date: Wed, 25 Sep 2024 06:44:05 -0300 Subject: [PATCH 2/4] Fix BinarySearch logic Updated the BinarySearch method in PrefixContainer to ensure that an exact match of a prefix does not incorrectly qualify as a valid prefix. Enhanced comments to clarify the flow and decision-making process when checking for delimiters. This change improves the accuracy of prefix matching for hierarchical key structures. --- .../src/ModelBinding/PrefixContainer.cs | 54 +++++++++++-------- 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/PrefixContainer.cs b/src/Mvc/Mvc.Core/src/ModelBinding/PrefixContainer.cs index aeab151eb80f..b81eaf818a1a 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/PrefixContainer.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/PrefixContainer.cs @@ -201,32 +201,40 @@ private int BinarySearch(string prefix) { Debug.Assert(candidate.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)); - // Okay, now we have a candidate that starts with the prefix. If the candidate is longer than - // the prefix, we need to look at the next character and see if it's a delimiter. - if (candidate.Length == prefix.Length) - { - // Exact match - return pivot; - } - - var c = candidate[prefix.Length]; - if (c == '.' || c == '[') + // At this point, we have identified a candidate that starts with the given prefix. + // If the candidate's length is greater than that of the prefix, we need to examine + // the character that immediately follows the prefix in the candidate string. + // This step is crucial to determine if the candidate is a valid prefix match. + // A valid prefix match occurs if the next character is either a delimiter ('.' or '['), + // indicating that the candidate is a sub-key of the prefix. If the next character + // is not a delimiter, it means the candidate contains additional characters that + // extend beyond the prefix without forming a valid hierarchical relationship, + // which should not qualify as a prefix match. Therefore, we will continue searching + // for valid matches in this case. + + if (candidate.Length > prefix.Length) { - // Match, followed by delimiter - return pivot; + var c = candidate[prefix.Length]; + if (c == '.' || c == '[') + { + // Match, followed by delimiter + return pivot; + } + + // Okay, so the candidate has some extra text. We need to keep searching. + // + // Can often assume the candidate string is greater than the prefix e.g. that works for + // prefix: product + // candidate: productId + // most of the time because "product", "product.id", etc. will sort earlier than "productId". But, + // the assumption isn't correct if "product[0]" is also in _sortedValues because that value will + // sort later than "productId". + // + // Fall back to brute force and cover all the cases. + return LinearSearch(prefix, start, end); } - // Okay, so the candidate has some extra text. We need to keep searching. - // - // Can often assume the candidate string is greater than the prefix e.g. that works for - // prefix: product - // candidate: productId - // most of the time because "product", "product.id", etc. will sort earlier than "productId". But, - // the assumption isn't correct if "product[0]" is also in _sortedValues because that value will - // sort later than "productId". - // - // Fall back to brute force and cover all the cases. - return LinearSearch(prefix, start, end); + return -1; } if (compare > 0) From 324652bad5f97fdebc55fa4be083d28348b53648 Mon Sep 17 00:00:00 2001 From: SAMI_DANIEL_SANTOS_SILVA <22201491@ALFAF52.FLORESTA> Date: Wed, 25 Sep 2024 06:48:44 -0300 Subject: [PATCH 3/4] =?UTF-8?q?Fix=20the=20name=20of=20the=20test=20that?= =?UTF-8?q?=20checks=20the=20Return=20False=20of=20ContaisPrefix=20if=20th?= =?UTF-8?q?e=20Prefix=20is=20=E2=80=8B=E2=80=8Bequal=20to=20the=20keys?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Mvc/Mvc.Core/test/ModelBinding/PrefixContainerTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/PrefixContainerTest.cs b/src/Mvc/Mvc.Core/test/ModelBinding/PrefixContainerTest.cs index d42cd384cec5..e8679baf72e1 100644 --- a/src/Mvc/Mvc.Core/test/ModelBinding/PrefixContainerTest.cs +++ b/src/Mvc/Mvc.Core/test/ModelBinding/PrefixContainerTest.cs @@ -184,7 +184,7 @@ public void ContainsPrefix_HasEntries_PartialAndPrefixMatch_WithSquareBrace(int [InlineData("parameter")] [InlineData("foo")] [InlineData("bar")] - public void ContainsPrefix_ReturnFalse_WhenPrefixMatchesExactly_WithKeys(string prefix) + public void ContainsPrefix_ReturnsFalse_WhenPrefixMatchesExactly_WithKeys(string prefix) { // Arrange var keys = new string[] { "parameter", "foo", "bar" }; From eb68105298e90c4e6912b1b841c3edc858e6614c Mon Sep 17 00:00:00 2001 From: Sami Daniel <130937402+sami-daniel@users.noreply.github.com> Date: Wed, 25 Sep 2024 20:30:33 -0300 Subject: [PATCH 4/4] Optimize Boundary Check in BinarySearch Method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit enhances the performance of the BinarySearch method by eliminating unnecessary boundary checks. The condition if ((uint)candidate.Length > (uint)prefix.Length) ensures that the index access candidate[prefix.Length] is safe, and the JIT compiler can now safely elide the boundary check, improving execution efficiency. Co-authored-by: Günther Foidl --- src/Mvc/Mvc.Core/src/ModelBinding/PrefixContainer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/PrefixContainer.cs b/src/Mvc/Mvc.Core/src/ModelBinding/PrefixContainer.cs index b81eaf818a1a..a158522c092f 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/PrefixContainer.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/PrefixContainer.cs @@ -212,7 +212,7 @@ private int BinarySearch(string prefix) // which should not qualify as a prefix match. Therefore, we will continue searching // for valid matches in this case. - if (candidate.Length > prefix.Length) + if ((uint)candidate.Length > (uint)prefix.Length) { var c = candidate[prefix.Length]; if (c == '.' || c == '[')