Skip to content

Commit

Permalink
Fix another code smell
Browse files Browse the repository at this point in the history
  • Loading branch information
Burak Akgerman committed Jun 1, 2022
1 parent 4899155 commit 72f70c6
Show file tree
Hide file tree
Showing 11 changed files with 1,197 additions and 39 deletions.
4 changes: 3 additions & 1 deletion src/NLog.Web.AspNetCore/NLog.Web.AspNetCore.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ NLog 5 release post: https://nlog-project.org/2021/08/25/nlog-5-0-preview1-ready
<DefineConstants>$(DefineConstants);ASP_NET_CORE;ASP_NET_CORE3</DefineConstants>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Microsoft.AspNetCore.Http.Extensions" Version="2.1.21" />
<PackageReference Include="Microsoft.IO.RecyclableMemoryStream" Version="2.2.0" />
<PackageReference Include="NLog.Extensions.Logging" Version="5.0.0" />
</ItemGroup>
<ItemGroup Condition=" '$(TargetFramework)' == 'netstandard2.0' or '$(TargetFramework)' == 'net461' ">
Expand All @@ -100,7 +102,7 @@ NLog 5 release post: https://nlog-project.org/2021/08/25/nlog-5-0-preview1-ready
<Compile Include="..\Shared\**\*.cs" />
</ItemGroup>
<ItemGroup>
<Content Include="readme.txt">
<Content Include="readme.txt">
<Pack>true</Pack>
<PackagePath>\</PackagePath>
</Content>
Expand Down
169 changes: 169 additions & 0 deletions src/NLog.Web.AspNetCore/NLogResponseBodyMiddleware.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
using System.IO;
using System.Text;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using Microsoft.IO;
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
///
/// Response body
///
/// The following are saved in the HttpContext.Items collection
///
/// __nlog-aspnet-response-body
///
/// Usage: app.UseMiddleware&lt;NLogResponseBodyMiddleware&gt;(); where app is an IApplicationBuilder
/// Register the NLogBodyMiddlewareOptions in the IoC so that the config gets passed to the constructor
/// Please use with caution, this will temporarily use 2x the memory for the response, so this may be
/// suitable only for responses &lt; 64 KB
/// </summary>
public class NLogResponseBodyMiddleware
{
private readonly RequestDelegate _next;

private readonly NLogResponseBodyMiddlewareOptions _options;

// Using this instead of new MemoryStream() is important to the performance.
// According to the articles, this should be used as a static and not as an instance.
// This will manage a pool of MemoryStream instead of creating a new MemoryStream every response.
private static readonly RecyclableMemoryStreamManager Manager = new RecyclableMemoryStreamManager();

/// <summary>
/// Constructor that takes a configuration
/// </summary>
/// <param name="next"></param>
/// <param name="options"></param>
public NLogResponseBodyMiddleware(RequestDelegate next, NLogResponseBodyMiddlewareOptions options)
{
_next = next;
_options = options;
}

/// <summary>
/// This allows interception of the HTTP pipeline for logging purposes
/// </summary>
/// <param name="context"></param>
/// <returns></returns>
public async Task Invoke(HttpContext context)
{
if (ShouldCaptureResponseBody(context))
{
using (var memoryStream = Manager.GetStream())
{
// Save away the true response stream
var originalStream = context.Response.Body;

// Make the Http Context Response Body refer to the Memory Stream
context.Response.Body = memoryStream;

// The Http Context Response then writes to the Memory Stream
await _next(context).ConfigureAwait(false);

var responseBody = await GetString(memoryStream).ConfigureAwait(false);

// Copy the contents of the memory stream back to the true response stream
await memoryStream.CopyToAsync(originalStream).ConfigureAwait(false);

// This next line enables NLog to log the response
if (!string.IsNullOrEmpty(responseBody) && _options.ShouldRetainCapture(context))
{
context.Items[AspNetResponseBodyLayoutRenderer.NLogResponseBodyKey] = responseBody;
}
}
}
else
{
if (context != null)
{
await _next(context).ConfigureAwait(false);
}
}
}

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

// Perform null checking
if (context.Response == null)
{
InternalLogger.Debug("NLogResponseBodyMiddleware: HttpContext.Response is null");
return false;
}

// Perform null checking
if (context.Response.Body == null)
{
InternalLogger.Debug("NLogResponseBodyMiddleware: HttpContext.Response.Body stream is null");
return false;
}

// If we cannot write the response stream we cannot capture the body
if (!context.Response.Body.CanWrite)
{
InternalLogger.Debug("NLogResponseBodyMiddleware: HttpContext.Response.Body stream is non-writeable");
return false;
}

// Use the predicate in the configuration instance that takes the HttpContext as an argument
if (!_options.ShouldCapture(context))
{
InternalLogger.Debug("NLogResponseBodyMiddleware: _configuration.ShouldCapture(HttpContext) predicate returned false");
return false;
}

return true;
}

