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

Better checking for availability of session #436

Merged
merged 3 commits into from
Jun 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
97 changes: 97 additions & 0 deletions src/Shared/Internal/HttpContextExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
using System;
#if !ASP_NET_CORE
using System.Web;
#else
using System.Text;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Features;
#endif
using NLog.Common;

namespace NLog.Web.Internal
304NotModified marked this conversation as resolved.
Show resolved Hide resolved
{
internal static class HttpContextExtensions
{
#if !ASP_NET_CORE
internal static HttpRequestBase TryGetRequest(this HttpContextBase context)
{
try
{
var request = context?.Request;
if (request == null)
InternalLogger.Debug("HttpContext Request Lookup returned null");
return request;
}
catch (HttpException ex)
{
InternalLogger.Debug(ex, "HttpContext Request Lookup failed.");
return null;
}
}
#else
internal static HttpRequest TryGetRequest(this HttpContext context)
{
var request = context?.Request;
if (request == null)
InternalLogger.Debug("HttpContext Request Lookup returned null");
return request;
}
#endif

#if ASP_NET_CORE
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);
}
#endif

#if !ASP_NET_CORE
internal static HttpSessionStateBase TryGetSession(this HttpContextBase context)
{
var session = context?.Session;
if (session == null)
InternalLogger.Debug("HttpContext Session Lookup returned null");
return session;
}
#else
internal static ISession TryGetSession(this HttpContext context)
{
try
{
if (context?.Features.Get<ISessionFeature>()?.Session != null)
{
var session = context?.Session;
if (session == null)
InternalLogger.Debug("HttpContext Session Lookup returned null");
return session;
}
else
{
InternalLogger.Debug("HttpContext Session Feature not available");
return null;
}
}
catch (InvalidOperationException ex)
{
InternalLogger.Debug(ex, "HttpContext Session Lookup failed.");
return null; // System.InvalidOperationException: Session has not been configured for this application or request.
}
}
#endif
}
}
57 changes: 0 additions & 57 deletions src/Shared/Internal/RequestAccessorExtension.cs

This file was deleted.

12 changes: 5 additions & 7 deletions src/Shared/LayoutRenderers/AspNetSessionIdLayoutRenderer.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
using System.Text;
using NLog.Common;
using NLog.Config;
using NLog.LayoutRenderers;
using NLog.Web.Internal;

#if !ASP_NET_CORE
using System.Web;
Expand All @@ -24,16 +24,14 @@ public class AspNetSessionIdLayoutRenderer : AspNetLayoutRendererBase
protected override void DoAppend(StringBuilder builder, LogEventInfo logEvent)
{
var context = HttpContextAccessor.HttpContext;
if (context?.Session == null)
{
InternalLogger.Debug("aspnet-sessionid - HttpContext Session is null");
var contextSession = context.TryGetSession();
if (contextSession == null)
return;
}

#if !ASP_NET_CORE
builder.Append(context.Session.SessionID);
builder.Append(contextSession.SessionID);
#else
builder.Append(context.Session.Id);
builder.Append(contextSession.Id);
#endif
}
}
Expand Down
27 changes: 5 additions & 22 deletions src/Shared/LayoutRenderers/AspNetSessionValueLayoutRenderer.cs
Original file line number Diff line number Diff line change
@@ -1,16 +1,11 @@
using System;
using System.Globalization;
using System.Linq;
using System.Text;
using NLog.Common;
using NLog.Config;
using NLog.LayoutRenderers;
using NLog.Web.Internal;
#if !ASP_NET_CORE
using System.Web;
#else
using Microsoft.AspNetCore.Http.Features;

#endif

namespace NLog.Web.LayoutRenderers
Expand Down Expand Up @@ -88,23 +83,16 @@ protected override void DoAppend(StringBuilder builder, LogEventInfo logEvent)
}

var context = HttpContextAccessor.HttpContext;
if (context?.Session == null)
{
InternalLogger.Trace("aspnet-session - HttpContext Session is null");
var contextSession = context.TryGetSession();
if (contextSession == null)
return;
}

#if !ASP_NET_CORE
var value = PropertyReader.GetValue(Variable, context.Session, (session,key) => session.Count > 0 ? session[key] : null, EvaluateAsNestedProperties);
var value = PropertyReader.GetValue(Variable, contextSession, (session,key) => session.Count > 0 ? session[key] : null, EvaluateAsNestedProperties);
#else
//because session.get / session.getstring also creating log messages in some cases, this could lead to stackoverflow issues.
//We remember on the context.Items that we are looking up a session value so we prevent stackoverflows
if (context.Items == null || context.Features.Get<ISessionFeature>()?.Session == null)
{
return;
}

if (context.Items.Count > 0 && context.Items.ContainsKey(NLogRetrievingSessionValue))
if (context.Items == null || (context.Items.Count > 0 && context.Items.ContainsKey(NLogRetrievingSessionValue)))
{
return;
}
Expand All @@ -114,12 +102,7 @@ protected override void DoAppend(StringBuilder builder, LogEventInfo logEvent)
object value;
try
{
value = PropertyReader.GetValue(Variable, context.Session, (session, key) => session.GetString(key), EvaluateAsNestedProperties);
}
catch (Exception ex)
{
InternalLogger.Warn(ex, "aspnet-session - Retrieving session value failed.");
return;
value = PropertyReader.GetValue(Variable, contextSession, (session, key) => session.GetString(key), EvaluateAsNestedProperties);
}
finally
{
Expand Down