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

Restore aspnet-request-posted-body with middleware #781

Merged
merged 25 commits into from
Jun 1, 2022
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
2764db7
Hopefully restore aspnet-request-posted-body using middleware approac…
May 24, 2022
fdd2a1c
Increase unit test coverage
May 24, 2022
ecd3edb
Increase unit test coverage
May 24, 2022
5dd35ea
Put EnableBuffering() before CanSeek for NLogRequestPostedBodyMiddleware
May 24, 2022
969bcff
Increase limit from 8KB to 30KB
May 24, 2022
1c39dc2
(Hopefully) Make the IHttpModule request body capture work for .NET 3…
May 25, 2022
b324383
Fix SonarQube detected defect in RequestPostedBodyHttpModule NET35-45…
May 25, 2022
75f7b20
Allow RequestPostedBodyHttpModule to set Encoding, BufferSize, and Sh…
May 25, 2022
2d60921
Fix defect in RequestPostedBodyHttpModule Encoding property not being…
May 25, 2022
2360654
Initially set responseText in RequestPostedBodyHttpModule instead of …
May 25, 2022
e5ec5b9
Upgrade the RequestPostedBodyHttpModule to have a Configuration prope…
May 25, 2022
465f50a
Make changes according to snakefoot review.
May 28, 2022
51fd3b0
2nd commit for snakefoots recommended changes
May 28, 2022
a77dd4c
refactor the request posted body http module class
May 28, 2022
46b384a
Improve unit test coverage for request post body middleware
May 28, 2022
c7cd7ab
Added another unit test for requiest poated body middleware
May 28, 2022
1d49852
Additional unit test coverage
May 28, 2022
b367aef
Removing NLogRequestPostedBodyHttpModule and related classes.
May 29, 2022
5bf8a9c
Use different middleware injection to not construct each time for eac…
May 30, 2022
263c015
Execute code review changes. Still need to convert string key to object
May 30, 2022
a0e81b2
Convert string key to object as requested
May 30, 2022
e3e8f73
Fix an incorrect comment
May 30, 2022
b3b67ff
Added a unit test for a null HttpContext
May 31, 2022
8603c6d
Review comments changed as requested
May 31, 2022
3c8d902
Use "option" instead of "configuration" for review feedback.
Jun 1, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 138 additions & 0 deletions src/NLog.Web.AspNetCore/NLogRequestPostedBodyMiddleware.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
using System.IO;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using NLog.Common;
using NLog.Web.LayoutRenderers;

