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.

Sign up for GitHub

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

Jump to bottom

fix: Defensive coding and exception handling during ASP.NET Core 6+ browser injection #2038

Merged
merged 2 commits into from
Nov 8, 2023
Merged

fix: Defensive coding and exception handling during ASP.NET Core 6+ browser injection #2038

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3429f82
fix: Add defensive coding and exception handling during ASP.NET Core …
tippmar-nr Nov 8, 2023
a5da07f
integration test cleanup
tippmar-nr Nov 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 11 additions & 9 deletions src/Agent/NewRelic/Agent/Core/BrowserMonitoring/BrowserScriptInjectionHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
// Copyright 2020 New Relic, Inc. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

using System;
using System.IO;
using System.Threading.Tasks;
using NewRelic.Agent.Api;
using NewRelic.Core.Logging;

namespace NewRelic.Agent.Core.BrowserMonitoring
{
Expand Down Expand Up @@ -33,15 +31,19 @@ public static async Task InjectBrowserScriptAsync(byte[] buffer, Stream baseStre

transaction?.LogFinest($"Injecting RUM script at byte index {index}.");

if (index < buffer.Length) // validate index is less than buffer length
{
// Write everything up to the insertion index
await baseStream.WriteAsync(buffer, 0, index);

// Write everything up to the insertion index
await baseStream.WriteAsync(buffer, 0, index);

// Write the RUM script
await baseStream.WriteAsync(rumBytes, 0, rumBytes.Length);
// Write the RUM script
await baseStream.WriteAsync(rumBytes, 0, rumBytes.Length);

// Write the rest of the doc, starting after the insertion index
await baseStream.WriteAsync(buffer, index, buffer.Length - index);
// Write the rest of the doc, starting after the insertion index
await baseStream.WriteAsync(buffer, index, buffer.Length - index);
}
else
transaction?.LogFinest($"Skipping RUM Injection: Insertion index was invalid.");
}
}
}
81 changes: 45 additions & 36 deletions src/Agent/NewRelic/Agent/Core/BrowserMonitoring/BrowserScriptInjectionIndexHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Text;
using System.Text.RegularExpressions;
using NewRelic.Core.Logging;

