Skip to content
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.

Sign up for GitHub

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

Fix ArgumentNullException for: $filter=property in [''] #3122

Merged
merged 5 commits into from
Dec 10, 2024
Merged

Fix ArgumentNullException for: $filter=property in [''] #3122

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e7c8cb2
Add Square Bracket when constructing collectionNode. Also add some tests
WanjohiSammy Nov 15, 2024
f4caa0c
Added E2E tests
WanjohiSammy Nov 15, 2024
b75161f
Add more test cases and refactor Assert
WanjohiSammy Nov 18, 2024
cae19e1
Use string create instead of allocate extra memory
WanjohiSammy Nov 25, 2024
e140298
Remove unnecessary variables
WanjohiSammy Nov 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 17 additions & 15 deletions src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,21 +97,23 @@ private CollectionNode GetCollectionOperandFromToken(QueryToken queryToken, IEdm
LiteralToken literalToken = queryToken as LiteralToken;
if (literalToken != null)
{
string originalLiteralText = literalToken.OriginalText;

// Parentheses-based collections are not standard JSON but bracket-based ones are.
// Temporarily switch our collection to bracket-based so that the JSON reader will
// correctly parse the collection. Then pass the original literal text to the token.
string bracketLiteralText = originalLiteralText;
if (bracketLiteralText[0] == '(')
ReadOnlySpan<char> bracketLiteralText = literalToken.OriginalText.AsSpan();

if (bracketLiteralText[0] == '(' || bracketLiteralText[0] == '[')
{
Debug.Assert(bracketLiteralText[bracketLiteralText.Length - 1] == ')',
"Collection with opening '(' should have corresponding ')'");
Debug.Assert((bracketLiteralText[0] == '(' && bracketLiteralText[^1] == ')') || (bracketLiteralText[0] == '[' && bracketLiteralText[^1] == ']'),
WanjohiSammy marked this conversation as resolved.
Show resolved Hide resolved
$"Collection with opening '{bracketLiteralText[0]}' should have corresponding '{bracketLiteralText[^1]}'");

StringBuilder replacedText = new StringBuilder(bracketLiteralText);
replacedText[0] = '[';
replacedText[replacedText.Length - 1] = ']';
bracketLiteralText = replacedText.ToString();
if(bracketLiteralText[0] == '(' && bracketLiteralText[^1] == ')')
{
Span<char> replacedText = new Span<char>(bracketLiteralText.ToArray());
WanjohiSammy marked this conversation as resolved.
Show resolved Hide resolved
replacedText[0] = '[';
replacedText[replacedText.Length - 1] = ']';
bracketLiteralText = replacedText;
}

Debug.Assert(expectedType.IsCollection());
string expectedTypeFullName = expectedType.Definition.AsElementType().FullTypeName();
Expand All @@ -122,15 +124,15 @@ private CollectionNode GetCollectionOperandFromToken(QueryToken queryToken, IEdm
// and also, per ABNF, a single quote within a string literal is "encoded" as two consecutive single quotes in either
// literal or percent - encoded representation.
// Sample: ['a''bc','''def','xyz'''] ==> ["a'bc","'def","xyz'"], which is legitimate Json format.
bracketLiteralText = NormalizeStringCollectionItems(bracketLiteralText);
bracketLiteralText = NormalizeStringCollectionItems(bracketLiteralText.ToString());
WanjohiSammy marked this conversation as resolved.
Show resolved Hide resolved
}
else if (expectedTypeFullName.Equals("Edm.Guid", StringComparison.Ordinal))
{
// For collection of Guids, need to convert the Guid literals to single-quoted form, so that it is compatible
// with the Json reader used for deserialization.
// Sample: [D01663CF-EB21-4A0E-88E0-361C10ACE7FD, 492CF54A-84C9-490C-A7A4-B5010FAD8104]
// ==> ['D01663CF-EB21-4A0E-88E0-361C10ACE7FD', '492CF54A-84C9-490C-A7A4-B5010FAD8104']
bracketLiteralText = NormalizeGuidCollectionItems(bracketLiteralText);
bracketLiteralText = NormalizeGuidCollectionItems(bracketLiteralText.ToString());
WanjohiSammy marked this conversation as resolved.
Show resolved Hide resolved
}
else if (expectedTypeFullName.Equals("Edm.DateTimeOffset", StringComparison.Ordinal) ||
expectedTypeFullName.Equals("Edm.Date", StringComparison.Ordinal) ||
Expand All @@ -141,12 +143,12 @@ private CollectionNode GetCollectionOperandFromToken(QueryToken queryToken, IEdm
// with the Json reader used for deserialization.
// Sample: [1970-01-01T00:00:00Z, 1980-01-01T01:01:01+01:00]
// ==> ['1970-01-01T00:00:00Z', '1980-01-01T01:01:01+01:00']
bracketLiteralText = NormalizeDateTimeCollectionItems(bracketLiteralText);
bracketLiteralText = NormalizeDateTimeCollectionItems(bracketLiteralText.ToString());
WanjohiSammy marked this conversation as resolved.
Show resolved Hide resolved
}
}

object collection = ODataUriConversionUtils.ConvertFromCollectionValue(bracketLiteralText, model, expectedType);
LiteralToken collectionLiteralToken = new LiteralToken(collection, originalLiteralText, expectedType);
object collection = ODataUriConversionUtils.ConvertFromCollectionValue(bracketLiteralText.ToString(), model, expectedType);
WanjohiSammy marked this conversation as resolved.
Show resolved Hide resolved
LiteralToken collectionLiteralToken = new LiteralToken(collection, literalToken.OriginalText, expectedType);
operand = this.bindMethod(collectionLiteralToken) as CollectionConstantNode;
}
else
Expand Down
31 changes: 31 additions & 0 deletions ...Tests/Tests/Client/Microsoft.OData.Client.E2E.Tests/ClientTests/Tests/ClientQueryTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,37 @@ public async Task DollarFilter_UsingContains_ExecutesSuccessfully(string query,
Assert.Equal(expectedCount, result.Length);
}

[Theory]
[InlineData("People?$filter=Name in ('')")]
[InlineData("People?$filter=Name in ['']")]
[InlineData("People?$filter=Name in ( '' )")]
[InlineData("People?$filter=Name in [ '' ]")]
[InlineData("People?$filter=Name in (\"\")")]
[InlineData("People?$filter=Name in [\"\"]")]
[InlineData("People?$filter=Name in ( \"\" )")]
[InlineData("People?$filter=Name in [ \"\" ]")]
[InlineData("People?$filter=Name in ( ' ' )")]
[InlineData("People?$filter=Name in [ ' ' ]")]
[InlineData("People?$filter=Name in ( \" \" )")]
[InlineData("People?$filter=Name in [ \" \"]")]
[InlineData("People?$filter=Name in ( '', ' ' )")]
[InlineData("People?$filter=Name in [ '', ' ' ]")]
[InlineData("People?$filter=Name in ( \"\", \" \" )")]
[InlineData("People?$filter=Name in [ \"\", \" \" ]")]
[InlineData("People?$filter=Name in ( '', \" \" )")]
[InlineData("People?$filter=Name in [ '', \" \" ]")]
[InlineData("People?$filter=Name in ( \"\", ' ' )")]
[InlineData("People?$filter=Name in [ \"\", ' ' ]")]
[InlineData("People?$filter=Name in [ 'null', 'null' ]")]
public async Task DollarFilter_WithCollectionWithEmptyString_ExecutesSuccessfully(string query)
{
// Act
var response = await _context.ExecuteAsync<Common.Clients.EndToEnd.Person>(new Uri(_baseUri.OriginalString + query));

// Assert
Assert.Empty(response.ToArray());
}

[Fact]
public async Task Using_LinqContains_ExecutesSuccessfully()
{
Expand Down
10 changes: 10 additions & 0 deletions ...Tests/Microsoft.OData.Core.Tests/ScenarioTests/UriBuilder/FilterAndOrderByBuilderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,16 @@ public void BuildFilterWithInOperatorUsingBracketedCollectionConstant()
Assert.Equal(new Uri("http://gobbledygook/People?$filter=ID%20in%20[1%2C2%2C3]"), actualUri, new UriComparer<Uri>());
}

[Theory]
[InlineData("People?$filter=Name in ('')", "http://gobbledygook/People?$filter=Name%20in%20(%27%27)")]
[InlineData("People?$filter=Name in ['']", "http://gobbledygook/People?$filter=Name%20in%20[%27%27]")]
public void BuildFilterWithInOperatorUsingCollectionWithEmptyString(string filterQuery, string expectedQuery)
{
Uri queryUri = new Uri(filterQuery, UriKind.Relative);
Uri actualUri = UriBuilder(queryUri, ODataUrlKeyDelimiter.Parentheses, settings);
Assert.Equal(new Uri(expectedQuery), actualUri, new UriComparer<Uri>());
}

[Fact]
public void BuildFilterWithInOperatorUsingSingleConstant()
{
Expand Down
12 changes: 8 additions & 4 deletions ...onalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriBuilder/SelectExpandBuilderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -668,12 +668,16 @@ public void ExpandWithDollarItInFilterComplexBinaryOperatorShouldWork()
Assert.Equal("http://gobbledygook/People?$expand=" + Uri.EscapeDataString("MyDog($filter=$it/MyAddress/City eq 'Seattle')"), actualUri.OriginalString);
}

[Fact]
public void ExpandWithDollarItInFilterInOperatorShouldWork()
[Theory]
[InlineData("People?$expand=MyDog($filter=$it/ID in ['1', '2', '3'])", "MyDog($filter=$it/ID in ['1', '2', '3'])")]
[InlineData("People?$expand=MyDog($filter=$it/ID in ('1', '2', '3'))", "MyDog($filter=$it/ID in ('1', '2', '3'))")]
[InlineData("People?$expand=MyDog($filter=$it/Name in (''))", "MyDog($filter=$it/Name in (''))")]
[InlineData("People?$expand=MyDog($filter=$it/Name in [''])", "MyDog($filter=$it/Name in [''])")]
public void ExpandWithDollarItInFilterInOperatorShouldWork(string filterQuery, string expectedSubQuery)
{
Uri queryUri = new Uri("People?$expand=MyDog($filter=$it/ID in ['1', '2', '3'])", UriKind.Relative);
Uri queryUri = new Uri(filterQuery, UriKind.Relative);
Uri actualUri = UriBuilder(queryUri, ODataUrlKeyDelimiter.Parentheses, settings);
Assert.Equal("http://gobbledygook/People?$expand=" + Uri.EscapeDataString("MyDog($filter=$it/ID in ['1', '2', '3'])"), actualUri.OriginalString);
Assert.Equal("http://gobbledygook/People?$expand=" + Uri.EscapeDataString(expectedSubQuery), actualUri.OriginalString);
}

[Fact]
Expand Down
Loading