thepirat000 / Audit.NET

An extensible framework to audit executing operations in .NET and .NET Core.
MIT License
2.31k stars 323 forks source link

Response body memory stream disposed #459

Closed valera1181 closed 3 years ago

valera1181 commented 3 years ago

https://github.com/thepirat000/Audit.NET/blob/7f0df5359224a3ce87e7719b36848e35a91ff636/src/Audit.WebApi/AuditMiddleware.cs#L54

When exception is thrown somewhere in controller InvokeNextAsync catches and rethrows exception. After that responseBody variable is disposed also as a context.Response.Body. This creates problems in middlewares declared before AuditMiddleware so that I can't write to response body anymore.

valera1181 commented 3 years ago
            // Admin website exception and error handling
            app.UseWhen(context => !context.IsWebApi(), adminApp =>
            {
                adminApp.UseExceptionHandler("/error/500");
                adminApp.UseStatusCodePagesWithRedirects("/error/{0}");
            });

            // Web api exception and error handling
            app.UseWhen(context => context.IsWebApi(), apiApp =>
            {
                apiApp.UseMiddleware<ApiErrorHandlerMiddleware>();
            });
            app.UseAuditMiddleware(config =>
            {
                config.FilterByRequest(r => r.IsAuditable());
                config.WithEventType("{verb}:{url}");
                config.IncludeHeaders();
                config.IncludeResponseHeaders();
                config.IncludeRequestBody(/*c => c.IncludeRequestBody()*/);
                config.IncludeResponseBody();
            });

    public class ApiErrorHandlerMiddleware
    {
        private readonly RequestDelegate _next;

        public ApiErrorHandlerMiddleware(RequestDelegate next)
        {
            _next = next;
        }

        public async Task Invoke(HttpContext context)
        {
            try
            {
                await _next(context);
            }
            catch (Exception exc)
            {
                var apiErrorResponse = new ApiErrorResponseDto();

                var response = context.Response;
                response.ContentType = "application/json";

                switch (exc)
                {
                    // custom application error
                    case AppException _:
                        response.StatusCode = (int)HttpStatusCode.BadRequest;
                        apiErrorResponse.Message = exc.Message;
                        break;
                    default:
                        // unhandled error
                        response.StatusCode = (int)HttpStatusCode.InternalServerError;
                        apiErrorResponse.Message = "Server error.";
                        break;
                }

                apiErrorResponse.StatusCode = response.StatusCode;

                var result = JsonSerializer.Serialize(apiErrorResponse);
                await response.WriteAsync(result);
            }
        }
    }

Exception disposed response body happens when response.WriteAsync(result); called

thepirat000 commented 3 years ago

I see where the bug is.

The AuditMiddleware temporarily copies the response body stream to a memory stream, and restores the original stream after invoking the next delegate, but when an exception was thrown invoking the next delegate, then the original body was not being restored and the memory stream disposed.

I will fix that so the response body stream gets restored in any case

thepirat000 commented 3 years ago

This was fixed on versions 18.1.6 and 19.0.0-rc.net60.2, please upgrade your references and re-test.