Skip to content

Commit 32fae69

Browse files
authored
Fire an event when allowing a bad request (#39944)
1 parent c6154ba commit 32fae69

File tree

3 files changed

+55
-1
lines changed

3 files changed

+55
-1
lines changed

src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,8 @@ private void ValidateNonOriginHostHeader(string hostText)
612612
// replace it with the value from the request line.
613613
if (_context.ServiceContext.ServerOptions.EnableInsecureAbsoluteFormHostOverride)
614614
{
615+
ReportAllowedBadRequest(KestrelBadHttpRequestException.GetException(RequestRejectionReason.InvalidHostHeader, hostText));
616+
615617
hostText = _absoluteRequestTarget.Authority + ":" + _absoluteRequestTarget.Port.ToString(CultureInfo.InvariantCulture);
616618
HttpRequestHeaders.HeaderHost = hostText;
617619
}

src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1340,6 +1340,24 @@ public void SetBadRequestState(BadHttpRequestException ex)
13401340
_keepAlive = false;
13411341
}
13421342

1343+
// Normally this would have been rejected, but the developer opted into allowing the bad behavior.
1344+
public void ReportAllowedBadRequest(BadHttpRequestException ex)
1345+
{
1346+
Log.ConnectionBadRequest(ConnectionId, ex);
1347+
// Set this so DiagnosticSource listeners can observe the exception via the IBadRequestExceptionFeature.
1348+
// Make sure to unset before exiting the method so the request doesn't actually get rejected.
1349+
// This means the exception will not be visible to normal middleware.
1350+
_requestRejectedException = ex;
1351+
1352+
const string badRequestEventName = "Microsoft.AspNetCore.Server.Kestrel.BadRequest";
1353+
if (ServiceContext.DiagnosticSource?.IsEnabled(badRequestEventName) == true)
1354+
{
1355+
ServiceContext.DiagnosticSource.Write(badRequestEventName, this);
1356+
}
1357+
1358+
_requestRejectedException = null;
1359+
}
1360+
13431361
public void ReportApplicationError(Exception? ex)
13441362
{
13451363
// ReportApplicationError can be called with a null exception from MessageBody

src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,25 +141,59 @@ public Task BadRequestIfHostHeaderDoesNotMatchRequestTarget(string requestTarget
141141
[Fact]
142142
public async Task CanOptOutOfBadRequestIfHostHeaderDoesNotMatchRequestTarget()
143143
{
144+
BadHttpRequestException loggedException = null;
145+
146+
var mockKestrelTrace = new Mock<IKestrelTrace>();
147+
mockKestrelTrace
148+
.Setup(trace => trace.IsEnabled(LogLevel.Information))
149+
.Returns(true);
150+
mockKestrelTrace
151+
.Setup(trace => trace.ConnectionBadRequest(It.IsAny<string>(), It.IsAny<BadHttpRequestException>()))
152+
.Callback<string, BadHttpRequestException>((connectionId, exception) => loggedException = exception);
153+
154+
// Set up a listener to catch the BadRequest event
155+
var diagListener = new DiagnosticListener("OptOutBadRequestTestsDiagListener");
156+
string eventProviderName = "";
157+
string exceptionString = "";
158+
var badRequestEventListener = new BadRequestEventListener(diagListener, (pair) => {
159+
eventProviderName = pair.Key;
160+
var featureCollection = pair.Value as IFeatureCollection;
161+
if (featureCollection is not null)
162+
{
163+
var badRequestFeature = featureCollection.Get<IBadRequestExceptionFeature>();
164+
exceptionString = badRequestFeature.Error.ToString();
165+
}
166+
});
167+
144168
var receivedHost = StringValues.Empty;
145169
await using var server = new TestServer(context =>
146170
{
147171
receivedHost = context.Request.Headers.Host;
148172
return Task.CompletedTask;
149-
}, new TestServiceContext(LoggerFactory)
173+
}, new TestServiceContext(LoggerFactory, mockKestrelTrace.Object)
150174
{
175+
DiagnosticSource = diagListener,
151176
ServerOptions = new KestrelServerOptions()
152177
{
153178
EnableInsecureAbsoluteFormHostOverride = true,
154179
}
155180
});
181+
156182
using var client = server.CreateConnection();
157183

158184
await client.SendAll($"GET http://www.foo.com/api/data HTTP/1.1\r\nHost: www.foo.comConnection: keep-alive\r\n\r\n");
159185

160186
await client.Receive("HTTP/1.1 200 OK");
161187

162188
Assert.Equal("www.foo.com:80", receivedHost);
189+
190+
mockKestrelTrace.Verify(trace => trace.ConnectionBadRequest(It.IsAny<string>(), It.IsAny<BadHttpRequestException>()));
191+
Assert.Equal(CoreStrings.FormatBadRequest_InvalidHostHeader_Detail("www.foo.comConnection: keep-alive"), loggedException.Message);
192+
193+
// Verify DiagnosticSource event for bad request
194+
Assert.True(badRequestEventListener.EventFired);
195+
Assert.Equal("Microsoft.AspNetCore.Server.Kestrel.BadRequest", eventProviderName);
196+
Assert.Contains(CoreStrings.FormatBadRequest_InvalidHostHeader_Detail("www.foo.comConnection: keep-alive"), exceptionString);
163197
}
164198

165199
[Fact]

0 commit comments

Comments
 (0)