OData / AspNetCoreOData

ASP.NET Core OData: A server library built upon ODataLib and ASP.NET Core
Other
457 stars 158 forks source link

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

Open spaasis opened 2 months ago

spaasis commented 2 months ago

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

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 commented 2 months ago

@spaasis

mikepizzo commented 2 months ago

System.Uri has a max limit of 65519 See discussion in https://github.com/dotnet/runtime/issues/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 commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

@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