Skip to content

Commit

Permalink
summary: ASP.NET Core 6+ Browser Injection Bug Fixes
Browse files Browse the repository at this point in the history
fix: Bypass gRPC requests in the browser injection middleware. Also added a configuration setting to disable ASP.NET Core 6+ browser injection; the setting enables browser injection by default but can be overridden if necessary.
  • Loading branch information
tippmar-nr authored Nov 9, 2023
1 parent 322a00e commit e571ac1
Show file tree
Hide file tree
Showing 11 changed files with 99 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2085,7 +2085,6 @@ private void UpdateDiagnosticsAgentTimingSettings()
_diagnosticsCaptureAgentTimingFrequency = configFreq;
}


private bool? _forceSynchronousTimingCalculationHttpClient;
public bool ForceSynchronousTimingCalculationHttpClient
{
Expand All @@ -2097,6 +2096,17 @@ public bool ForceSynchronousTimingCalculationHttpClient
}
}

private bool? _enableAspNetCore6PlusBrowserInjection;
public bool EnableAspNetCore6PlusBrowserInjection
{
get
{
return _enableAspNetCore6PlusBrowserInjection.HasValue
? _enableAspNetCore6PlusBrowserInjection.Value
: (_enableAspNetCore6PlusBrowserInjection = TryGetAppSettingAsBoolWithDefault("EnableAspNetCore6PlusBrowserInjection", true)).Value;
}
}

private TimeSpan? _metricsHarvestCycleOverride = null;
public TimeSpan MetricsHarvestCycle
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,9 @@ public ReportedConfiguration(IConfiguration configuration)
[JsonProperty("agent.force_synchronous_timing_calculation_for_http_client")]
public bool ForceSynchronousTimingCalculationHttpClient => _configuration.ForceSynchronousTimingCalculationHttpClient;

[JsonProperty("agent.enable_asp_net_core_6plus_browser_injection")]
public bool EnableAspNetCore6PlusBrowserInjection => _configuration.EnableAspNetCore6PlusBrowserInjection;