namespace NLog.Web
{
/// <summary>
/// This class is to intercept the HTTP pipeline and to allow additional logging of the following
///
/// POST request body
///
/// The following are saved in the HttpContext.Items collection
///
/// __nlog-aspnet-request-posted-body
///
/// Usage: app.UseMiddleware&lt;NLogRequestPostBodyMiddleware&gt;(); where app is an IApplicationBuilder
/// Register the NLogRequestPostBodyMiddlewareConfiguration in the IoC so that the config gets passed to the constructor
/// </summary>
public class NLogRequestPostedBodyMiddleware : IMiddleware
{
private NLogRequestPostedBodyMiddlewareConfiguration _configuration { get; }
snakefoot marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Constructor that takes a configuration
/// </summary>
/// <param name="configuration"></param>
public NLogRequestPostedBodyMiddleware(NLogRequestPostedBodyMiddlewareConfiguration configuration)
{
_configuration = configuration;
}

/// <summary>
/// This allows interception of the HTTP pipeline for logging purposes
/// </summary>
/// <param name="context">The HttpContext</param>
/// <param name="next">The RequestDelegate that is to be executed next in the HTTP pipeline</param>
/// <returns></returns>
public async Task InvokeAsync(HttpContext context, RequestDelegate next)
{
// Perform null checking
if (context.Request == null)
snakefoot marked this conversation as resolved.
Show resolved Hide resolved
snakefoot marked this conversation as resolved.
Show resolved Hide resolved
{
InternalLogger.Debug("NLogRequestPostedBodyMiddleware: HttpContext.Request stream is null");
// Execute the next class in the HTTP pipeline, this can be the next middleware or the actual handler
await next(context).ConfigureAwait(false);
return;
}

// Perform null checking
if (context.Request.Body == null)
{
InternalLogger.Debug("NLogRequestPostedBodyMiddleware: HttpContext.Request.Body stream is null");
// Execute the next class in the HTTP pipeline, this can be the next middleware or the actual handler
await next(context).ConfigureAwait(false);
return;
}

// If we cannot read the stream we cannot capture the body
if (!context.Request.Body.CanRead)
{
InternalLogger.Debug("NLogRequestPostedBodyMiddleware: HttpContext.Request.Body stream is non-readable");
// Execute the next class in the HTTP pipeline, this can be the next middleware or the actual handler
await next(context).ConfigureAwait(false);
return;
}

// This is required, otherwise reading the request will destructively read the request
context.Request.EnableBuffering();
snakefoot marked this conversation as resolved.
Show resolved Hide resolved

// If we cannot reset the stream position to zero, and then back to the original position
// we cannot capture the body
if (!context.Request.Body.CanSeek)
{
InternalLogger.Debug("NLogRequestPostedBodyMiddleware: HttpContext.Request.Body stream is non-seekable");
// Execute the next class in the HTTP pipeline, this can be the next middleware or the actual handler
await next(context).ConfigureAwait(false);
return;
}

// Use the predicate in the configuration instance that takes the HttpContext as an argument
if (_configuration.ShouldCapture(context))
{
// Save the POST request body in HttpContext.Items with a key of '__nlog-aspnet-request-posted-body'
context.Items[AspNetRequestPostedBodyLayoutRenderer.NLogPostedRequestBodyKey] =
await GetString(context?.Request.Body).ConfigureAwait(false);
snakefoot marked this conversation as resolved.
Show resolved Hide resolved
}
else
{
InternalLogger.Debug("NLogRequestPostedBodyMiddleware: _configuration.ShouldCapture(HttpContext) predicate returned false");
}

// Execute the next class in the HTTP pipeline, this can be the next middleware or the actual handler
await next(context).ConfigureAwait(false);
}

/// <summary>
/// Convert the stream to a String for logging.
/// If the stream is binary please do not utilize this middleware
/// Arguably, logging a byte array in a sensible format is simply not possible.
/// </summary>
/// <param name="stream"></param>
/// <returns>The contents of the Stream read fully from start to end as a String</returns>
private async Task<string> GetString(Stream stream)
{
// Save away the original stream position
var originalPosition = stream.Position;

// This is required to reset the stream position to the beginning in order to properly read all of the stream.
stream.Position = 0;

string responseText = null;

// The last argument, leaveOpen, is set to true, so that the stream is not pre-maturely closed
// therefore preventing the next reader from reading the stream.
// The middle three arguments are from the configuration instance
// These default to UTF-8, true, and 1024.
using (var streamReader = new StreamReader(
stream,
_configuration.Encoding,
_configuration.DetectEncodingFromByteOrderMark,
_configuration.BufferSize,
leaveOpen: true))
{
// This is the most straight forward logic to read the entire body
responseText = await streamReader.ReadToEndAsync().ConfigureAwait(false);
}

// This is required to reset the stream position to the original, in order to
// properly let the next reader process the stream from the original point
stream.Position = originalPosition;

// Return the string of the body
return responseText;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
using System;
using System.Text;
using Microsoft.AspNetCore.Http;

namespace NLog.Web
{
/// <summary>
/// Contains the configuration for the NLogRequestPostedBodyMiddleware
/// </summary>
public class NLogRequestPostedBodyMiddlewareConfiguration
snakefoot marked this conversation as resolved.
Show resolved Hide resolved
{
/// <summary>
/// The default configuration
/// </summary>
public static readonly NLogRequestPostedBodyMiddlewareConfiguration Default = new NLogRequestPostedBodyMiddlewareConfiguration();
snakefoot marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Defaults to UTF-8
/// </summary>
public Encoding Encoding { get; set; } = Encoding.UTF8;

/// <summary>
/// Defaults to 1024
/// </summary>
public int BufferSize { get; set; } = 1024;

/// <summary>
/// Defaults to true
/// </summary>
public bool DetectEncodingFromByteOrderMark { get; set; } = true;

/// <summary>
/// If this returns true, the post request body will be captured
/// Defaults to true if content length &lt;= 30KB
/// This can be used to capture only certain content types,
/// only certain hosts, only below a certain request body size, and so forth
/// </summary>
/// <returns></returns>
public Predicate<HttpContext> ShouldCapture { get; set; } = DefaultCapture;

/// <summary>
/// The default predicate for ShouldCapture
/// Returns true if content length &lt;= 30KB
/// </summary>
public static bool DefaultCapture(HttpContext context)
snakefoot marked this conversation as resolved.
Show resolved Hide resolved
{
return context?.Request?.ContentLength != null && context?.Request?.ContentLength <= 30 * 1024;
}
}
}
169 changes: 169 additions & 0 deletions src/NLog.Web/NLogRequestPostedBodyHttpModule.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
using System;
using System.Collections;
using System.IO;
using System.Text;
using System.Web;
using NLog.Common;
using NLog.Web.LayoutRenderers;

namespace NLog.Web
{
/// <summary>
/// This class is to intercept the HTTP pipeline and to allow additional logging of the following
///
/// POST request body
///
/// The following are saved in the HttpContext.Items collection
///
/// __nlog-aspnet-request-posted-body
///
/// To use, subclass this class and set your own Configuration
/// </summary>
public class NLogRequestPostedBodyHttpModule : IHttpModule
{
/// <summary>
/// The name of the HttpModule
/// You may override in the subclass with your own name
/// </summary>
public string ModuleName { get; set; } = "NLog Request Posted Body Module";

/// <summary>
/// The configuration for the HttpModule
/// </summary>
public NLogRequestPostedBodyMiddlewareConfiguration Configuration { get; set; } =
NLogRequestPostedBodyMiddlewareConfiguration.Default;


/// <summary>
/// Hook in to the BeginRequest event to capture the request posted body
/// </summary>
/// <param name="context"></param>
public void Init(HttpApplication context)
{
context.BeginRequest += InterceptRequest;
}

/// <summary>
/// This will forward the necessary arguments to the capture request body method
/// </summary>
/// <param name="sender"></param>
/// <param name="args"></param>
protected void InterceptRequest(object sender, EventArgs args)
snakefoot marked this conversation as resolved.
Show resolved Hide resolved
{
HttpApplication app = sender as HttpApplication;

CaptureRequestPostedBody(
snakefoot marked this conversation as resolved.
Show resolved Hide resolved
app?.Request?.InputStream,
app?.Context?.Items,
Configuration.ShouldCapture(app));
}

/// <summary>
/// Public to be unit testable, HttpContext and HttpRequest are un-mockable
/// unless you are using ASP.NET Core. HttpContext and HttpRequest are sealed
/// and no not have an interface so NSubstitute throws an Exception mocking them.
/// </summary>
/// <param name="bodyStream"></param>
/// <param name="items"></param>
/// <param name="shouldCapture"></param>
public void CaptureRequestPostedBody(
Stream bodyStream,
IDictionary items,
bool shouldCapture)
{
if (bodyStream == null)
{
InternalLogger.Debug("NLogRequestPostedBodyMiddleware: HttpContext.Request.InputStream stream is null");
return;
}

if (!bodyStream.CanRead)
{
InternalLogger.Debug("NLogRequestPostedBodyMiddleware: HttpContext.Request.InputStream stream is non-readable");
return;
}

if (!bodyStream.CanSeek)
{
InternalLogger.Debug("NLogRequestPostedBodyMiddleware: HttpContext.Request.InputStream stream is non-seekable");
return;
}

if (shouldCapture)
{
items[AspNetRequestPostedBodyLayoutRenderer.NLogPostedRequestBodyKey] = GetString(bodyStream);
snakefoot marked this conversation as resolved.
Show resolved Hide resolved
}
else
{
InternalLogger.Debug("NLogRequestPostedBodyMiddleware: ShouldCapture(HttpContext) predicate returned false");
}
}

/// <summary>
/// Reads the posted body stream into a string
/// </summary>
/// <param name="stream"></param>
/// <returns></returns>
protected string GetString(Stream stream)
{
string responseText = null;

// Save away the original stream position
var originalPosition = stream.Position;

// This is required to reset the stream position to the beginning in order to properly read all of the stream.
stream.Position = 0;

#if NET46_OR_GREATER

//This required 5 argument constructor with leaveOpen available was added in 4.5,
//but the project and its unit test project are built for 4.6
//and we should not change the csproj file just for this single class

//If the 4 argument constructor with leaveOpen missing is used, the stream is closed after the
//ReadToEnd() operation completes and the request stream is no longer open for the actual consumer
//This causes the unit test to fail and should cause a failure during actual usage.

using (var streamReader = new StreamReader(
stream,
Configuration.Encoding,
detectEncodingFromByteOrderMarks: Configuration.DetectEncodingFromByteOrderMark,
bufferSize: Configuration.BufferSize,
leaveOpen: true))
{
// This is the most straight forward logic to read the entire body
responseText = streamReader.ReadToEnd();
}

#else
byte[] byteArray = new byte[stream.Length];

using (var ms = new MemoryStream())
{
int read = 0;

while ((read = stream.Read(byteArray, 0, byteArray.Length)) > 0)
{
ms.Write(byteArray, 0, read);
}

responseText = Configuration.Encoding.GetString(ms.ToArray());
}
#endif
// This is required to reset the stream position to the original, in order to
// properly let the next reader process the stream from the original point
stream.Position = originalPosition;

// Return the string of the body
return responseText;
}

/// <summary>
/// This is a no-op
/// </summary>
public void Dispose()
{
// Nothing to be disposed of
}
}
}
Loading