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

Jump to bottom

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

Merged
merged 5 commits into from
Dec 10, 2024
Merged

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

merged 5 commits into from
Dec 10, 2024
+170 −16

Conversation

WanjohiSammy
Copy link
Contributor

@WanjohiSammy WanjohiSammy commented Nov 15, 2024 •
edited
Loading

Issues

This pull request fixes #3092

Description

This PR addresses an issue where the OData URI parser throws an ArgumentNullException when encountering a list with empty string in square brackets.

For example, the following query will throw an ArgumentNullException:

  • /People?$filter=Name in ['']
  • /People?$filter=Name in [ ' ' ]
  • /People?$filter=Name in [ \"\", ' ' ]
  • /People?$filter=Name in [ \"\", '' ]
  • /People?$filter=Name in [\"\"]
  • /People?$filter=Name in [ \"\", \" \" ]
  • /People?$filter=Name in [ \"\" ]
  • /People?$filter=Name in [ '' , ' ' ]
System.InvalidOperationException : An error occurred while processing this request.
---- Microsoft.OData.Client.DataServiceTransportException : System.ArgumentNullException: Value cannot be null or empty. (Parameter 'literalText')
   at Microsoft.OData.ExceptionUtils.CheckArgumentStringNotNullOrEmpty(String value, String parameterName) in C:\Work\odata.net\src\Microsoft.OData.Core\ExceptionUtils.cs:line 101
   .....

Change

This change ensures the Empty String in square brackets are handle the same way as parentheses when constructing CollectionNode in GetCollectionOperandFromToken method. This method calls the following methods to handle Single and Double quotes:

  • ProcessSingleQuotedStringItem is called to handle single-quoted string items within a collection. This method converts single-quoted strings to double-quoted strings, ensuring compatibility with JSON format. It manages the escaping of single quotes by unescaping double single quotes and escaping double quotes. For example, it converts ''' to "" and 'ghi''' to "ghi'"
  • ProcessDoubleQuotedStringItem is then called to process double-quoted string items within a collection. This method ensures that the string is properly escaped for JSON format, handling the escaping of double quotes and backslashes. For example, it converts "" to "" to avoid passing an empty string to the ConstantNode.

Checklist (Uncheck if it is not completed)

  • Test cases added
  • Build and test with one-click build and test script passed

Additional work necessary

If documentation update is needed, please add "Docs Needed" label to the issue and provide details about the required document change in the issue.

@WanjohiSammy WanjohiSammy requested a review from xuzhg November 18, 2024 19:55
@gathogojr
Copy link
Contributor

@WanjohiSammy
This change ensures the square brackets are handle the same way as parentheses when constructing CollectionNode in GetCollectionOperandFromToken method:

  • Is use of square brackets in the same manner as parentheses supported by the OData standard?
  • Can you confirm that we support non-empty square brackets? For example, $filter=Name in ['Sue','Joe']. Maybe even add a test

@WanjohiSammy
Copy link
Contributor Author

@gathogojr

  • Is use of square brackets in the same manner as parentheses supported by the OData standard?
  • Can you confirm that we support non-empty square brackets? For example, $filter=Name in ['Sue','Joe']. Maybe even add a test

I have updated the PR description to reflect correctly that this was failing because the empty string in square brackets was not handle the same as empty string in parenthesis

@gathogojr
Copy link
Contributor

@WanjohiSammy Can you confirm what I asked here #3122 (comment)

@gathogojr
Copy link
Contributor

@WanjohiSammy I have confirmed that we support brackets, i.e., Books?$filter=Title in ('Harry Porter') and Books?$filter=Title in ['Harry Porter'] are both handled fine by the library

@WanjohiSammy
Copy link
Contributor Author

@gathogojr

Can you confirm what I asked here #3122 (comment)

I thought I confirmed here #3122 (comment)
The issue solved by this PR is when there is an empty string in square brackets.

-We do support square brackets but in case of empty string in these brackets an exception is thrown.
-Since there is no exception with Empty string in parenthesis, this change ensures empty string in square brackets are treated the same as empty string in parenthesis.

@WanjohiSammy WanjohiSammy requested a review from habbes November 25, 2024 12:54
@WanjohiSammy WanjohiSammy merged commit 5e715a2 into main Dec 10, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gathogojr gathogojr gathogojr approved these changes

@marabooy marabooy Awaiting requested review from marabooy

@DButoyez DButoyez Awaiting requested review from DButoyez

@xuzhg xuzhg Awaiting requested review from xuzhg

@habbes habbes Awaiting requested review from habbes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ArgumentNullException for: $filter=property in ['']
4 participants
@WanjohiSammy @gathogojr @habbes @xuzhg