[JsonProperty("agent.exclude_new_relic_header")]
public bool ExcludeNewrelicHeader => _configuration.ExcludeNewrelicHeader;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ public interface IConfiguration
string ProcessHostDisplayName { get; }
int DatabaseStatementCacheCapacity { get; }
bool ForceSynchronousTimingCalculationHttpClient { get; }
bool EnableAspNetCore6PlusBrowserInjection { get; }
bool ExcludeNewrelicHeader { get; }
bool ApplicationLoggingEnabled { get; }
bool LogMetricsCollectorEnabled { get; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,12 @@ public Action<IApplicationBuilder> Configure(Action<IApplicationBuilder> next)
return builder =>
{
builder.UseMiddleware<WrapPipelineMiddleware>(_agent);
builder.UseMiddleware<BrowserInjectionMiddleware>(_agent);

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

next(builder);
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,23 @@ public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall ins
{
return Delegates.GetDelegateFor(onSuccess: () =>
{
// Wrap _compressionStream and replace the current value with our wrapped version
// check whether we've already wrapped the stream so we don't do it twice
var currentCompressionStream = _compressionStreamFieldGetter.Invoke(instrumentedMethodCall.MethodCall.InvocationTarget);
if (currentCompressionStream != null && currentCompressionStream.GetType() != typeof(BrowserInjectingStreamWrapper))
var context = _contextFieldGetter.Invoke(instrumentedMethodCall.MethodCall.InvocationTarget);

// only wrap the compression stream if browser injection is enabled and the request is not a gRPC request.
if (agent.Configuration.BrowserMonitoringAutoInstrument && agent.Configuration.EnableAspNetCore6PlusBrowserInjection && context.Request.ContentType?.ToLower() != "application/grpc")
{
var context = _contextFieldGetter.Invoke(instrumentedMethodCall.MethodCall.InvocationTarget);
// Wrap _compressionStream and replace the current value with our wrapped version
// check whether we've already wrapped the stream so we don't do it twice
var currentCompressionStream = _compressionStreamFieldGetter.Invoke(instrumentedMethodCall.MethodCall.InvocationTarget);

if (currentCompressionStream != null && currentCompressionStream.GetType() != typeof(BrowserInjectingStreamWrapper))
{

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

_compressionStreamFieldSetter.Invoke(instrumentedMethodCall.MethodCall.InvocationTarget, responseWrapper);
_compressionStreamFieldSetter.Invoke(instrumentedMethodCall.MethodCall.InvocationTarget,
responseWrapper);
}
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -405,5 +405,14 @@ public NewRelicConfigModifier ConfigureFasterUpdateLoadedModulesCycle(int second
CommonUtils.SetConfigAppSetting(_configFilePath, "OverrideUpdateLoadedModulesCycle", seconds.ToString(), "urn:newrelic-config");
return this;
}

public NewRelicConfigModifier EnableAspNetCore6PlusBrowserInjection(bool enableBrowserInjection)
{
CommonUtils.ModifyOrCreateXmlNodeInNewRelicConfig(_configFilePath, new[] { "configuration" }, "appSettings", string.Empty);
CommonUtils.ModifyOrCreateXmlNodeInNewRelicConfig(_configFilePath, new[] { "configuration", "appSettings" }, "add", string.Empty);
CommonUtils.ModifyOrCreateXmlAttributeInNewRelicConfig(_configFilePath, new[] { "configuration", "appSettings", "add"}, "key", "EnableAspNetCore6PlusBrowserInjection");
CommonUtils.ModifyOrCreateXmlAttributeInNewRelicConfig(_configFilePath, new[] { "configuration", "appSettings", "add"}, "value", $"{enableBrowserInjection}");
return this;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,14 @@ public abstract class BrowserAgentAutoInjection6PlusBase : NewRelicIntegrationTe
private string _htmlContent;
private string _staticContent;
private string _fooContent;
private bool _browserInjectionEnabled;

protected BrowserAgentAutoInjection6PlusBase(BasicAspNetCoreRazorApplicationFixture fixture,
ITestOutputHelper output, bool enableResponseCompression, string loaderType = "rum")
ITestOutputHelper output, bool enableBrowserInjection, bool enableResponseCompression, string loaderType = "rum")
: base(fixture)
{
_browserInjectionEnabled = enableBrowserInjection;

_fixture = fixture;
_fixture.TestLogger = output;
_fixture.Actions
Expand All @@ -31,6 +34,7 @@ protected BrowserAgentAutoInjection6PlusBase(BasicAspNetCoreRazorApplicationFixt
var configModifier = new NewRelicConfigModifier(configPath);
configModifier.AutoInstrumentBrowserMonitoring(true);
configModifier.BrowserMonitoringEnableAttributes(true);
configModifier.EnableAspNetCore6PlusBrowserInjection(enableBrowserInjection);

configModifier.BrowserMonitoringLoader(loaderType);
},
Expand Down Expand Up @@ -65,9 +69,18 @@ public void Test()
var jsAgentFromStaticContent = JavaScriptAgent.GetJavaScriptAgentScriptFromSource(_staticContent);
var jsAgentFromFooContent = JavaScriptAgent.GetJavaScriptAgentScriptFromSource(_fooContent);

Assert.Equal(jsAgentFromConnectResponse, jsAgentFromHtmlContent);
Assert.Equal(jsAgentFromConnectResponse, jsAgentFromStaticContent);
Assert.Null(jsAgentFromFooContent);
if (_browserInjectionEnabled)
{
Assert.Equal(jsAgentFromConnectResponse, jsAgentFromHtmlContent);
Assert.Equal(jsAgentFromConnectResponse, jsAgentFromStaticContent);
Assert.Null(jsAgentFromFooContent);
}
else
{
Assert.Null(jsAgentFromHtmlContent);
Assert.Null(jsAgentFromStaticContent);
Assert.Null(jsAgentFromFooContent);
}

// verify that the browser injecting stream wrapper didn't catch an exception and disable itself
var agentDisabledLogLine = _fixture.AgentLog.TryGetLogLine(AgentLogBase.ErrorLogLinePrefixRegex + "Unexpected exception. Browser injection will be disabled. *?");
Expand All @@ -79,7 +92,7 @@ public void Test()
public class BrowserAgentAutoInjection6PlusRumUnCompressed : BrowserAgentAutoInjection6PlusBase
{
public BrowserAgentAutoInjection6PlusRumUnCompressed(BasicAspNetCoreRazorApplicationFixture fixture, ITestOutputHelper output)
: base(fixture, output, false)
: base(fixture, output, true, false)
{
}
}
Expand All @@ -88,17 +101,34 @@ public BrowserAgentAutoInjection6PlusRumUnCompressed(BasicAspNetCoreRazorApplica
public class BrowserAgentAutoInjection6PlusRumCompressed : BrowserAgentAutoInjection6PlusBase
{
public BrowserAgentAutoInjection6PlusRumCompressed(BasicAspNetCoreRazorApplicationFixture fixture, ITestOutputHelper output)
: base(fixture, output, true)
: base(fixture, output, true, true)
{
}
}

[NetCoreTest]
public class BrowserAgentAutoInjection6PlusInjectionDisabledRumUnCompressed : BrowserAgentAutoInjection6PlusBase
{
public BrowserAgentAutoInjection6PlusInjectionDisabledRumUnCompressed(BasicAspNetCoreRazorApplicationFixture fixture, ITestOutputHelper output)
: base(fixture, output, true, false)
{
}
}

[NetCoreTest]
public class BrowserAgentAutoInjection6PlusInjectionDisabledRumCompressed : BrowserAgentAutoInjection6PlusBase
{
public BrowserAgentAutoInjection6PlusInjectionDisabledRumCompressed(BasicAspNetCoreRazorApplicationFixture fixture, ITestOutputHelper output)
: base(fixture, output, false, true)
{
}
}

[NetCoreTest]
public class BrowserAgentAutoInjection6PlusSpa : BrowserAgentAutoInjection6PlusBase
{
public BrowserAgentAutoInjection6PlusSpa(BasicAspNetCoreRazorApplicationFixture fixture, ITestOutputHelper output)
: base(fixture, output, true, "spa")
: base(fixture, output, true, true, "spa")
{
}
}
Expand All @@ -107,7 +137,7 @@ public BrowserAgentAutoInjection6PlusSpa(BasicAspNetCoreRazorApplicationFixture
public class BrowserAgentAutoInjection6PlusFull : BrowserAgentAutoInjection6PlusBase
{
public BrowserAgentAutoInjection6PlusFull(BasicAspNetCoreRazorApplicationFixture fixture, ITestOutputHelper output)
: base(fixture, output, true, "full")
: base(fixture, output, true, true, "full")
{
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3241,6 +3241,18 @@ public bool AsyncHttpClientSegmentsDoNotCountTowardsParentExclusiveTimeTests(str
return defaultConfig.ForceSynchronousTimingCalculationHttpClient;
}

[TestCase(null, ExpectedResult = true)]
[TestCase("not a bool", ExpectedResult = true)]
[TestCase("false", ExpectedResult = false)]
[TestCase("true", ExpectedResult = true)]
public bool AspNetCore6PlusBrowserInjectionTests(string localConfigValue)
{
_localConfig.appSettings.Add(new configurationAdd { key = "EnableAspNetCore6PlusBrowserInjection", value = localConfigValue });
var defaultConfig = new TestableDefaultConfiguration(_environment, _localConfig, _serverConfig, _runTimeConfig, _securityPoliciesConfiguration, _processStatic, _httpRuntimeStatic, _configurationManagerStatic, _dnsStatic);

return defaultConfig.EnableAspNetCore6PlusBrowserInjection;
}

[TestCase("true", true, ExpectedResult = true)]
[TestCase("true", false, ExpectedResult = true)]
[TestCase("true", null, ExpectedResult = true)]
Expand Down
Loading

0 comments on commit e571ac1

Please sign in to comment.