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.

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

Really long POST $query results in incomplete JSON response #1293

Open
spaasis opened this issue Aug 6, 2024 · 5 comments
Open

Really long POST $query results in incomplete JSON response #1293

spaasis opened this issue Aug 6, 2024 · 5 comments
Assignees
Labels
bug Something isn't working investigating P2

Comments

@spaasis
Copy link

spaasis commented Aug 6, 2024

POSTing a really long $query breaks the JSON output of the response.
Since UseODataQueryRequest does not actually add any endpoints that accept and handle as pure POST, but rather just redirects the POST to the corresponding GET, there is a chance that the query exceeds the server limits and does not work. This, I think, is the cause for this issue

Assemblies affected

Microsoft.AspNetCore.OData 8.2.5
Asp.Versioning.OData 8.1.0
Asp.Versioning.OData.ApiExplorer 8.1.0

Reproduce steps

A recent extreme example I had the misfortune of debugging:

POST /ObservationSites/$query
Body:
$select=name,observationSiteId&$filter=observationSiteId in (<over 7000 ids>)&$top=10&$skip=0&$count=true

Calling the same endpoint with some 3000 ids works as expected.

And yeah, I know the query itself is quite insane, but at least it highlighted this bug 😅

Expected result

  • the server would return correctly formatted json
  • the logs would not contain the POST body parameters (since it does now, as the query is actually a GET)

Actual result

The JSON response contains the correct data but is malformed:

{"@odata.context":"<metadata>","@odata.count":7087,"value":[
{"observationSiteId":"1","name":"someName"}
//the 9 other queried 
}

So the data is fetched, but the json is missing the last ] and }. This happens the same way every time we tried.

And the backend server logs An unhandled exception has occurred while executing the request. System.UriFormatException: Invalid URI: The Uri string is too long. from the GET endpoint since, well, the request is pretty damn long.

I find it really weird that the data actually gets fetched here, but it does..

Additional detail

I would love to have an option to manually construct a query, so I could define an endpoint e.g.

[EnableQuery]
[HttpPost("$query")]
public IQueryable<Thing> PostQuery(string query){
  var opts = new ODataQueryOptions(magically create this from the parameter)
  return opts.ApplyTo(things)
}
@WanjohiSammy
Copy link

WanjohiSammy commented Aug 6, 2024

@spaasis

  • Please try remove $top and $skip
  • Kindly share the reproduce steps

@WanjohiSammy WanjohiSammy added investigating bug Something isn't working labels Aug 6, 2024
@xuzhg xuzhg transferred this issue from OData/WebApi Aug 6, 2024
@xuzhg xuzhg added the P2 label Aug 6, 2024
@mikepizzo
Copy link
Member

System.Uri has a max limit of 65519 See discussion in dotnet/runtime#96544.

I suspect internally we are trying to build a System.Uri for the original query string (i.e., for generating a nextLink, which shouldn't really be necessary given the $top=10) and are running into this limitation.

@spaasis
Copy link
Author

spaasis commented Aug 7, 2024

@mikepizzo that makes sense. The stack trace says as much too - sorry I didn't read into it enough:

[16:27:24 ERR] An unhandled exception has occurred while executing the request.
System.UriFormatException: Invalid URI: The Uri string is too long. at System.Uri.CreateThis(String uri, Boolean dontEscape, UriKind uriKind, UriCreationOptions& creationOptions) at System.Uri..ctor(String uriString) at Microsoft.AspNetCore.OData.Formatter.Serialization.ODataResourceSetSerializer.<>c__DisplayClass17_2.<GetNextLinkGenerator>b__2(Object obj)

Should the nextlink generation be disabled altogether when the caller is using the $query syntax? I don't really see the point of providing a GET endpoint since the user has started with a POST, supposedly for a reason.

Or as a simple workaround, catch and log the error in nextlink generation but allow it to be empty?

I can still provide a minimal repro if that is useful?

@mikepizzo
Copy link
Member

mikepizzo commented Aug 7, 2024

Have you set PageSize in your EnableQuery attribute or ODataQuerySettings? What happens if you set it to null?

We shouldn't really need to create a nextLink since you are only retrieving the top=10 items, but perhaps we are creating it eagerly.

We may also be creating a Uri for some reason unrelated to nextLink, so yes -- a repro would be helpful.

@spaasis
Copy link
Author

spaasis commented Aug 8, 2024

@mikepizzo Sorry, the formatting in my last message somehow cut out the relevant part - the error is thrown in GetNextLinkGenerator. I updated the formatting to show this.

@WanjohiSammy Here's a minimal repro with the webapi dotnet template https://github.com/spaasis/odata-uri-error-repro/tree/main The repro steps are in the readme. I tested also with just the $filter, so no $top, $skip or $count, and it still happens.

At least with no explicit configuration of PageSize anywhere in that repo the error still happens

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working investigating P2
Projects
None yet
Development

No branches or pull requests

4 participants