/// <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,
Encoding.UTF8,
true,
1024,
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;
}
}
}
63 changes: 63 additions & 0 deletions src/NLog.Web.AspNetCore/NLogResponseBodyMiddlewareOptions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
using System;
using Microsoft.AspNetCore.Http;

namespace NLog.Web
{
/// <summary>
/// Contains the configuration for the NLogRequestPostedBodyMiddleware
/// </summary>
public class NLogResponseBodyMiddlewareOptions
{
/// <summary>
/// The default configuration
/// </summary>
internal static readonly NLogResponseBodyMiddlewareOptions Default = new NLogResponseBodyMiddlewareOptions();

/// <summary>
/// Default Constructor
/// </summary>
public NLogResponseBodyMiddlewareOptions()
{
ShouldRetainCapture = DefaultRetainCapture;
ShouldCapture = DefaultShouldCapture;
}

/// <summary>
/// The maximum response size that will be captured
/// Defaults to 30KB
/// </summary>
public int MaximumResponseSize { get; set; } = 30 * 1024;

/// <summary>
/// If this returns true, the response body will be captured
/// Defaults to true
/// 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; }

/// <summary>
/// The default predicate for ShouldCapture
/// Returns true
/// </summary>
private bool DefaultShouldCapture(HttpContext context)
{
return true;
}

/// <summary>
/// Defaults to true if content length &lt;= 30KB
/// </summary>
public Predicate<HttpContext> ShouldRetainCapture { get; set; }

/// <summary>
/// The default predicate for ShouldCapture
/// Returns true if content length &lt;= 30KB
/// </summary>
private bool DefaultRetainCapture(HttpContext context)
{
return context?.Response?.ContentLength != null && context?.Response?.ContentLength <= MaximumResponseSize;
}
}
}
36 changes: 0 additions & 36 deletions src/Shared/Internal/HttpContextExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,42 +62,6 @@ internal static HttpResponse TryGetResponse(this HttpContext context)
}
#endif

#if ASP_NET_CORE2
internal static string GetString(this ISession session, string key)
{
if (!session.TryGetValue(key, out var data))
{
return null;
}

if (data == null)
{
return null;
}

if (data.Length == 0)
{
return string.Empty;
}

return Encoding.UTF8.GetString(data);
}

public static int? GetInt32(this ISession session, string key)
{
if (!session.TryGetValue(key, out var data))
{
return null;
}

if (data == null || data.Length < 4)
{
return null;
}
return data[0] << 24 | data[1] << 16 | data[2] << 8 | data[3];
}
#endif

#if !ASP_NET_CORE
internal static HttpSessionStateBase TryGetSession(this HttpContextBase context)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ private void SerializePairsFlat(IEnumerable<KeyValuePair<string, string>> pairs,


#endregion

#region Singles

/// <summary>
Expand All @@ -193,7 +193,7 @@ protected void SerializeSingles(IEnumerable<string> values, StringBuilder builde
}
}

private void SerializeSinglesJson(IEnumerable<string> values, StringBuilder builder)
private static void SerializeSinglesJson(IEnumerable<string> values, StringBuilder builder)
{
var firstItem = true;
foreach (var item in values)
Expand Down
58 changes: 58 additions & 0 deletions src/Shared/LayoutRenderers/AspNetResponseBodyLayoutRenderer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
using System.Text;
using NLog.LayoutRenderers;
#if ASP_NET_CORE
using Microsoft.AspNetCore.Http;
#else
using System.Web;
#endif
namespace NLog.Web.LayoutRenderers
{
/// <summary>
/// ASP.NET response body
/// </summary>
[LayoutRenderer("aspnet-response-body")]
public class AspNetResponseBodyLayoutRenderer : AspNetLayoutRendererBase
{

/// <summary>
/// The object for the key in HttpContext.Items for the response body
/// </summary>
internal static readonly object NLogResponseBodyKey = new object();

/// <summary>Renders the ASP.NET response body</summary>
/// <param name="builder">The <see cref="T:System.Text.StringBuilder" /> to append the rendered data to.</param>
/// <param name="logEvent">Logging event.</param>
protected override void DoAppend(StringBuilder builder, LogEventInfo logEvent)
{
var httpContext = HttpContextAccessor.HttpContext;
if (httpContext == null)
{
return;
}

var items = httpContext.Items;
if (items == null)
{
return;
}

if (httpContext.Items.Count == 0)
{
return;
}

#if !ASP_NET_CORE
if (!items.Contains(NLogResponseBodyKey))
{
return;
}
#else
if (!items.ContainsKey(NLogResponseBodyKey))
{
return;
}
#endif
builder.Append(items[NLogResponseBodyKey] as string);
}
}
}
Loading

0 comments on commit 72f70c6

Please sign in to comment.