namespace NewRelic.Agent.Core.BrowserMonitoring
{
Expand All @@ -23,53 +24,61 @@ internal static class BrowserScriptInjectionIndexHelper
/// </remarks>
public static int TryFindInjectionIndex(byte[] content)
{
var contentAsString = Encoding.UTF8.GetString(content);
try
{
var contentAsString = Encoding.UTF8.GetString(content);

var openingHeadTagIndex = FindFirstOpeningHeadTag(contentAsString);
var openingHeadTagIndex = FindFirstOpeningHeadTag(contentAsString);

// No <HEAD> tag. Attempt to insert before <BODY> tag (not a great fallback option).
if (openingHeadTagIndex == -1)
{
return FindIndexBeforeBodyTag(content, contentAsString);
}
// No <HEAD> tag. Attempt to insert before <BODY> tag (not a great fallback option).
if (openingHeadTagIndex == -1)
{
return FindIndexBeforeBodyTag(content, contentAsString);
}

// Since we have a head tag (top of 'page'), search for <X_UA_COMPATIBLE> and for <CHARSET> tags in Head section
var xUaCompatibleFilterMatch = XUaCompatibleFilter.Match(contentAsString, openingHeadTagIndex);
var charsetFilterMatch = CharsetFilter.Match(contentAsString, openingHeadTagIndex);
// Since we have a head tag (top of 'page'), search for <X_UA_COMPATIBLE> and for <CHARSET> tags in Head section
var xUaCompatibleFilterMatch = XUaCompatibleFilter.Match(contentAsString, openingHeadTagIndex);
var charsetFilterMatch = CharsetFilter.Match(contentAsString, openingHeadTagIndex);

// Try to find </HEAD> tag. (It's okay if we don't find it!)
var closingHeadTagIndex = contentAsString.IndexOf("</head>", StringComparison.InvariantCultureIgnoreCase);
// Try to find </HEAD> tag. (It's okay if we don't find it!)
var closingHeadTagIndex = contentAsString.IndexOf("</head>", StringComparison.InvariantCultureIgnoreCase);

// Find which of the two tags occurs latest (if at all) and ensure that at least
// one of the matches occurs prior to the closing head tag
if ((xUaCompatibleFilterMatch.Success || charsetFilterMatch.Success) &&
(xUaCompatibleFilterMatch.Index < closingHeadTagIndex || charsetFilterMatch.Index < closingHeadTagIndex))
{
var match = charsetFilterMatch;
if (xUaCompatibleFilterMatch.Index > charsetFilterMatch.Index)
// Find which of the two tags occurs latest (if at all) and ensure that at least
// one of the matches occurs prior to the closing head tag
if ((xUaCompatibleFilterMatch.Success || charsetFilterMatch.Success) &&
(xUaCompatibleFilterMatch.Index < closingHeadTagIndex || charsetFilterMatch.Index < closingHeadTagIndex))
{
match = xUaCompatibleFilterMatch;
}
var match = charsetFilterMatch;
if (xUaCompatibleFilterMatch.Index > charsetFilterMatch.Index)
{
match = xUaCompatibleFilterMatch;
}

// find the index just after the end of the regex match in the UTF-8 buffer
var contentSubString = contentAsString.Substring(match.Index, match.Length);
var utf8HeadMatchIndex = IndexOfByteArray(content, contentSubString, out var substringBytesLength);
// find the index just after the end of the regex match in the UTF-8 buffer
var contentSubString = contentAsString.Substring(match.Index, match.Length);
var utf8HeadMatchIndex = IndexOfByteArray(content, contentSubString, out var substringBytesLength);

return utf8HeadMatchIndex + substringBytesLength;
}
return utf8HeadMatchIndex + substringBytesLength;
}

// found opening head tag but no meta tags, insert immediately after the opening head tag
// Find first '>' after the opening head tag, which will be end of head opening tag.
var indexOfEndHeadOpeningTag = contentAsString.IndexOf('>', openingHeadTagIndex);
// found opening head tag but no meta tags, insert immediately after the opening head tag
// Find first '>' after the opening head tag, which will be end of head opening tag.
var indexOfEndHeadOpeningTag = contentAsString.IndexOf('>', openingHeadTagIndex);

// The <HEAD> tag may be malformed or simply be another type of tag, if so do not use it
if (!(indexOfEndHeadOpeningTag > openingHeadTagIndex))
return -1;
// The <HEAD> tag may be malformed or simply be another type of tag, if so do not use it
if (!(indexOfEndHeadOpeningTag > openingHeadTagIndex))
return -1;

// Get the whole open HEAD tag string
var headOpeningTag = contentAsString.Substring(openingHeadTagIndex, (indexOfEndHeadOpeningTag - openingHeadTagIndex) + 1);
var utf8HeadOpeningTagIndex = IndexOfByteArray(content, headOpeningTag, out var headOpeningTagBytesLength);
return utf8HeadOpeningTagIndex + headOpeningTagBytesLength;
// Get the whole open HEAD tag string
var headOpeningTag = contentAsString.Substring(openingHeadTagIndex, (indexOfEndHeadOpeningTag - openingHeadTagIndex) + 1);
var utf8HeadOpeningTagIndex = IndexOfByteArray(content, headOpeningTag, out var headOpeningTagBytesLength);
return utf8HeadOpeningTagIndex + headOpeningTagBytesLength;
}
catch (Exception e)
{
Log.LogMessage(LogLevel.Error, e, "Unexpected exception in TryFindInjectionIndex().");
return -1;
}
}

private static int FindIndexBeforeBodyTag(byte[] content, string contentAsString)
Expand Down
125 changes: 79 additions & 46 deletions ...Relic/Agent/Extensions/Providers/Wrapper/AspNetCore6Plus/BrowserInjectingStreamWrapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using NewRelic.Agent.Api;
using NewRelic.Agent.Extensions.Logging;

namespace NewRelic.Providers.Wrapper.AspNetCore6Plus
{
Expand All @@ -29,11 +30,16 @@ public BrowserInjectingStreamWrapper(IAgent agent, Stream baseStream, HttpContex
CanWrite = true;
}

/// <summary>
/// Flag gets set to true if we've captured an exception and need to disable browser injection
/// </summary>
public static bool Disabled { get; set; }

public override Task FlushAsync(CancellationToken cancellationToken)
{
if (!_isContentLengthSet && IsHtmlResponse())
if (!Disabled && !_isContentLengthSet && IsHtmlResponse())
{
_context.Response.Headers.ContentLength = null;
_context.Response.ContentLength = null;
_isContentLengthSet = true;
}

Expand All @@ -50,7 +56,8 @@ public override void SetLength(long value)
{
_baseStream.SetLength(value);

IsHtmlResponse(forceReCheck: true);
if (!Disabled)
IsHtmlResponse(forceReCheck: true);
}

public override void Write(ReadOnlySpan<byte> buffer) => _baseStream.Write(buffer);
Expand All @@ -62,13 +69,19 @@ public override void Write(byte[] buffer, int offset, int count)
{
// pass through without modification if we're already in the middle of injecting
// don't inject if the response isn't an HTML response
if (!CurrentlyInjecting() && IsHtmlResponse())
if (!Disabled && !CurrentlyInjecting() && IsHtmlResponse())
{
// Set a flag on the context to indicate we're in the middle of injecting - prevents multiple recursions when response compression is in use
StartInjecting();
_agent.TryInjectBrowserScriptAsync(_context.Response.ContentType, _context.Request.Path.Value, buffer, _baseStream)
.GetAwaiter().GetResult();
FinishInjecting();
try
{
// Set a flag on the context to indicate we're in the middle of injecting - prevents multiple recursions when response compression is in use
StartInjecting();
_agent.TryInjectBrowserScriptAsync(_context.Response.ContentType, _context.Request.Path.Value, buffer, _baseStream)
.GetAwaiter().GetResult();
}
finally
{
FinishInjecting();
}

return;
}
Expand All @@ -82,12 +95,18 @@ public override async ValueTask WriteAsync(ReadOnlyMemory<byte> buffer, Cancella
{
// pass through without modification if we're already in the middle of injecting
// don't inject if the response isn't an HTML response
if (!CurrentlyInjecting() && IsHtmlResponse())
if (!Disabled && !CurrentlyInjecting() && IsHtmlResponse())
{
// Set a flag on the context to indicate we're in the middle of injecting - prevents multiple recursions when response compression is in use
StartInjecting();
await _agent.TryInjectBrowserScriptAsync(_context.Response.ContentType, _context.Request.Path.Value, buffer.ToArray(), _baseStream);
FinishInjecting();
try
{
// Set a flag on the context to indicate we're in the middle of injecting - prevents multiple recursions when response compression is in use
StartInjecting();
await _agent.TryInjectBrowserScriptAsync(_context.Response.ContentType, _context.Request.Path.Value, buffer.ToArray(), _baseStream);
}
finally
{
FinishInjecting();
}

return;
}
Expand All @@ -98,9 +117,9 @@ public override async ValueTask WriteAsync(ReadOnlyMemory<byte> buffer, Cancella

private const string InjectingRUM = "InjectingRUM";

private void FinishInjecting() => _context.Items.Remove(InjectingRUM);
private void StartInjecting() => _context.Items.Add(InjectingRUM, null);
private bool CurrentlyInjecting() => _context.Items.ContainsKey(InjectingRUM);
private void FinishInjecting() => _context?.Items.Remove(InjectingRUM);
private void StartInjecting() => _context?.Items.Add(InjectingRUM, null);
private bool CurrentlyInjecting() => _context?.Items.ContainsKey(InjectingRUM) ?? false;

public override async ValueTask DisposeAsync()
{
Expand All @@ -120,41 +139,55 @@ public override async ValueTask DisposeAsync()

private bool IsHtmlResponse(bool forceReCheck = false)
{
if (!forceReCheck && _isHtmlResponse != null)
return _isHtmlResponse.Value;

// we need to check if the active request is still valid
// this can fail if we're in the middle of an error response
// or url rewrite in which case we can't intercept
if (_context?.Response == null)
return false;

// Requirements for script injection:
// * text/html response
// * UTF-8 formatted (either explicitly or no charset defined)

_isHtmlResponse =
_context.Response.ContentType.Contains("text/html", StringComparison.OrdinalIgnoreCase) &&
(_context.Response.ContentType.Contains("utf-8", StringComparison.OrdinalIgnoreCase) ||
!_context.Response.ContentType.Contains("charset=", StringComparison.OrdinalIgnoreCase));

if (!_isHtmlResponse.Value)
try
{
_agent.CurrentTransaction?.LogFinest($"Skipping RUM injection: Not an HTML response. ContentType is {_context.Response.ContentType}");
return false;
if (!forceReCheck && _isHtmlResponse != null)
return _isHtmlResponse.Value;

// we need to check if the active request is still valid
// this can fail if we're in the middle of an error response
// or url rewrite in which case we can't intercept
if (_context?.Response == null)
return false;

// Requirements for script injection:
// * text/html response
// * UTF-8 formatted (either explicitly or no charset defined)
_isHtmlResponse =
_context.Response.ContentType != null &&
_context.Response.ContentType.Contains("text/html", StringComparison.OrdinalIgnoreCase) &&
(_context.Response.ContentType.Contains("utf-8", StringComparison.OrdinalIgnoreCase) ||
!_context.Response.ContentType.Contains("charset=", StringComparison.OrdinalIgnoreCase));

if (!_isHtmlResponse.Value)
{
_agent.CurrentTransaction?.LogFinest($"Skipping RUM injection: Not an HTML response. ContentType is {_context.Response.ContentType}");
return false;
}

// Make sure we force dynamic content type since we're
// rewriting the content - static content will set the header explicitly
// and fail when it doesn't match if (_isHtmlResponse.Value)
if (!_isContentLengthSet && _context.Response.ContentLength != null)
{
_context.Response.ContentLength = null;
_isContentLengthSet = true;
}
}

// Make sure we force dynamic content type since we're
// rewriting the content - static content will set the header explicitly
// and fail when it doesn't match if (_isHtmlResponse.Value)
if (!_isContentLengthSet && _context.Response.ContentLength != null)
catch (Exception e)
{
_context.Response.Headers.ContentLength = null;
_isContentLengthSet = true;
LogExceptionAndDisable(e);
}

return _isHtmlResponse.Value;
return _isHtmlResponse ?? false;
}

private void LogExceptionAndDisable(Exception e)
{
_agent.Logger.Log(Level.Error,
jaffinito marked this conversation as resolved.
Show resolved Hide resolved
$"Unexpected exception. Browser injection will be disabled. Exception: {e.Message}: {e.StackTrace}");

Disabled = true;
}
}
}
9 changes: 6 additions & 3 deletions ...NewRelic/Agent/Extensions/Providers/Wrapper/AspNetCore6Plus/BrowserInjectionMiddleware.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@ public BrowserInjectionMiddleware(RequestDelegate next, IAgent agent)

public Task Invoke(HttpContext context)
{
// wrap the response body in our stream wrapper which will inject the RUM script if appropriate
using var injectedResponse = new BrowserInjectingStreamWrapper(_agent, context.Response.Body, context);
context.Features.Set<IHttpResponseBodyFeature>(new StreamResponseBodyFeature(injectedResponse));
if (!BrowserInjectingStreamWrapper.Disabled)
{
// wrap the response body in our stream wrapper which will inject the RUM script if appropriate
using var injectedResponse = new BrowserInjectingStreamWrapper(_agent, context.Response.Body, context);
context.Features.Set<IHttpResponseBodyFeature>(new StreamResponseBodyFeature(injectedResponse));
}

return _next(context);
}
Expand Down
18 changes: 18 additions & 0 deletions tests/Agent/IntegrationTests/Applications/BasicAspNetCoreRazorApplication/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Copyright 2020 New Relic, Inc. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

using System.Text.Json;
using System.Text.Json.Serialization;
using ApplicationLifecycle;

namespace BasicAspNetCoreRazorApplication
Expand Down Expand Up @@ -44,6 +46,22 @@ public static async Task Main(string[] args)

app.MapRazorPages();

app.MapGet("/foo", async context =>
{
var subscriptions = new
{
Foo = 1, Bar = "Something"
};

await context.Response.WriteAsync(JsonSerializer.Serialize(subscriptions,
new JsonSerializerOptions
{
PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull
}));
});


app.Urls.Add($"http://*:{_port}");

var task = app.RunAsync(ct.Token);
Expand Down
4 changes: 4 additions & 0 deletions tests/Agent/IntegrationTests/IntegrationTestHelpers/JavaScriptAgent.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@


using System.Collections.Generic;
using System.Linq;
using System.Text.RegularExpressions;
using Newtonsoft.Json;
using Xunit;
Expand All @@ -24,6 +25,9 @@ public static string GetJavaScriptAgentScriptFromSource(string source)
{
const string regex = @"<script type=""text/javascript"">(.*?)</script>";
var matches = Regex.Matches(source, regex, RegexOptions.Singleline);
if (matches.Count <= 0)
return null;

var match = matches[1]; //specifically look for the 2nd match. The first match contains settings. The 2nd match contains the actual browser agent.
Assert.True(match.Success, "Did not find a match for the JavaScript agent config in the provided page.");
return match.Groups[1].Value;
Expand Down