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

Azure.Search: Verify escaping when splitting OData $select, etc. #10600

Closed
tg-msft opened this issue Mar 13, 2020 · 4 comments
Closed

Azure.Search: Verify escaping when splitting OData $select, etc. #10600

tg-msft opened this issue Mar 13, 2020 · 4 comments
Assignees
@tg-msft
Labels
blocking-release Blocks release Client This issue points to a problem in the data-plane of the library. Search
Milestone
[2020] August

Comments

@tg-msft
Copy link
Member

tg-msft commented Mar 13, 2020

We turn comma separated strings of OData values into collections and need to verify that we don't accidentally split a value that should be escaped.
@tg-msft tg-msft added Client This issue points to a problem in the data-plane of the library. Search labels Mar 13, 2020
@tg-msft tg-msft self-assigned this Mar 13, 2020
@tg-msft tg-msft added this to the [2020] April milestone Mar 19, 2020
@tg-msft
Copy link
Member Author

tg-msft commented Mar 19, 2020

azure-sdk-for-net/sdk/search/Azure.Search.Documents/src/Options/SearchOptions.cs

Line 85 in 47caf36

set => Select = SearchExtensions.CommaSplit(value);
as one example

@tg-msft
Copy link
Member Author

tg-msft commented Mar 19, 2020

This might only be a problem for $orderby and we'll need to dig into the REST OData docs

@tg-msft tg-msft modified the milestones: [2020] April, [2020] May Apr 6, 2020
@AlexGhiondea AlexGhiondea mentioned this issue Apr 8, 2020
Closed
26 tasks
@AlexGhiondea AlexGhiondea modified the milestones: [2020] May, [2020] June Apr 27, 2020
@AlexGhiondea AlexGhiondea mentioned this issue May 8, 2020
Closed
17 tasks
@tg-msft tg-msft modified the milestones: [2020] June, [2020] July Jun 9, 2020
@AlexGhiondea AlexGhiondea mentioned this issue Jun 11, 2020
Closed
15 tasks
@tg-msft tg-msft added the blocking-release Blocks release label Jun 16, 2020
tg-msft added a commit to tg-msft/azure-sdk-for-net that referenced this issue Jun 25, 2020
@tg-msft
Search: Escape OData OrderBy splitting
475b2ab 
Fixes Azure#10600 - it only affects OrderBy per

https://docs.microsoft.com/en-us/azure/search/search-query-odata-syntax-reference
@tg-msft tg-msft mentioned this issue Jun 25, 2020
Closed
tg-msft added a commit to tg-msft/azure-sdk-for-net that referenced this issue Jul 1, 2020
@tg-msft
Search: remove explicit continuation tokens
a3881fe 
Addressing Azure#10600.
This was referenced Jul 1, 2020
Merged
Closed
@tg-msft
Copy link
Member Author

tg-msft commented Jul 1, 2020

Associating with #13150 to decide how to handle the broader question.

@tg-msft tg-msft modified the milestones: [2020] July, [2020] August Jul 1, 2020
tg-msft added a commit that referenced this issue Jul 1, 2020
@tg-msft
Search: remove explicit continuation tokens (#13149)
0053c49 
Addressing #10600.
prmathur-microsoft pushed a commit that referenced this issue Jul 8, 2020
@tg-msft @prmathur-microsoft
Search: remove explicit continuation tokens (#13149)
ef2b99e 
Addressing #10600.
@tg-msft
Copy link
Member Author

tg-msft commented Aug 11, 2020

We've decided to solve continuation tokens differently and won't be making this change.

@tg-msft tg-msft closed this as completed Aug 11, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Mar 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@tg-msft tg-msft

Labels
blocking-release Blocks release Client This issue points to a problem in the data-plane of the library. Search
Projects
None yet
Milestone
[2020] August
Development

Successfully merging a pull request may close this issue.

Search: Escape OData OrderBy splitting tg-msft/azure-sdk-for-net
2 participants
@tg-msft @AlexGhiondea