Error returned is 500 when 400 is expected

[berubejp]

Assemblies affected ASP.NET Core OData 8.x

Describe the bug When calling a OData endpoint, I get a 500 error instead of a 400. This occurs when adding an invalid key in the URL. Example odata/books/1' or odata/books/aaaaaaaa .

Reproduce steps Using my example here https://github.com/berubejp/webapiodata500 which is a basic web api with EF Core in memory and OData.

When calling odata/books/aaaaaaaa, I get a 500 response. When calling odata/books/1', I get a 500 response. Notice the single quote at the end or the URL.

The error is ODataException: The key value (xxx) from request is not valid.

Expected behavior This should be a 400 error since this is a client error.

[julealgon]

While your tool is classifying this as a potential SQL injection attack, I don't see how that would apply in this case considering the value is clearly being validated, so I'd suggest renaming the issue to avoid confusion.

The problem is that the error should be a 4XX (client error) instead of a 5XX (server error). The tool is probably considering that "if the server is throwing a server error, its probably trying to execute something with that character".

The fact that it is performing this before authentication kicks in is actually a separate issue IMHO (that is also potentially valid).

Thus I think this should also be split into 2 distinct issues, one for each aspect.

[berubejp]

While your tool is classifying this as a potential SQL injection attack, I don't see how that would apply in this case considering the value is clearly being validated, so I'd suggest renaming the issue to avoid confusion.

The problem is that the error should be a 4XX (client error) instead of a 5XX (server error). The tool is probably considering that "if the server is throwing a server error, its probably trying to execute something with that character".

The fact that it is performing this before authentication kicks in is actually a separate issue IMHO (that is also potentially valid).

Thus I think this should also be split into 2 distinct issues, one for each aspect.

Done

[habbes]

Hi @berubejp @julealgon thanks for reporting this. Seems like there are inconsistencies in how we handle validation error.

If there's an error in the query options, e.g. $selecting a field that does not exist, this returns a 400 bad request where the response body is JSON-formatted error object. In this case, OData Core throws an ODataException and EnableQueryAttribute catches the exception and sets the error result.

If there's unsupported syntax in the path section like in your example, an ODataException is thrown, but the error is not caught, leading to an unhandled exception and therefore 500.

A workaround I can think of right now is to create a custom middleware or filter than catches ODataExceptions and returns a suitable error response.

[dnperfors]

This workaround is not really an option, since ODataException is being thrown all over the place and it doesn't contain any information except for the message and the stacktrace.

[julealgon]

This workaround is not really an option, since ODataException is being thrown all over the place and it doesn't contain any information except for the message and the stacktrace.

I assume you mean in the sense that it would not be easy to differentiate "validation" exceptions from other exceptions? So we would in some cases end up converting ODataException instances that are maybe supposed to be server errors into client errors.

Do we have any documentation regarding ODataException and all the types of problems it includes? I don't know of the top of my head if all of them would map to a client error myself.

If it turns out multiple categories of problems all rely on ODataException, it might be sensible to extract a new exception type, maybe ODataValidationException, that is specific to validation concerns.

[dnperfors]

I assume you mean in the sense that it would not be easy to differentiate "validation" exceptions from other exceptions? So we would in some cases end up converting ODataException instances that are maybe supposed to be server errors into client errors.

Do we have any documentation regarding ODataException and all the types of problems it includes? I don't know of the top of my head if all of them would map to a client error myself. (I doubt it...)

If it turns out multiple categories of problems all rely on ODataException, it might be sensible to extract a new exception type, maybe ODataValidationException, that is specific to validation concerns.

Yes, that is correct. This specific exception is thrown by the Uri parser which sits (if I am correct) in the OData.Core packages. But most problems will generate an ODataException (There is a ValidationException using in EdmPrimitiveHelper as well, but I didn't see that one anywhere in my logs).