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

Removed ${aspnet-request-posted-body} because not threadsafe #754

Merged
merged 1 commit into from
Feb 12, 2022

Conversation

snakefoot
Copy link
Contributor

@snakefoot snakefoot commented Feb 3, 2022

NLog 5.0 only supports threadsafe layoutrenderers. This LayoutRenderer modifies the Body-Stream while reading, so it gives unwanted side-effects and unexpected errors. See also #753

Instead request-body should be explicitly captured using the middleware-pipeline. And maybe stored in HttpContext.Items or in ScopeContext-Property.

@codecov-commenter
Copy link

Codecov Report

Merging #754 (473b356) into master (d85dfab) will increase coverage by 1%.
The diff coverage is n/a.

❗ Current head 473b356 differs from pull request most recent head 998710d. Consider uploading reports for the commit 998710d to get more accurate results

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #754   +/-   ##
=====================================
+ Coverage      63%    64%   +1%     
=====================================
  Files          49     48    -1     
  Lines         834    779   -55     
  Branches      206    193   -13     
=====================================
- Hits          522    497   -25     
+ Misses        238    214   -24     
+ Partials       74     68    -6     
Impacted Files Coverage Δ
...utRenderers/AspNetRequestDurationLayoutRenderer.cs 58% <0%> (+2%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 82867e7...998710d. Read the comment docs.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 8, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@snakefoot
Copy link
Contributor Author

snakefoot commented Jul 25, 2022

Thanks to @bakgerman now NLog.Web.AspNetCore v5.1 re-introduces ${aspnet-request-posted-body} with help from middleware:

app.UseMiddleware<NLog.Web.NLogRequestPostedBodyMiddleware>();

It is no longer necessary to explicit call context.Request.EnableBuffering(); as it is handled by the middleware.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants