Skip to content

Commit 5ddd0ea

Browse files
tippmar-nrtippmar-nr
authored
fix: Add defensive coding and exception handling during ASP.NET Core 6+ browser injection. (#2035) (#2038)
1 parent 36376dd commit 5ddd0ea

File tree

8 files changed

+185
-101
lines changed

8 files changed

+185
-101
lines changed

‎src/Agent/NewRelic/Agent/Core/BrowserMonitoring/BrowserScriptInjectionHelper.cs

+11-9
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
11
// Copyright 2020 New Relic, Inc. All rights reserved.
22
// SPDX-License-Identifier: Apache-2.0
33

4-
using System;
54
using System.IO;
65
using System.Threading.Tasks;
76
using NewRelic.Agent.Api;
8-
using NewRelic.Core.Logging;
97

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

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

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

37-
// Write everything up to the insertion index
38-
await baseStream.WriteAsync(buffer, 0, index);
39-
40-
// Write the RUM script
41-
await baseStream.WriteAsync(rumBytes, 0, rumBytes.Length);
39+
// Write the RUM script
40+
await baseStream.WriteAsync(rumBytes, 0, rumBytes.Length);
4241

43-
// Write the rest of the doc, starting after the insertion index
44-
await baseStream.WriteAsync(buffer, index, buffer.Length - index);
42+
// Write the rest of the doc, starting after the insertion index
43+
await baseStream.WriteAsync(buffer, index, buffer.Length - index);
44+
}
45+
else
46+
transaction?.LogFinest($"Skipping RUM Injection: Insertion index was invalid.");
4547
}
4648
}
4749
}

‎src/Agent/NewRelic/Agent/Core/BrowserMonitoring/BrowserScriptInjectionIndexHelper.cs

+45-36
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System;
55
using System.Text;
66
using System.Text.RegularExpressions;
7+
using NewRelic.Core.Logging;
78

