diff --git a/src/Core/Models/RestRequestContexts/RestRequestContext.cs b/src/Core/Models/RestRequestContexts/RestRequestContext.cs index 70d6a371b5..e9987730a0 100644 --- a/src/Core/Models/RestRequestContexts/RestRequestContext.cs +++ b/src/Core/Models/RestRequestContexts/RestRequestContext.cs @@ -77,6 +77,12 @@ protected RestRequestContext(string entityName, DatabaseObject dbo) /// public NameValueCollection ParsedQueryString { get; set; } = new(); + /// + /// Raw query string from the HTTP request (URL-encoded). + /// Used to preserve encoding for special characters in query parameters. + /// + public string RawQueryString { get; set; } = string.Empty; + /// /// String holds information needed for pagination. /// Based on request this property may or may not be populated. diff --git a/src/Core/Parsers/RequestParser.cs b/src/Core/Parsers/RequestParser.cs index 6402ce4ecb..1ed1807e16 100644 --- a/src/Core/Parsers/RequestParser.cs +++ b/src/Core/Parsers/RequestParser.cs @@ -113,14 +113,16 @@ public static void ParseQueryString(RestRequestContext context, ISqlMetadataProv context.FieldsToBeReturned = context.ParsedQueryString[key]!.Split(",").ToList(); break; case FILTER_URL: - // save the AST that represents the filter for the query - // ?$filter= - string filterQueryString = $"?{FILTER_URL}={context.ParsedQueryString[key]}"; - context.FilterClauseInUrl = sqlMetadataProvider.GetODataParser().GetFilterClause(filterQueryString, $"{context.EntityName}.{context.DatabaseObject.FullName}"); + // Use raw (URL-encoded) filter value to preserve special characters like & + string? rawFilterValue = ExtractRawQueryParameter(context.RawQueryString, FILTER_URL); + if (rawFilterValue is not null) + context.FilterClauseInUrl = sqlMetadataProvider.GetODataParser().GetFilterClause($"?{FILTER_URL}={rawFilterValue}", $"{context.EntityName}.{context.DatabaseObject.FullName}"); break; case SORT_URL: - string sortQueryString = $"?{SORT_URL}={context.ParsedQueryString[key]}"; - (context.OrderByClauseInUrl, context.OrderByClauseOfBackingColumns) = GenerateOrderByLists(context, sqlMetadataProvider, sortQueryString); + // Use raw (URL-encoded) orderby value to preserve special characters + string? rawSortValue = ExtractRawQueryParameter(context.RawQueryString, SORT_URL); + if (rawSortValue is not null) + (context.OrderByClauseInUrl, context.OrderByClauseOfBackingColumns) = GenerateOrderByLists(context, sqlMetadataProvider, $"?{SORT_URL}={rawSortValue}"); break; case AFTER_URL: context.After = context.ParsedQueryString[key]; @@ -283,5 +285,22 @@ private static bool IsNull(string value) { return string.IsNullOrWhiteSpace(value) || string.Equals(value, "null", StringComparison.OrdinalIgnoreCase); } + + /// + /// Extracts the raw (URL-encoded) value of a query parameter from a query string. + /// Preserves special characters like & in filter values (e.g., %26 stays as %26). + /// + private static string? ExtractRawQueryParameter(string queryString, string parameterName) + { + if (string.IsNullOrWhiteSpace(queryString)) return null; + + foreach (string param in queryString.TrimStart('?').Split('&')) + { + int idx = param.IndexOf('='); + if (idx >= 0 && param.Substring(0, idx).Equals(parameterName, StringComparison.OrdinalIgnoreCase)) + return idx < param.Length - 1 ? param.Substring(idx + 1) : string.Empty; + } + return null; + } } } diff --git a/src/Core/Services/RestService.cs b/src/Core/Services/RestService.cs index 6a2308dd83..a338f9ce3f 100644 --- a/src/Core/Services/RestService.cs +++ b/src/Core/Services/RestService.cs @@ -174,6 +174,7 @@ RequestValidator requestValidator if (!string.IsNullOrWhiteSpace(queryString)) { + context.RawQueryString = queryString; context.ParsedQueryString = HttpUtility.ParseQueryString(queryString); RequestParser.ParseQueryString(context, sqlMetadataProvider); } @@ -277,6 +278,7 @@ private void PopulateStoredProcedureContext( // So, $filter will be treated as any other parameter (inevitably will raise a Bad Request) if (!string.IsNullOrWhiteSpace(queryString)) { + context.RawQueryString = queryString; context.ParsedQueryString = HttpUtility.ParseQueryString(queryString); } diff --git a/src/Service.Tests/DatabaseSchema-DwSql.sql b/src/Service.Tests/DatabaseSchema-DwSql.sql index daed665949..913f4fd9a8 100644 --- a/src/Service.Tests/DatabaseSchema-DwSql.sql +++ b/src/Service.Tests/DatabaseSchema-DwSql.sql @@ -337,7 +337,8 @@ VALUES (1, 'Awesome book', 1234), (18, '[Special Book]', 1234), (19, 'ME\YOU', 1234), (20, 'C:\\LIFE', 1234), -(21, '', 1234); +(21, '', 1234), +(22, 'filter & test', 1234); INSERT INTO book_website_placements(id, book_id, price) VALUES (1, 1, 100), (2, 2, 50), (3, 3, 23), (4, 5, 33); diff --git a/src/Service.Tests/DatabaseSchema-MsSql.sql b/src/Service.Tests/DatabaseSchema-MsSql.sql index 4e87394aee..1228071a1c 100644 --- a/src/Service.Tests/DatabaseSchema-MsSql.sql +++ b/src/Service.Tests/DatabaseSchema-MsSql.sql @@ -532,7 +532,8 @@ VALUES (1, 'Awesome book', 1234), (18, '[Special Book]', 1234), (19, 'ME\YOU', 1234), (20, 'C:\\LIFE', 1234), -(21, '', 1234); +(21, '', 1234), +(22, 'filter & test', 1234); SET IDENTITY_INSERT books OFF SET IDENTITY_INSERT books_mm ON diff --git a/src/Service.Tests/DatabaseSchema-MySql.sql b/src/Service.Tests/DatabaseSchema-MySql.sql index dda93d86d1..3db6a5db98 100644 --- a/src/Service.Tests/DatabaseSchema-MySql.sql +++ b/src/Service.Tests/DatabaseSchema-MySql.sql @@ -389,7 +389,8 @@ INSERT INTO books(id, title, publisher_id) (18, '[Special Book]', 1234), (19, 'ME\\YOU', 1234), (20, 'C:\\\\LIFE', 1234), - (21, '', 1234); + (21, '', 1234), + (22, 'filter & test', 1234); INSERT INTO book_website_placements(book_id, price) VALUES (1, 100), (2, 50), (3, 23), (5, 33); INSERT INTO website_users(id, username) VALUES (1, 'George'), (2, NULL), (3, ''), (4, 'book_lover_95'), (5, 'null'); INSERT INTO book_author_link(book_id, author_id) VALUES (1, 123), (2, 124), (3, 123), (3, 124), (4, 123), (4, 124), (5, 126); diff --git a/src/Service.Tests/DatabaseSchema-PostgreSql.sql b/src/Service.Tests/DatabaseSchema-PostgreSql.sql index 523e96c22f..b3dcab8fa0 100644 --- a/src/Service.Tests/DatabaseSchema-PostgreSql.sql +++ b/src/Service.Tests/DatabaseSchema-PostgreSql.sql @@ -392,7 +392,8 @@ INSERT INTO books(id, title, publisher_id) (18, '[Special Book]', 1234), (19, 'ME\YOU', 1234), (20, 'C:\\LIFE', 1234), - (21, '', 1234); + (21, '', 1234), + (22, 'filter & test', 1234); INSERT INTO book_website_placements(book_id, price) VALUES (1, 100), (2, 50), (3, 23), (5, 33); INSERT INTO website_users(id, username) VALUES (1, 'George'), (2, NULL), (3, ''), (4, 'book_lover_95'), (5, 'null'); INSERT INTO book_author_link(book_id, author_id) VALUES (1, 123), (2, 124), (3, 123), (3, 124), (4, 123), (4, 124), (5, 126);; diff --git a/src/Service.Tests/SqlTests/RestApiTests/Find/DwSqlFindApiTests.cs b/src/Service.Tests/SqlTests/RestApiTests/Find/DwSqlFindApiTests.cs index 8c78a27061..503229bf3c 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Find/DwSqlFindApiTests.cs +++ b/src/Service.Tests/SqlTests/RestApiTests/Find/DwSqlFindApiTests.cs @@ -221,6 +221,12 @@ public class DwSqlFindApiTests : FindApiTestBase $"WHERE (NOT (id < 3) OR id < 4) OR NOT (title = 'Awesome book') " + $"FOR JSON PATH, INCLUDE_NULL_VALUES" }, + { + "FindTestWithFilterContainingSpecialCharacters", + $"SELECT * FROM { _integrationTableName } " + + $"WHERE title = 'filter & test' " + + $"FOR JSON PATH, INCLUDE_NULL_VALUES" + }, { "FindTestWithPrimaryKeyContainingForeignKey", $"SELECT [id], [content] FROM reviews " + diff --git a/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs b/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs index 483d870d85..e8a1c17a5f 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs +++ b/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs @@ -693,6 +693,22 @@ await SetupAndRunRestApiTest( ); } + /// + /// Tests the REST Api for Find operation with a filter containing special characters + /// like ampersand (&) that need to be URL-encoded. This validates that the fix for + /// the double-decoding issue is working correctly. + /// + [TestMethod] + public async Task FindTestWithFilterContainingSpecialCharacters() + { + await SetupAndRunRestApiTest( + primaryKeyRoute: string.Empty, + queryString: "?$filter=title eq 'filter & test'", + entityNameOrPath: _integrationEntityName, + sqlQuery: GetQuery(nameof(FindTestWithFilterContainingSpecialCharacters)) + ); + } + /// /// Tests the REST Api for Find operation where we compare one field /// to the bool returned from another comparison. diff --git a/src/Service.Tests/SqlTests/RestApiTests/Find/MsSqlFindApiTests.cs b/src/Service.Tests/SqlTests/RestApiTests/Find/MsSqlFindApiTests.cs index 6f43fb2073..d93ebacd43 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Find/MsSqlFindApiTests.cs +++ b/src/Service.Tests/SqlTests/RestApiTests/Find/MsSqlFindApiTests.cs @@ -228,6 +228,12 @@ public class MsSqlFindApiTests : FindApiTestBase $"WHERE (NOT (id < 3) OR id < 4) OR NOT (title = 'Awesome book') " + $"FOR JSON PATH, INCLUDE_NULL_VALUES" }, + { + "FindTestWithFilterContainingSpecialCharacters", + $"SELECT * FROM { _integrationTableName } " + + $"WHERE title = 'filter & test' " + + $"FOR JSON PATH, INCLUDE_NULL_VALUES" + }, { "FindTestWithPrimaryKeyContainingForeignKey", $"SELECT [id], [content] FROM reviews " + diff --git a/src/Service.Tests/SqlTests/RestApiTests/Find/MySqlFindApiTests.cs b/src/Service.Tests/SqlTests/RestApiTests/Find/MySqlFindApiTests.cs index f9a3fdb764..9914beec16 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Find/MySqlFindApiTests.cs +++ b/src/Service.Tests/SqlTests/RestApiTests/Find/MySqlFindApiTests.cs @@ -397,6 +397,18 @@ ORDER BY id asc ) AS subq " }, + { + "FindTestWithFilterContainingSpecialCharacters", + @" + SELECT JSON_ARRAYAGG(JSON_OBJECT('id', id, 'title', title, 'publisher_id', publisher_id)) AS data + FROM ( + SELECT * + FROM " + _integrationTableName + @" + WHERE title = 'filter & test' + ORDER BY id asc + ) AS subq + " + }, { "FindTestWithFilterQueryStringBoolResultFilter", @" diff --git a/src/Service.Tests/SqlTests/RestApiTests/Find/PostgreSqlFindApiTests.cs b/src/Service.Tests/SqlTests/RestApiTests/Find/PostgreSqlFindApiTests.cs index 9abcfe88c2..7bed112983 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Find/PostgreSqlFindApiTests.cs +++ b/src/Service.Tests/SqlTests/RestApiTests/Find/PostgreSqlFindApiTests.cs @@ -411,6 +411,17 @@ SELECT json_agg(to_jsonb(subq)) AS data ORDER BY id asc ) AS subq" }, + { + "FindTestWithFilterContainingSpecialCharacters", + @" + SELECT json_agg(to_jsonb(subq)) AS data + FROM ( + SELECT * + FROM " + _integrationTableName + @" + WHERE title = 'filter & test' + ORDER BY id asc + ) AS subq" + }, { "FindTestWithPrimaryKeyContainingForeignKey", @" diff --git a/src/Service.Tests/UnitTests/RequestParserUnitTests.cs b/src/Service.Tests/UnitTests/RequestParserUnitTests.cs new file mode 100644 index 0000000000..82adc885d3 --- /dev/null +++ b/src/Service.Tests/UnitTests/RequestParserUnitTests.cs @@ -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 +{ + /// + /// Test class for RequestParser utility methods. + /// Specifically tests the ExtractRawQueryParameter method which preserves + /// URL encoding for special characters in query parameters. + /// + [TestClass] + public class RequestParserUnitTests + { + /// + /// Tests that ExtractRawQueryParameter correctly extracts URL-encoded + /// parameter values, preserving special characters like ampersand (&). + /// + [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")] + [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}'"); + } + + /// + /// Tests that ExtractRawQueryParameter returns null when parameter is not found. + /// + [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}'"); + } + + /// + /// Tests that ExtractRawQueryParameter handles edge cases correctly. + /// + [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}'"); + } + } +}