Skip to content

Commit cad4c5f

Browse files
committed
Log
1 parent c2a38e8 commit cad4c5f

File tree

10 files changed

+47
-4
lines changed

10 files changed

+47
-4
lines changed
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
#nullable enable
22
Microsoft.Net.Http.Headers.MediaTypeHeaderValue.MatchesMediaType(Microsoft.Extensions.Primitives.StringSegment otherMediaType) -> bool
3-
3+
static readonly Microsoft.Net.Http.Headers.HeaderNames.ProxyConnection -> string!

src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8319,7 +8319,10 @@ internal partial class HttpResponseHeaders
83198319

83208320
public bool HasConnection => (_bits & 0x2L) != 0;
83218321
public bool HasDate => (_bits & 0x4L) != 0;
8322+
public bool HasKeepAlive => (_bits & 0x10L) != 0;
83228323
public bool HasTransferEncoding => (_bits & 0x80L) != 0;
8324+
public bool HasUpgrade => (_bits & 0x100L) != 0;
8325+
public bool HasProxyConnection => (_bits & 0x4000000L) != 0;
83238326
public bool HasServer => (_bits & 0x10000000L) != 0;
83248327

83258328

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

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1088,10 +1088,23 @@ private HttpResponseHeaders CreateResponseHeaders(bool appCompleted)
10881088
var hasConnection = responseHeaders.HasConnection;
10891089
var hasTransferEncoding = responseHeaders.HasTransferEncoding;
10901090

1091+
// We opt to remove the following headers from an HTTP/2+ response since their presence would be considered a protocol violation.
1092+
// This is done quietly because these headers are valid in other contexts and this saves the app from being broken by
1093+
// low level protocol details. Http.Sys also removes these headers silently.
1094+
//
10911095
// https://tools.ietf.org/html/rfc7540#section-8.1.2.2
1096+
// "This means that an intermediary transforming an HTTP/1.x message to HTTP/2 will need to remove any header fields
1097+
// nominated by the Connection header field, along with the Connection header field itself.
10921098
// Such intermediaries SHOULD also remove other connection-specific header fields, such as Keep-Alive,
1093-
// Proxy-Connection, Transfer-Encoding, and Upgrade, even if they are not nominated by the Connection header field.
1094-
if (_httpVersion > Http.HttpVersion.Http11)
1099+
// Proxy-Connection, Transfer-Encoding, and Upgrade, even if they are not nominated by the Connection header field."
1100+
//
1101+
// Http/3 has a similar requirement: https://quicwg.org/base-drafts/draft-ietf-quic-http.html#name-field-formatting-and-compre
1102+
if (_httpVersion > Http.HttpVersion.Http11
1103+
&& (hasTransferEncoding
1104+
|| hasConnection
1105+
|| responseHeaders.HasKeepAlive
1106+
|| responseHeaders.HasUpgrade
1107+
|| responseHeaders.HasProxyConnection))
10951108
{
10961109
responseHeaders.ClearTransferEncoding();
10971110
responseHeaders.ClearConnection();
@@ -1101,6 +1114,8 @@ private HttpResponseHeaders CreateResponseHeaders(bool appCompleted)
11011114

11021115
hasTransferEncoding = false;
11031116
hasConnection = false;
1117+
1118+
Log.InvalidResponseHeaderRemoved();
11041119
}
11051120

11061121
if (_keepAlive &&

src/Servers/Kestrel/Core/src/Internal/Infrastructure/IKestrelTrace.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,5 +76,7 @@ internal interface IKestrelTrace : ILogger
7676
void Http2FrameSending(string connectionId, Http2Frame frame);
7777

7878
void Http2MaxConcurrentStreamsReached(string connectionId);
79+
80+
void InvalidResponseHeaderRemoved();
7981
}
8082
}

src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelTrace.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,10 @@ internal class KestrelTrace : IKestrelTrace
117117
LoggerMessage.Define<string>(LogLevel.Debug, new EventId(40, nameof(Http2MaxConcurrentStreamsReached)),
118118
@"Connection id ""{ConnectionId}"" reached the maximum number of concurrent HTTP/2 streams allowed.");
119119

120+
private static readonly Action<ILogger, Exception> _invalidResponseHeaderRemoved =
121+
LoggerMessage.Define(LogLevel.Debug, new EventId(41, nameof(InvalidResponseHeaderRemoved)),
122+
"A response header has been removed because it was invalid for the current protocol.");
123+
120124
protected readonly ILogger _logger;
121125

122126
public KestrelTrace(ILogger logger)
@@ -295,6 +299,11 @@ public void Http2MaxConcurrentStreamsReached(string connectionId)
295299
_http2MaxConcurrentStreamsReached(_logger, connectionId, null);
296300
}
297301

302+
public void InvalidResponseHeaderRemoved()
303+
{
304+
_invalidResponseHeaderRemoved(_logger, null);
305+
}
306+
298307
public virtual void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter)
299308
=> _logger.Log(logLevel, eventId, state, exception, formatter);
300309

src/Servers/Kestrel/perf/Kestrel.Performance/Mocks/MockTrace.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,5 +56,6 @@ public void Http2ConnectionClosed(string connectionId, int highestOpenedStreamId
5656
public void Http2FrameReceived(string connectionId, Http2Frame frame) { }
5757
public void Http2FrameSending(string connectionId, Http2Frame frame) { }
5858
public void Http2MaxConcurrentStreamsReached(string connectionId) { }
59+
public void InvalidResponseHeaderRemoved() { }
5960
}
6061
}

src/Servers/Kestrel/shared/KnownHeaders.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,10 @@ static KnownHeaders()
132132
"Connection",
133133
"Server",
134134
"Date",
135-
"Transfer-Encoding"
135+
"Transfer-Encoding",
136+
"Keep-Alive",
137+
"Upgrade",
138+
"Proxy-Connection"
136139
};
137140
var enhancedHeaders = new[]
138141
{

src/Servers/Kestrel/shared/test/CompositeKestrelTrace.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,5 +235,11 @@ public void Http2MaxConcurrentStreamsReached(string connectionId)
235235
_trace1.Http2MaxConcurrentStreamsReached(connectionId);
236236
_trace2.Http2MaxConcurrentStreamsReached(connectionId);
237237
}
238+
239+
public void InvalidResponseHeaderRemoved()
240+
{
241+
_trace1.InvalidResponseHeaderRemoved();
242+
_trace2.InvalidResponseHeaderRemoved();
243+
}
238244
}
239245
}

src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4825,6 +4825,8 @@ await ExpectAsync(Http2FrameType.DATA,
48254825
Assert.Equal("200", _decodedHeaders[HeaderNames.Status]);
48264826

48274827
Assert.True(_helloWorldBytes.AsSpan().SequenceEqual(dataFrame1.PayloadSequence.ToArray()));
4828+
4829+
Assert.Contains(LogMessages, m => m.Message.Equals("A response header has been removed because it was invalid for the current protocol."));
48284830
}
48294831
}
48304832
}

src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -618,6 +618,8 @@ public async Task RemoveConnectionSpecificHeaders()
618618

619619
var responseData = await requestStream.ExpectDataAsync();
620620
Assert.Equal("Hello world", Encoding.ASCII.GetString(responseData.ToArray()));
621+
622+
Assert.Contains(LogMessages, m => m.Message.Equals("A response header has been removed because it was invalid for the current protocol."));
621623
}
622624

623625
[Fact(Skip = "Http3OutputProducer.Complete is called before input recognizes there is an error. Why is this different than HTTP/2?")]

0 commit comments

Comments
 (0)