89
namespace NewRelic.Agent.Core.BrowserMonitoring
910
{
@@ -23,53 +24,61 @@ internal static class BrowserScriptInjectionIndexHelper
2324
/// </remarks>
2425
public static int TryFindInjectionIndex(byte[] content)
2526
{
26-
var contentAsString = Encoding.UTF8.GetString(content);
27+
try
28+
{
29+
var contentAsString = Encoding.UTF8.GetString(content);
2730

28-
var openingHeadTagIndex = FindFirstOpeningHeadTag(contentAsString);
31+
var openingHeadTagIndex = FindFirstOpeningHeadTag(contentAsString);
2932

30-
// No <HEAD> tag. Attempt to insert before <BODY> tag (not a great fallback option).
31-
if (openingHeadTagIndex == -1)
32-
{
33-
return FindIndexBeforeBodyTag(content, contentAsString);
34-
}
33+
// No <HEAD> tag. Attempt to insert before <BODY> tag (not a great fallback option).
34+
if (openingHeadTagIndex == -1)
35+
{
36+
return FindIndexBeforeBodyTag(content, contentAsString);
37+
}
3538

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

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

43-
// Find which of the two tags occurs latest (if at all) and ensure that at least
44-
// one of the matches occurs prior to the closing head tag
45-
if ((xUaCompatibleFilterMatch.Success || charsetFilterMatch.Success) &&
46-
(xUaCompatibleFilterMatch.Index < closingHeadTagIndex || charsetFilterMatch.Index < closingHeadTagIndex))
47-
{
48-
var match = charsetFilterMatch;
49-
if (xUaCompatibleFilterMatch.Index > charsetFilterMatch.Index)
46+
// Find which of the two tags occurs latest (if at all) and ensure that at least
47+
// one of the matches occurs prior to the closing head tag
48+
if ((xUaCompatibleFilterMatch.Success || charsetFilterMatch.Success) &&
49+
(xUaCompatibleFilterMatch.Index < closingHeadTagIndex || charsetFilterMatch.Index < closingHeadTagIndex))
5050
{
51-
match = xUaCompatibleFilterMatch;
52-
}
51+
var match = charsetFilterMatch;
52+
if (xUaCompatibleFilterMatch.Index > charsetFilterMatch.Index)
53+
{
54+
match = xUaCompatibleFilterMatch;
55+
}
5356

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

58-
return utf8HeadMatchIndex + substringBytesLength;
59-
}
61+
return utf8HeadMatchIndex + substringBytesLength;
62+
}
6063

61-
// found opening head tag but no meta tags, insert immediately after the opening head tag
62-
// Find first '>' after the opening head tag, which will be end of head opening tag.
63-
var indexOfEndHeadOpeningTag = contentAsString.IndexOf('>', openingHeadTagIndex);
64+
// found opening head tag but no meta tags, insert immediately after the opening head tag
65+
// Find first '>' after the opening head tag, which will be end of head opening tag.
66+
var indexOfEndHeadOpeningTag = contentAsString.IndexOf('>', openingHeadTagIndex);
6467

65-
// The <HEAD> tag may be malformed or simply be another type of tag, if so do not use it
66-
if (!(indexOfEndHeadOpeningTag > openingHeadTagIndex))
67-
return -1;
68+
// The <HEAD> tag may be malformed or simply be another type of tag, if so do not use it
69+
if (!(indexOfEndHeadOpeningTag > openingHeadTagIndex))
70+
return -1;
6871

69-
// Get the whole open HEAD tag string
70-
var headOpeningTag = contentAsString.Substring(openingHeadTagIndex, (indexOfEndHeadOpeningTag - openingHeadTagIndex) + 1);
71-
var utf8HeadOpeningTagIndex = IndexOfByteArray(content, headOpeningTag, out var headOpeningTagBytesLength);
72-
return utf8HeadOpeningTagIndex + headOpeningTagBytesLength;
72+
// Get the whole open HEAD tag string
73+
var headOpeningTag = contentAsString.Substring(openingHeadTagIndex, (indexOfEndHeadOpeningTag - openingHeadTagIndex) + 1);
74+
var utf8HeadOpeningTagIndex = IndexOfByteArray(content, headOpeningTag, out var headOpeningTagBytesLength);
75+
return utf8HeadOpeningTagIndex + headOpeningTagBytesLength;
76+
}
77+
catch (Exception e)
78+
{
79+
Log.LogMessage(LogLevel.Error, e, "Unexpected exception in TryFindInjectionIndex().");
80+
return -1;
81+
}
7382
}
7483

7584
private static int FindIndexBeforeBodyTag(byte[] content, string contentAsString)

‎src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AspNetCore6Plus/BrowserInjectingStreamWrapper.cs

+79-46
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System.Threading.Tasks;
88
using Microsoft.AspNetCore.Http;
99
using NewRelic.Agent.Api;
10+
using NewRelic.Agent.Extensions.Logging;
1011

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

33+
/// <summary>
34+
/// Flag gets set to true if we've captured an exception and need to disable browser injection
35+
/// </summary>
36+
public static bool Disabled { get; set; }
37+
3238
public override Task FlushAsync(CancellationToken cancellationToken)
3339
{
34-
if (!_isContentLengthSet && IsHtmlResponse())
40+
if (!Disabled && !_isContentLengthSet && IsHtmlResponse())
3541
{
36-
_context.Response.Headers.ContentLength = null;
42+
_context.Response.ContentLength = null;
3743
_isContentLengthSet = true;
3844
}
3945

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

53-
IsHtmlResponse(forceReCheck: true);
59+
if (!Disabled)
60+
IsHtmlResponse(forceReCheck: true);
5461
}
5562

