-
Notifications
You must be signed in to change notification settings - Fork 332
Fix: URL-encoded special characters in $filter and $orderby break OData parsing #3080
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 6 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
855efed
Initial plan
Copilot 8b01458
Implement fix for special characters in filter clause
Copilot 8f1c4c0
Add test data and test case for filter with special characters
Copilot f2d8fbd
Add unit tests for RequestParser URL encoding preservation
Copilot 55a1f98
Improve ExtractRawQueryParameter with better edge case handling and d…
Copilot 7a8f2aa
Make code more terse and concise
Copilot 029d51b
Merge branch 'main' into copilot/fix-filter-clause-issue
JerryNixon cffedaa
Address PR feedback: Add null checks, more special character tests, a…
Copilot e754517
Fix critical issues: URL-encode integration test queries, improve met…
Copilot 00df410
Merge branch 'main' into copilot/fix-filter-clause-issue
JerryNixon b4cd2ba
Merge branch 'main' into copilot/fix-filter-clause-issue
Aniruddh25 4f43f5d
Add test for $orderby with special characters (URL-encoded)
Copilot 1c39d43
Merge branch 'main' into copilot/fix-filter-clause-issue
Aniruddh25 a78b348
Fix dotnet format errors: Add blank lines and braces per coding stand…
Copilot 4c5ed20
Remove added test data to avoid breaking existing tests - use existin…
Copilot 89b52af
Remove FindTestWithOrderByContainingSpecialCharacters integration tes…
Copilot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT License. | ||
|
|
||
| using System.Reflection; | ||
| using Azure.DataApiBuilder.Core.Parsers; | ||
| using Microsoft.VisualStudio.TestTools.UnitTesting; | ||
|
|
||
| namespace Azure.DataApiBuilder.Service.Tests.UnitTests | ||
| { | ||
| /// <summary> | ||
| /// Test class for RequestParser utility methods. | ||
| /// Specifically tests the ExtractRawQueryParameter method which preserves | ||
| /// URL encoding for special characters in query parameters. | ||
| /// </summary> | ||
| [TestClass] | ||
| public class RequestParserUnitTests | ||
| { | ||
| /// <summary> | ||
| /// Tests that ExtractRawQueryParameter correctly extracts URL-encoded | ||
| /// parameter values, preserving special characters like ampersand (&). | ||
| /// </summary> | ||
| [DataTestMethod] | ||
| [DataRow("?$filter=region%20eq%20%27filter%20%26%20test%27", "$filter", "region%20eq%20%27filter%20%26%20test%27", DisplayName = "Extract filter with encoded ampersand")] | ||
|
JerryNixon marked this conversation as resolved.
Outdated
|
||
| [DataRow("?$filter=title%20eq%20%27A%20%26%20B%27&$select=id", "$filter", "title%20eq%20%27A%20%26%20B%27", DisplayName = "Extract filter with ampersand and other params")] | ||
| [DataRow("?$select=id&$filter=name%20eq%20%27test%27", "$filter", "name%20eq%20%27test%27", DisplayName = "Extract filter when not first parameter")] | ||
| [DataRow("?$orderby=name%20asc", "$orderby", "name%20asc", DisplayName = "Extract orderby parameter")] | ||
| [DataRow("?param1=value1¶m2=value%26with%26ampersands", "param2", "value%26with%26ampersands", DisplayName = "Extract parameter with multiple ampersands")] | ||
| [DataRow("$filter=title%20eq%20%27test%27", "$filter", "title%20eq%20%27test%27", DisplayName = "Extract without leading question mark")] | ||
| [DataRow("?$filter=", "$filter", "", DisplayName = "Extract empty filter value")] | ||
| public void ExtractRawQueryParameter_PreservesEncoding(string queryString, string parameterName, string expectedValue) | ||
| { | ||
| // Use reflection to call the private static method | ||
| MethodInfo? method = typeof(RequestParser).GetMethod( | ||
| "ExtractRawQueryParameter", | ||
| BindingFlags.NonPublic | BindingFlags.Static); | ||
|
|
||
| Assert.IsNotNull(method, "ExtractRawQueryParameter method should exist"); | ||
|
|
||
| string? result = (string?)method.Invoke(null, new object[] { queryString, parameterName }); | ||
|
|
||
| Assert.AreEqual(expectedValue, result, | ||
| $"Expected '{expectedValue}' but got '{result}' for parameter '{parameterName}' in query '{queryString}'"); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Tests that ExtractRawQueryParameter returns null when parameter is not found. | ||
| /// </summary> | ||
| [DataTestMethod] | ||
| [DataRow("?$filter=test", "$orderby", DisplayName = "Parameter not in query string")] | ||
| [DataRow("", "$filter", DisplayName = "Empty query string")] | ||
| [DataRow(null, "$filter", DisplayName = "Null query string")] | ||
| [DataRow("?otherParam=value", "$filter", DisplayName = "Different parameter")] | ||
| public void ExtractRawQueryParameter_ReturnsNull_WhenParameterNotFound(string? queryString, string parameterName) | ||
| { | ||
| // Use reflection to call the private static method | ||
| MethodInfo? method = typeof(RequestParser).GetMethod( | ||
| "ExtractRawQueryParameter", | ||
| BindingFlags.NonPublic | BindingFlags.Static); | ||
|
|
||
| Assert.IsNotNull(method, "ExtractRawQueryParameter method should exist"); | ||
|
|
||
| string? result = (string?)method.Invoke(null, new object?[] { queryString, parameterName }); | ||
|
|
||
| Assert.IsNull(result, | ||
| $"Expected null but got '{result}' for parameter '{parameterName}' in query '{queryString}'"); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Tests that ExtractRawQueryParameter handles edge cases correctly. | ||
|
JerryNixon marked this conversation as resolved.
Outdated
|
||
| /// </summary> | ||
| [DataTestMethod] | ||
| [DataRow("?$filter=value&$filter=anothervalue", "$filter", "value", DisplayName = "Multiple same parameters - returns first")] | ||
| [DataRow("?$FILTER=value", "$filter", "value", DisplayName = "Case insensitive parameter matching")] | ||
| [DataRow("?param=value1&value2", "param", "value1", DisplayName = "Value with unencoded ampersand after parameter")] | ||
| public void ExtractRawQueryParameter_HandlesEdgeCases(string queryString, string parameterName, string expectedValue) | ||
| { | ||
| // Use reflection to call the private static method | ||
| MethodInfo? method = typeof(RequestParser).GetMethod( | ||
| "ExtractRawQueryParameter", | ||
| BindingFlags.NonPublic | BindingFlags.Static); | ||
|
|
||
| Assert.IsNotNull(method, "ExtractRawQueryParameter method should exist"); | ||
|
|
||
| string? result = (string?)method.Invoke(null, new object[] { queryString, parameterName }); | ||
|
|
||
| Assert.AreEqual(expectedValue, result, | ||
| $"Expected '{expectedValue}' but got '{result}' for parameter '{parameterName}' in query '{queryString}'"); | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.