OData
Some OData exceptions are not caught by the WebApi, but swallowed by Odata
We have a WebApi in AspNet core 2.2 that uses AspNetCore.OData version 7.2.1. For security reasons we want to have full control over the responses when exceptions are thrown, so OData stacktraces are not send back to our Api consumers. However, some OData exceptions cannot be caught in the API; They are swallowed by the OData layer.
We are looking for a way to catch these exceptions, but are unable to do so. Below is a way to reproduce these kind of exceptions.
Assemblies affected
Microsoft.AspNetCore.OData 7.2.1
Reproduce steps
Reproduction steps:
- In visual studio create a new ASP.NET Core Web Application project, Target the ASP.NET Core framework version 2.2 and add the latest nuget package Microsoft.AspNetCore.OData (currently at version 7.2.1) and configure OData
- Add an example controller, e.g.:
[Route("api/[controller]")]
public class BooksController : ODataController
{
// GET api/books
[HttpGet]
[ODataSampleEnableQuery]
public IQueryable<Book> Get(ODataQueryOptions queryOptions)
{
ODataSampleEnableQueryAttribute.Validate(queryOptions);
return new Book[]
{
new Book() { Id = 1, Title = "1984", Author = "George Orwell", Available = true},
new Book() { Id = 2, Title = "The Hobbit", Author = "J.R.R. Tolkien", Available = false}
}.AsQueryable();
}
}
public class Book
{
public long Id { get; set; }
public string Title { get; set; }
public string Author { get; set; }
public bool Available { get; set; }
}
- Make sure all exceptionhandling is in place
A. Catch application exceptions:- Add in Startup.cs.Configure(..,..) (above the statement "app.UseMvc(..)"):
app.UseExceptionHandler(appError =>
{
appError.Run(async context =>
{
context.Response.StatusCode = (int)HttpStatusCode.InternalServerError;
context.Response.ContentType = "application/json";
//Error logging goes here
await context.Response.WriteAsync("An error has occurred. Check the log.");
});
});
B. Catch OData validation exceptions: Create an inherited class from the EnableQueryAttribute to verify the ODataQueryOptions in your application code:
public class ODataSampleEnableQueryAttribute : EnableQueryAttribute
{
public override void ValidateQuery(HttpRequest request, ODataQueryOptions queryOptions)
{
try
{
Validate(queryOptions);
base.ValidateQuery(request, queryOptions);
}
//Errors are of type ODataException or ArgumentOutOfRangeException
catch (Exception e)
{
//Will be a specific exception in production code
throw new Exception("An error occurred", e)
{ };
}
}
public static void Validate(ODataQueryOptions queryOptions)
{
try
{
queryOptions.Validate(new ODataValidationSettings() { });
}
catch (Exception e)
{
//Will be a specific exception in production code
throw new Exception(e.Message, e)
{ };
}
}
}
- Send an url to the API that is not caught bu the application: GET https://localhost:44304/api/Books?$count=True GET https://localhost:44304/api/Books?$filter=startswith(title, null)
Expected result
Exception is caught by the application's exception handler OR Exception is thrown by the ODataQueryOptions.Validate() (and in our case redirected to the application's exception handler)
Actual result
No exception is caught by the application. OData swallows/catches the exception and returnes a 400 response with full stacktrace:
{
"Message": "The query specified in the URI is not valid. The 'startswith' function cannot be applied to an enumeration-typed argument.",
"ExceptionMessage": "The 'startswith' function cannot be applied to an enumeration-typed argument.",
"ExceptionType": "Microsoft.OData.ODataException",
"StackTrace": " at Microsoft.AspNet.OData.Query.Expressions.FilterBinder.ValidateAllStringArguments(String functionName, Expression[] arguments)\r\n at Microsoft.AspNet.OData.Query.Expressions.FilterBinder.BindStartsWith(SingleValueFunctionCallNode node)\r\n at Microsoft.AspNet.OData.Query.Expressions.FilterBinder.BindSingleValueFunctionCallNode(SingleValueFunctionCallNode node)\r\n at Microsoft.AspNet.OData.Query.Expressions.FilterBinder.BindSingleValueNode(SingleValueNode node)\r\n at Microsoft.AspNet.OData.Query.Expressions.FilterBinder.Bind(QueryNode node)\r\n at Microsoft.AspNet.OData.Query.Expressions.FilterBinder.BindExpression(SingleValueNode expression, RangeVariable rangeVariable, Type elementType)\r\n at Microsoft.AspNet.OData.Query.Expressions.FilterBinder.BindFilterClause(FilterBinder binder, FilterClause filterClause, Type filterType)\r\n at Microsoft.AspNet.OData.Query.Expressions.FilterBinder.Bind(IQueryable baseQuery, FilterClause filterClause, Type filterType, ODataQueryContext context, ODataQuerySettings querySettings)\r\n at Microsoft.AspNet.OData.Query.FilterQueryOption.ApplyTo(IQueryable query, ODataQuerySettings querySettings)\r\n at Microsoft.AspNet.OData.Query.ODataQueryOptions.ApplyTo(IQueryable query, ODataQuerySettings querySettings)\r\n at Microsoft.AspNet.OData.EnableQueryAttribute.ApplyQuery(IQueryable queryable, ODataQueryOptions queryOptions)\r\n at Microsoft.AspNet.OData.EnableQueryAttribute.ExecuteQuery(Object responseValue, IQueryable singleResultCollection, IWebApiActionDescriptor actionDescriptor, Func`2 modelFunction, IWebApiRequestMessage request, Func`2 createQueryOptionFunction)\r\n at Microsoft.AspNet.OData.EnableQueryAttribute.OnActionExecuted(Object responseValue, IQueryable singleResultCollection, IWebApiActionDescriptor actionDescriptor, IWebApiRequestMessage request, Func`2 modelFunction, Func`2 createQueryOptionFunction, Action`1 createResponseAction, Action`3 createErrorAction)"
}
I'm facing the same issue! Does anyone know a way to handle this exception?
@KanishManuja-MS Is there a way to handle this?
I'm facing the same issue! Does anyone know a way to handle this exception?
One way it worked for me is to explicitly call 'queryOption.Validate()'. this will throw an exception if there is any issue. The same can be caught my middleware.
My solution:
public class ErrorHandlingMiddleware {
private readonly RequestDelegate next;
public ErrorHandlingMiddleware(RequestDelegate next) {
this.next = next;
}
public async Task Invoke(HttpContext context /* other dependencies */ ) {
bool modifyResponse = true;
Stream originBody = null;
if (modifyResponse) {
//uncomment this line only if you need to read context.Request.Body stream
//context.Request.EnableRewind();
originBody = ReplaceBody(context.Response);
}
try {
await next(context);
} catch (Exception ex) {
await HandleExceptionAsync(context, ex);
} finally {
//as we replaced the Response.Body with a MemoryStream instance before,
//here we can read/write Response.Body
//containing the data written by middlewares down the pipeline
//finally, write modified data to originBody and set it back as Response.Body value
ReturnBody(context.Response, originBody);
}
}
private Stream ReplaceBody(HttpResponse response) {
var originBody = response.Body;
response.Body = new MemoryStream();
return originBody;
}
private void ReturnBody(HttpResponse response, Stream originBody) {
response.Body.Seek(0, SeekOrigin.Begin);
response.Body.CopyTo(originBody);
response.Body = originBody;
}
private Task HandleExceptionAsync(HttpContext context, Exception ex) {
var code = HttpStatusCode.InternalServerError; // 500 if unexpected
var result = JsonConvert.SerializeObject(new {Error= ex.Message});
context.Response.Body.Seek(0, SeekOrigin.Begin);
context.Response.ContentType = "application/json";
context.Response.StatusCode = (int) code;
return context.Response.WriteAsync(result);
}
}
In Startup.cs
//Add in Configure before UseMvc
app.UseMiddleware(typeof(ErrorHandlingMiddleware));
Usually errors that occur outside the MVC pipeline has to be handled by a middleware. These errors include:
- any errors prior MVC pipeline is entered
- any errors after the MVC pipeline (deferred iqueryable errors, serialization errors)
If you are returning an un-evaluated IQueryable (deferred) then you might have SQL execution errors, but also if there's some Serialization issue (circular dependencies, etc.)...
One way it worked for me is to explicitly call 'queryOption.Validate()'. this will throw an exception if there is any issue. The same can be caught my middleware.
Thanks for the revert.
When I pass some query as expand, my api gives correct result and in case of in vague query parameter for example "$expand=qwe123" it gives me 400 bad request with exception message : "Query specified in URI is invalid"
type : ODataException
Is there any way that I can handle this in my controller?
in the controller get method.... use OdataQueryOptions explicitly... like the one below.. or u can catch that error in middleware. Supporting ODataQueryOptions in existing Web API
| | | | | |
|
| | | | Supporting ODataQueryOptions in existing Web API
I have a Web API project which has been used for several years without OData support, just with standard URL par... |
|
|
On Thursday, 18 June, 2020, 06:44:52 pm IST, Ishan Shah <[email protected]> wrote:
One way it worked for me is to explicitly call 'queryOption.Validate()'. this will throw an exception if there is any issue. The same can be caught my middleware.
Thanks for the revert.
When I pass some query as expand, my api gives correct result and in case of in vague query parameter for example "$expand=qwe123" it gives me 400 bad request with exception message : "Query specified in URI is invalid"
type : ODataException
Is there any way that I can handle this in my controller?
— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.
Using the custom EnableQueryAttribute in the original comment by @mabouwhui fixes the problem in our application in that it redirects the ODataException to our own error handling logic, but I want to add my +1 to the original issue because the current error handling exposes the full stack trace and implementation detail in production. This seems like a big security leak the OData team seems willing to handle in most other cases. Severity on this one should be escalated, IMHO.
Using the custom
EnableQueryAttributein the original comment by @mabouwhui fixes the problem in our application in that it redirects the ODataException to our own error handling logic, but I want to add my +1 to the original issue because the current error handling exposes the full stack trace and implementation detail in production. This seems like a big security leak the OData team seems willing to handle in most other cases. Severity on this one should be escalated, IMHO.
I have created the CustomEnableQueryAttribute and used it for endpoints. Debugger strikes try block , but exception is not caught. Debugger does not enters the catch block.
This is with oData-7.5
Any updates on this ? I'm still relying on @mabouwhui solution, but it really feels to me that in 2023 a better solution should be available... I'm I wrong ? I'm I missing something ?
This issue is closed, because - I think - a workaround is available. For clarification I will write a wrap-up.
To catch ALL OData exceptions it is not a good enough solution to create a catch in the override of the EnableQueryAttribute class. Under some circumstances exceptions will not be caught and may 'leak' back to the API consumer. This was the reason why I added this issue.
The workaround is to create a middleware that will catch all exceptions in your API, as suggested by @SenyaMur. This is a good idea anyway, because exceptions from other middleware might also 'leak'. Probably an open door, but a small addition to the suggested solution: Be aware of the position in startup.cs of the code where you add the middleware (app.UseMiddleware(... ). It is typically the first line in the Configure() method.
If you are using the AspNetCoreOData package, the solution I've found is to combine a custom ODataSerializer with an error-handling middleware. This way you can control the response body and thus clear the stack trace from it. Some exceptions will be caught in your middleware, and some in the serializer (because some exceptions are being handled automatically within the private methods of this package and no middleware will help you).
The approach with the custom ODataErrorSerializer, as mentioned by @hexagonite is very interesting. However, I find it to be still very limiting. For my case, I am getting a SerializableError which is practically a Dictionary<string, string>. This makes a problem when I want to handle certain errors differently, forcing me to parse the strings. The API must be friendlier. I think it might be a fair expectation that at minimum the ErrorCode key is set.
My solution:
public class ErrorHandlingMiddleware { private readonly RequestDelegate next; public ErrorHandlingMiddleware(RequestDelegate next) { this.next = next; } public async Task Invoke(HttpContext context /* other dependencies */ ) { bool modifyResponse = true; Stream originBody = null; if (modifyResponse) { //uncomment this line only if you need to read context.Request.Body stream //context.Request.EnableRewind(); originBody = ReplaceBody(context.Response); } try { await next(context); } catch (Exception ex) { await HandleExceptionAsync(context, ex); } finally { //as we replaced the Response.Body with a MemoryStream instance before, //here we can read/write Response.Body //containing the data written by middlewares down the pipeline //finally, write modified data to originBody and set it back as Response.Body value ReturnBody(context.Response, originBody); } } private Stream ReplaceBody(HttpResponse response) { var originBody = response.Body; response.Body = new MemoryStream(); return originBody; } private void ReturnBody(HttpResponse response, Stream originBody) { response.Body.Seek(0, SeekOrigin.Begin); response.Body.CopyTo(originBody); response.Body = originBody; } private Task HandleExceptionAsync(HttpContext context, Exception ex) { var code = HttpStatusCode.InternalServerError; // 500 if unexpected var result = JsonConvert.SerializeObject(new {Error= ex.Message}); context.Response.Body.Seek(0, SeekOrigin.Begin); context.Response.ContentType = "application/json"; context.Response.StatusCode = (int) code; return context.Response.WriteAsync(result); } }In Startup.cs
//Add in Configure before UseMvc app.UseMiddleware(typeof(ErrorHandlingMiddleware));
For .NET6 you have to make ReturnBody async. Otherwise an exception 'Synchronous operations are disallowed. Call WriteAsync or set AllowSynchronousIO to true instead.' is thrown.
public class ErrorHandlingMiddleware {
private readonly RequestDelegate next;
public ErrorHandlingMiddleware(RequestDelegate next) {
this.next = next;
}
public async Task Invoke(HttpContext context /* other dependencies */ ) {
bool modifyResponse = true;
Stream originBody = null;
if (modifyResponse) {
//uncomment this line only if you need to read context.Request.Body stream
//context.Request.EnableRewind();
originBody = ReplaceBody(context.Response);
}
try {
await next(context);
} catch (Exception ex) {
await HandleExceptionAsync(context, ex);
} finally {
//as we replaced the Response.Body with a MemoryStream instance before,
//here we can read/write Response.Body
//containing the data written by middlewares down the pipeline
//finally, write modified data to originBody and set it back as Response.Body value
await ReturnBodyAsync(context.Response, originBody); // <-- await Call
}
}
private Stream ReplaceBody(HttpResponse response) {
var originBody = response.Body;
response.Body = new MemoryStream();
return originBody;
}
private async Task ReturnBodyAsync(HttpResponse response, Stream originBody) { // <-- changed to async method
response.Body.Seek(0, SeekOrigin.Begin);
await response.Body.CopyToAsync(originBody); // <-- changed to CopyAsync
response.Body = originBody;
}
private Task HandleExceptionAsync(HttpContext context, Exception ex) {
var code = HttpStatusCode.InternalServerError; // 500 if unexpected
var result = JsonConvert.SerializeObject(new {Error= ex.Message});
context.Response.Body.Seek(0, SeekOrigin.Begin);
context.Response.ContentType = "application/json";
context.Response.StatusCode = (int) code;
return context.Response.WriteAsync(result);
}
}
I've found a way to get the exception when you have a ModelState:
private Exception? GetExceptionFromModelState(ModelStateDictionary pModelState)
{
var xError = pModelState.Values.SelectMany(p => p.Errors)
.Where(p => p.Exception != null)
.Select(p => p.Exception!)
.FirstOrDefault();
return xError;
}
With this, you can rethow the exception, or deal with it in another way. Example:
[HttpPatch]
public virtual async Task<IActionResult> Patch(int key
, Delta<Item> pItem)
{
var xError = GetExceptionFromModelState(ModelState);
if (xError != null)
throw xError;
...
Using the custom
EnableQueryAttributein the original comment by @mabouwhui fixes the problem in our application in that it redirects the ODataException to our own error handling logic, but I want to add my +1 to the original issue because the current error handling exposes the full stack trace and implementation detail in production. This seems like a big security leak the OData team seems willing to handle in most other cases. Severity on this one should be escalated, IMHO.I have created the
CustomEnableQueryAttributeand used it for endpoints. Debugger strikes try block , but exception is not caught. Debugger does not enters thecatchblock.
This is with
oData-7.5
This solution is not working for $select and expand throwing exception System.ArgumentException: 'Object of type 'System.Linq.EnumerableQuery1[Microsoft.AspNetCore.OData.Query.Wrapper.SelectSome1[models]]' cannot be converted to type 'System.Linq.IQueryable`1[models]'.'
If you are using the AspNetCoreOData package, the solution I've found is to combine a custom ODataSerializer with an error-handling middleware. This way you can control the response body and thus clear the stack trace from it. Some exceptions will be caught in your middleware, and some in the serializer (because some exceptions are being handled automatically within the private methods of this package and no middleware will help you).
@hexagonite We have a use case where we want to handle exceptions differently and we don't want to show the Inner Exception details to the clients. I have successfully implemented an ODataSerializer to modify the error response from the below reference
https://github.com/OData/AspNetCoreOData/blob/main/docs/customize_odataerror.md
Here we set the inner exception to null before sending the response but wanted to log the exceptions to ILogger before that. But unable to log them as I'm getting an exception while doing the dependency injection for loggerservice. Here I have tried different ways to pass the logger instance from program.cs. But the expected service provider doesn't have logger instance.
**Program.cs
builder.Services.AddControllers(options =>
{
options.Filters.Add<EISODataAuthorizationFilter>();
}).AddOData(options =>
{
options.AddRouteComponents("latest", GetEdmModel(),
service => service.AddSingleton<IODataSerializerProvider>((container) =>
{
var logger = container.GetRequiredService<ILoggerFactory>().CreateLogger<ErrorSerializer>();
return new EisErrorSerializerProvider(container, logger);
})
);
ODataSerializerProvider: public class EisErrorSerializerProvider : ODataSerializerProvider { private readonly ILogger<ErrorSerializer> logger; public EisErrorSerializerProvider(IServiceProvider sp, ILogger<ErrorSerializer> _logger) : base(sp) { logger = _logger; }
public override IODataSerializer GetODataPayloadSerializer(Type type, HttpRequest request)
{
if ((type == typeof(ODataError) || type == typeof(SerializableError)))
{
return new ErrorSerializer(logger);
}
return base.GetODataPayloadSerializer(type, request);
}
}
public class ErrorSerializer : ODataErrorSerializer { private readonly string _attributeName; private readonly ILogger<ErrorSerializer> _logger;
public ErrorSerializer(ILogger<ErrorSerializer> logger)
{
_attributeName = nameof(ErrorSerializer);
_logger = logger;
_logger.LogInformation($"Logger created for ErrorSerializer");
}
public override Task WriteObjectAsync(object graph, Type type, ODataMessageWriter messageWriter, ODataSerializerContext writeContext)
{
var methodName = nameof(WriteObjectAsync);
_logger.LogError($"AttributeName: {_attributeName} Method: {methodName} Started");
if (graph is SerializableError error)
{
ODataError oDataError = error.CreateODataError();
_logger.LogInformation($"Error occurred while Serializing the Query input in Method:{methodName}, ExceptionMessage: {oDataError?.InnerError?.Message}, StackTrace:{oDataError?.InnerError?.StackTrace}, InnerError: {oDataError?.InnerError?.InnerError}");
oDataError.InnerError = null;
return base.WriteObjectAsync(oDataError, typeof(ODataError), messageWriter, writeContext);
}
else if (graph is ODataError oDataError)
{
_logger.LogError($"Error occurred while executing the input query in Method:{methodName}, Exception: {oDataError.InnerError}");
oDataError.InnerError = null;
return base.WriteObjectAsync(oDataError, typeof(ODataError), messageWriter, writeContext);
}
return base.WriteObjectAsync(graph, type, messageWriter, writeContext);
}
}**
I have tried another way to implement the combination of Errormiddlware and ODataSerializerProvider to capture the error from the below reference. [OData] Ensure that Odata runtime exceptions are not swallowed NoxOrg/Nox#362
In this case as well won't be able to catch the exception in the catch block of ErrorMiddleware.
Do you have any solution for this? we want to capture the logs to the appinsightsTelemetry
@raginigunnam it's been a while since I worked with this and I don't even have access to the code I worked on anymore. However, if there is no solution for dependency injection to work (assuming you've done everything correctly), I would suggest implementing a workaround, which is static loggers which don't require dependency injection.
For example, consider using Serilog with static loggers: Serilog - Writing Log Events
I'm fully aware that this is not a perfect solution, just trying to help in any way I can.