5663
public override void Write(ReadOnlySpan<byte> buffer) => _baseStream.Write(buffer);
@@ -62,13 +69,19 @@ public override void Write(byte[] buffer, int offset, int count)
6269
{
6370
// pass through without modification if we're already in the middle of injecting
6471
// don't inject if the response isn't an HTML response
65-
if (!CurrentlyInjecting() && IsHtmlResponse())
72+
if (!Disabled && !CurrentlyInjecting() && IsHtmlResponse())
6673
{
67-
// Set a flag on the context to indicate we're in the middle of injecting - prevents multiple recursions when response compression is in use
68-
StartInjecting();
69-
_agent.TryInjectBrowserScriptAsync(_context.Response.ContentType, _context.Request.Path.Value, buffer, _baseStream)
70-
.GetAwaiter().GetResult();
71-
FinishInjecting();
74+
try
75+
{
76+
// Set a flag on the context to indicate we're in the middle of injecting - prevents multiple recursions when response compression is in use
77+
StartInjecting();
78+
_agent.TryInjectBrowserScriptAsync(_context.Response.ContentType, _context.Request.Path.Value, buffer, _baseStream)
79+
.GetAwaiter().GetResult();
80+
}
81+
finally
82+
{
83+
FinishInjecting();
84+
}
7285

7386
return;
7487
}
@@ -82,12 +95,18 @@ public override async ValueTask WriteAsync(ReadOnlyMemory<byte> buffer, Cancella
8295
{
8396
// pass through without modification if we're already in the middle of injecting
8497
// don't inject if the response isn't an HTML response
85-
if (!CurrentlyInjecting() && IsHtmlResponse())
98+
if (!Disabled && !CurrentlyInjecting() && IsHtmlResponse())
8699
{
87-
// Set a flag on the context to indicate we're in the middle of injecting - prevents multiple recursions when response compression is in use
88-
StartInjecting();
89-
await _agent.TryInjectBrowserScriptAsync(_context.Response.ContentType, _context.Request.Path.Value, buffer.ToArray(), _baseStream);
90-
FinishInjecting();
100+
try
101+
{
102+
// Set a flag on the context to indicate we're in the middle of injecting - prevents multiple recursions when response compression is in use
103+
StartInjecting();
104+
await _agent.TryInjectBrowserScriptAsync(_context.Response.ContentType, _context.Request.Path.Value, buffer.ToArray(), _baseStream);
105+
}
106+
finally
107+
{
108+
FinishInjecting();
109+
}
91110

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

99118
private const string InjectingRUM = "InjectingRUM";
100119

101-
private void FinishInjecting() => _context.Items.Remove(InjectingRUM);
102-
private void StartInjecting() => _context.Items.Add(InjectingRUM, null);
103-
private bool CurrentlyInjecting() => _context.Items.ContainsKey(InjectingRUM);
120+
private void FinishInjecting() => _context?.Items.Remove(InjectingRUM);
121+
private void StartInjecting() => _context?.Items.Add(InjectingRUM, null);
122+
private bool CurrentlyInjecting() => _context?.Items.ContainsKey(InjectingRUM) ?? false;
104123

105124
public override async ValueTask DisposeAsync()
106125
{
@@ -120,41 +139,55 @@ public override async ValueTask DisposeAsync()
120139

121140
private bool IsHtmlResponse(bool forceReCheck = false)
122141
{
123-
if (!forceReCheck && _isHtmlResponse != null)
124-
return _isHtmlResponse.Value;
125-
126-
// we need to check if the active request is still valid
127-
// this can fail if we're in the middle of an error response
128-
// or url rewrite in which case we can't intercept
129-
if (_context?.Response == null)
130-
return false;
131-
132-
// Requirements for script injection:
133-
// * text/html response
134-
// * UTF-8 formatted (either explicitly or no charset defined)
135-
136-
_isHtmlResponse =
137-
_context.Response.ContentType.Contains("text/html", StringComparison.OrdinalIgnoreCase) &&
138-
(_context.Response.ContentType.Contains("utf-8", StringComparison.OrdinalIgnoreCase) ||
139-
!_context.Response.ContentType.Contains("charset=", StringComparison.OrdinalIgnoreCase));
140-
141-
if (!_isHtmlResponse.Value)
142+
try
142143
{
143-
_agent.CurrentTransaction?.LogFinest($"Skipping RUM injection: Not an HTML response. ContentType is {_context.Response.ContentType}");
144-
return false;
144+
if (!forceReCheck && _isHtmlResponse != null)
145+
return _isHtmlResponse.Value;
146+
147+
// we need to check if the active request is still valid
148+
// this can fail if we're in the middle of an error response
149+
// or url rewrite in which case we can't intercept
150+
if (_context?.Response == null)
151+
return false;
152+
153+
// Requirements for script injection:
154+
// * text/html response
155+
// * UTF-8 formatted (either explicitly or no charset defined)
156+
_isHtmlResponse =
157+
_context.Response.ContentType != null &&
158+
_context.Response.ContentType.Contains("text/html", StringComparison.OrdinalIgnoreCase) &&
159+
(_context.Response.ContentType.Contains("utf-8", StringComparison.OrdinalIgnoreCase) ||
160+
!_context.Response.ContentType.Contains("charset=", StringComparison.OrdinalIgnoreCase));
161+
162+
if (!_isHtmlResponse.Value)
163+
{
164+
_agent.CurrentTransaction?.LogFinest($"Skipping RUM injection: Not an HTML response. ContentType is {_context.Response.ContentType}");
165+
return false;
166+
}
167+
168+
// Make sure we force dynamic content type since we're
169+
// rewriting the content - static content will set the header explicitly
170+
// and fail when it doesn't match if (_isHtmlResponse.Value)
171+
if (!_isContentLengthSet && _context.Response.ContentLength != null)
172+
{
173+
_context.Response.ContentLength = null;
174+
_isContentLengthSet = true;
175+
}
145176
}
146-
147-
// Make sure we force dynamic content type since we're
148-
// rewriting the content - static content will set the header explicitly
149-
// and fail when it doesn't match if (_isHtmlResponse.Value)
150-
if (!_isContentLengthSet && _context.Response.ContentLength != null)
177+
catch (Exception e)
151178
{
152-
_context.Response.Headers.ContentLength = null;
153-
_isContentLengthSet = true;
179+
LogExceptionAndDisable(e);
154180
}
155181

156-
return _isHtmlResponse.Value;
182+
return _isHtmlResponse ?? false;
157183
}
158184

185+
private void LogExceptionAndDisable(Exception e)
186+
{
187+
_agent.Logger.Log(Level.Error,
188+
$"Unexpected exception. Browser injection will be disabled. Exception: {e.Message}: {e.StackTrace}");
189+
190+
Disabled = true;
191+
}
159192
}
160193
}

‎src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AspNetCore6Plus/BrowserInjectionMiddleware.cs

+6-3
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,12 @@ public BrowserInjectionMiddleware(RequestDelegate next, IAgent agent)
2121

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

2831
return _next(context);
2932
}

‎tests/Agent/IntegrationTests/Applications/BasicAspNetCoreRazorApplication/Program.cs

+18
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Copyright 2020 New Relic, Inc. All rights reserved.
22
// SPDX-License-Identifier: Apache-2.0
33

4+
using System.Text.Json;
5+
using System.Text.Json.Serialization;
46
using ApplicationLifecycle;
57

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

4547
app.MapRazorPages();
4648

49+
app.MapGet("/foo", async context =>
50+
{
51+
var subscriptions = new
52+
{
53+
Foo = 1, Bar = "Something"
54+
};
55+
56+
await context.Response.WriteAsync(JsonSerializer.Serialize(subscriptions,
57+
new JsonSerializerOptions
58+
{
59+
PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
60+
DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull
61+
}));
62+
});
63+
64+
4765
app.Urls.Add($"http://*:{_port}");
4866

4967
var task = app.RunAsync(ct.Token);

‎tests/Agent/IntegrationTests/IntegrationTestHelpers/JavaScriptAgent.cs

+4
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44

55
using System.Collections.Generic;
6+
using System.Linq;
67
using System.Text.RegularExpressions;
78
using Newtonsoft.Json;
89
using Xunit;
@@ -24,6 +25,9 @@ public static string GetJavaScriptAgentScriptFromSource(string source)
2425
{
2526
const string regex = @"<script type=""text/javascript"">(.*?)</script>";
2627
var matches = Regex.Matches(source, regex, RegexOptions.Singleline);
28+
if (matches.Count <= 0)
29+
return null;
30+
2731
var match = matches[1]; //specifically look for the 2nd match. The first match contains settings. The 2nd match contains the actual browser agent.
2832
Assert.True(match.Success, "Did not find a match for the JavaScript agent config in the provided page.");
2933
return match.Groups[1].Value;

0 commit comments

Comments
 (0)