Skip to content

Commit 5f5cf12

Browse files
authored
fix: Add more null checks to Asp.NET Core 6+ browser instrumentation logic (#3070)
1 parent a6c6d7d commit 5f5cf12

File tree

3 files changed

+27
-26
lines changed

3 files changed

+27
-26
lines changed

src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AspNetCore6Plus/AddNewRelicStartupFilter.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public Action<IApplicationBuilder> Configure(Action<IApplicationBuilder> next)
3232

3333
// only inject the middleware if browser injection is enabled and the request is not a gRPC request.
3434
builder.UseWhen(
35-
context => _agent.Configuration.BrowserMonitoringAutoInstrument && _agent.Configuration.EnableAspNetCore6PlusBrowserInjection && context.Request.ContentType?.ToLower() != "application/grpc",
35+
context => _agent.Configuration.BrowserMonitoringAutoInstrument && _agent.Configuration.EnableAspNetCore6PlusBrowserInjection && context.Request?.ContentType?.ToLower() != "application/grpc",
3636
b => b.UseMiddleware<BrowserInjectionMiddleware>(_agent));
3737

3838
next(builder);

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

+24-22
Original file line numberDiff line numberDiff line change
@@ -37,46 +37,45 @@ public BrowserInjectingStreamWrapper(IAgent agent, Stream baseStream, HttpContex
3737

3838
public override Task FlushAsync(CancellationToken cancellationToken)
3939
{
40-
if (!Disabled && !_isContentLengthSet && IsHtmlResponse())
40+
if (_context is { Response: not null } && !Disabled && !_isContentLengthSet && IsHtmlResponse())
4141
{
4242
if (!_context.Response.HasStarted) // can't set headers if response has already started
4343
_context.Response.ContentLength = null;
4444
_isContentLengthSet = true;
4545
}
4646

47-
return _baseStream.FlushAsync(cancellationToken);
47+
return _baseStream?.FlushAsync(cancellationToken) ?? Task.CompletedTask;
4848
}
4949

50-
public override void Flush() => _baseStream.Flush();
50+
public override void Flush() => _baseStream?.Flush();
5151

52-
public override int Read(byte[] buffer, int offset, int count) => _baseStream.Read(buffer, offset, count);
52+
public override int Read(byte[] buffer, int offset, int count) => _baseStream?.Read(buffer, offset, count) ?? 0;
5353

54-
public override long Seek(long offset, SeekOrigin origin) => _baseStream.Seek(offset, origin);
54+
public override long Seek(long offset, SeekOrigin origin) => _baseStream?.Seek(offset, origin) ?? 0;
5555

5656
public override void SetLength(long value)
5757
{
58-
_baseStream.SetLength(value);
58+
_baseStream?.SetLength(value);
5959

6060
if (!Disabled)
6161
IsHtmlResponse(forceReCheck: true);
6262
}
6363

64-
public override void Write(ReadOnlySpan<byte> buffer) => _baseStream.Write(buffer);
65-
66-
public override void WriteByte(byte value) => _baseStream.WriteByte(value);
64+
public override void Write(ReadOnlySpan<byte> buffer) => _baseStream?.Write(buffer);
6765

66+
public override void WriteByte(byte value) => _baseStream?.WriteByte(value);
6867

6968
public override void Write(byte[] buffer, int offset, int count)
7069
{
7170
// pass through without modification if we're already in the middle of injecting
7271
// don't inject if the response isn't an HTML response
73-
if (!Disabled && !CurrentlyInjecting() && IsHtmlResponse())
72+
if (_context != null && !Disabled && !CurrentlyInjecting() && IsHtmlResponse())
7473
{
7574
try
7675
{
7776
// Set a flag on the context to indicate we're in the middle of injecting - prevents multiple recursions when response compression is in use
7877
StartInjecting();
79-
_agent.TryInjectBrowserScriptAsync(_context.Response.ContentType, _context.Request.Path.Value, buffer, _baseStream)
78+
_agent.TryInjectBrowserScriptAsync(_context.Response?.ContentType, _context.Request?.Path, buffer, _baseStream)
8079
.GetAwaiter().GetResult();
8180
}
8281
finally
@@ -96,13 +95,13 @@ public override async ValueTask WriteAsync(ReadOnlyMemory<byte> buffer, Cancella
9695
{
9796
// pass through without modification if we're already in the middle of injecting
9897
// don't inject if the response isn't an HTML response
99-
if (!Disabled && !CurrentlyInjecting() && IsHtmlResponse())
98+
if (_context != null & !Disabled && !CurrentlyInjecting() && IsHtmlResponse())
10099
{
101100
try
102101
{
103102
// Set a flag on the context to indicate we're in the middle of injecting - prevents multiple recursions when response compression is in use
104103
StartInjecting();
105-
await _agent.TryInjectBrowserScriptAsync(_context.Response.ContentType, _context.Request.Path.Value, buffer.ToArray(), _baseStream);
104+
await _agent.TryInjectBrowserScriptAsync(_context.Response?.ContentType, _context.Request?.Path, buffer.ToArray(), _baseStream);
106105
}
107106
finally
108107
{
@@ -126,8 +125,11 @@ public override async ValueTask DisposeAsync()
126125
{
127126
_context = null;
128127

129-
await _baseStream.DisposeAsync();
130-
_baseStream = null;
128+
if (_baseStream != null)
129+
{
130+
await _baseStream.DisposeAsync();
131+
_baseStream = null;
132+
}
131133
}
132134

133135
public override bool CanRead { get; }
@@ -154,15 +156,16 @@ private bool IsHtmlResponse(bool forceReCheck = false)
154156
// Requirements for script injection:
155157
// * text/html response
156158
// * UTF-8 formatted (either explicitly or no charset defined)
159+
var responseContentType = _context.Response.ContentType;
157160
_isHtmlResponse =
158-
_context.Response.ContentType != null &&
159-
_context.Response.ContentType.Contains("text/html", StringComparison.OrdinalIgnoreCase) &&
160-
(_context.Response.ContentType.Contains("utf-8", StringComparison.OrdinalIgnoreCase) ||
161-
!_context.Response.ContentType.Contains("charset=", StringComparison.OrdinalIgnoreCase));
161+
!string.IsNullOrEmpty(responseContentType) &&
162+
responseContentType.Contains("text/html", StringComparison.OrdinalIgnoreCase) &&
163+
(responseContentType.Contains("utf-8", StringComparison.OrdinalIgnoreCase) ||
164+
!responseContentType.Contains("charset=", StringComparison.OrdinalIgnoreCase));
162165

163166
if (!_isHtmlResponse.Value)
164167
{
165-
_agent.CurrentTransaction?.LogFinest($"Skipping RUM injection: Not an HTML response. ContentType is {_context.Response.ContentType}");
168+
_agent.CurrentTransaction?.LogFinest($"Skipping RUM injection: Not an HTML response. ContentType is {responseContentType}");
166169
return false;
167170
}
168171

@@ -186,8 +189,7 @@ private bool IsHtmlResponse(bool forceReCheck = false)
186189

187190
private void LogExceptionAndDisable(Exception e)
188191
{
189-
_agent.Logger.Log(Level.Error,
190-
$"Unexpected exception. Browser injection will be disabled. Exception: {e.Message}: {e.StackTrace}");
192+
_agent.Logger.Log(Level.Error, e, "Unexpected exception. Browser injection will be disabled.");
191193

192194
Disabled = true;
193195
}

src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/AspNetCore6Plus/ResponseCompressionBodyOnWriteWrapper.cs

+2-3
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall ins
4545
var context = _contextFieldGetter.Invoke(instrumentedMethodCall.MethodCall.InvocationTarget);
4646

4747
// only wrap the compression stream if browser injection is enabled and the request is not a gRPC request.
48-
if (agent.Configuration.BrowserMonitoringAutoInstrument && agent.Configuration.EnableAspNetCore6PlusBrowserInjection && context.Request.ContentType?.ToLower() != "application/grpc")
48+
if (context != null && agent.Configuration.BrowserMonitoringAutoInstrument && agent.Configuration.EnableAspNetCore6PlusBrowserInjection && context.Request?.ContentType?.ToLower() != "application/grpc")
4949
{
5050
// Wrap _compressionStream and replace the current value with our wrapped version
5151
// check whether we've already wrapped the stream so we don't do it twice
@@ -56,8 +56,7 @@ public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall ins
5656

5757
var responseWrapper = new BrowserInjectingStreamWrapper(agent, currentCompressionStream, context);
5858

59-
_compressionStreamFieldSetter.Invoke(instrumentedMethodCall.MethodCall.InvocationTarget,
60-
responseWrapper);
59+
_compressionStreamFieldSetter.Invoke(instrumentedMethodCall.MethodCall.InvocationTarget, responseWrapper);
6160
}
6261
}
6362
});

0 commit comments

Comments
 (0)