Skip to content

Support LF request line terminator in HttpParser #43202

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Aug 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
302 changes: 180 additions & 122 deletions src/Servers/Kestrel/Core/src/Internal/Http/HttpParser.cs

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/Servers/Kestrel/Core/src/Internal/KestrelServerImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ private static ServiceContext CreateServiceContext(IOptions<KestrelServerOptions
{
Log = trace,
Scheduler = PipeScheduler.ThreadPool,
HttpParser = new HttpParser<Http1ParsingHandler>(trace.IsEnabled(LogLevel.Information)),
HttpParser = new HttpParser<Http1ParsingHandler>(trace.IsEnabled(LogLevel.Information), serverOptions.DisableHttp1LineFeedTerminators),
SystemClock = heartbeatManager,
DateHeaderValueManager = dateHeaderValueManager,
ConnectionManager = connectionManager,
Expand Down
20 changes: 20 additions & 0 deletions src/Servers/Kestrel/Core/src/KestrelServerOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core;
/// </summary>
public class KestrelServerOptions
{
internal const string DisableHttp1LineFeedTerminatorsSwitchKey = "Microsoft.AspNetCore.Server.Kestrel.DisableHttp1LineFeedTerminators";

// internal to fast-path header decoding when RequestHeaderEncodingSelector is unchanged.
internal static readonly Func<string, Encoding?> DefaultHeaderEncodingSelector = _ => null;

Expand Down Expand Up @@ -175,6 +177,24 @@ internal bool EnableWebTransportAndH3Datagrams
set => _enableWebTransportAndH3Datagrams = value;
}

/// <summary>
/// Internal AppContext switch to toggle whether a request line can end with LF only instead of CR/LF.
/// </summary>
private bool? _disableHttp1LineFeedTerminators;
internal bool DisableHttp1LineFeedTerminators
{
get
{
if (!_disableHttp1LineFeedTerminators.HasValue)
{
_disableHttp1LineFeedTerminators = AppContext.TryGetSwitch(DisableHttp1LineFeedTerminatorsSwitchKey, out var disabled) && disabled;
}

return _disableHttp1LineFeedTerminators.Value;
}
set => _disableHttp1LineFeedTerminators = value;
}

/// <summary>
/// Specifies a configuration Action to run for each newly created endpoint. Calling this again will replace
/// the prior action.
Expand Down
291 changes: 261 additions & 30 deletions src/Servers/Kestrel/Core/test/HttpParserTests.cs

Large diffs are not rendered by default.

37 changes: 34 additions & 3 deletions src/Servers/Kestrel/shared/test/HttpParsingData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,15 @@ public static IEnumerable<string> RequestLineInvalidData
"CUSTOM / HTTP/1.1a\n",
"CUSTOM / HTTP/1.1a\r\n",
"CUSTOM / HTTP/1.1ab\r\n",
"CUSTOM / H\n",
"CUSTOM / HT\n",
"CUSTOM / HTT\n",
"CUSTOM / HTTP\n",
"CUSTOM / HTTP/\n",
"CUSTOM / HTTP/1\n",
"CUSTOM / HTTP/1.\n",
"CUSTOM / hello\r\n",
"CUSTOM / hello\n",
"CUSTOM ? HTTP/1.1\r\n",
"CUSTOM /a?b=cHTTP/1.1\r\n",
"CUSTOM /a%20bHTTP/1.1\r\n",
Expand All @@ -217,6 +225,21 @@ public static IEnumerable<string> RequestLineInvalidData
}
}

// This list is valid in quirk mode
public static IEnumerable<string> RequestLineInvalidDataLineFeedTerminator
{
get
{
return new[]
{
"GET / HTTP/1.0\n",
"GET / HTTP/1.1\n",
"CUSTOM / HTTP/1.0\n",
"CUSTOM / HTTP/1.1\n",
};
}
}

// Bad HTTP Methods (invalid according to RFC)
public static IEnumerable<string> MethodWithNonTokenCharData
{
Expand Down Expand Up @@ -364,13 +387,19 @@ public static IEnumerable<string> TargetWithNullCharData
"8charact",
};

public static IEnumerable<object[]> RequestHeaderInvalidData => new[]
public static IEnumerable<object[]> RequestHeaderInvalidDataLineFeedTerminator => new[]
{
// Missing CR
new[] { "Header: value\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header: value\x0A") },
new[] { "Header-1: value1\nHeader-2: value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: value1\x0A") },
new[] { "Header-1: value1\r\nHeader-2: value2\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-2: value2\x0A") },

// Empty header name
new[] { ":a\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@":a\x0A") },
};

public static IEnumerable<object[]> RequestHeaderInvalidData => new[]
{
// Line folding
new[] { "Header: line1\r\n line2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@" line2\x0D\x0A") },
new[] { "Header: line1\r\n\tline2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"\x09line2\x0D\x0A") },
Expand Down Expand Up @@ -404,7 +433,7 @@ public static IEnumerable<string> TargetWithNullCharData
new[] { "Header-1 value1\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1 value1\x0D\x0A") },
new[] { "Header-1 value1\r\nHeader-2: value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1 value1\x0D\x0A") },
new[] { "Header-1: value1\r\nHeader-2 value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-2 value2\x0D\x0A") },
new[] { "\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"\x0A") },
new[] { "HeaderValue1\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"HeaderValue1\x0D\x0A") },

// Starting with whitespace
new[] { " Header: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@" Header: value\x0D\x0A") },
Expand Down Expand Up @@ -435,11 +464,13 @@ public static IEnumerable<string> TargetWithNullCharData

// Headers not ending in CRLF line
new[] { "Header-1: value1\r\nHeader-2: value2\r\n\r\r", CoreStrings.BadRequest_InvalidRequestHeadersNoCRLF },
new[] { "Header-1: value1\r\nHeader-2: value2\r\n\r ", CoreStrings.BadRequest_InvalidRequestHeadersNoCRLF },
new[] { "Header-1: value1\r\nHeader-2: value2\r\n\r ", CoreStrings.BadRequest_InvalidRequestHeadersNoCRLF },
new[] { "Header-1: value1\r\nHeader-2: value2\r\n\r \n", CoreStrings.BadRequest_InvalidRequestHeadersNoCRLF },
new[] { "Header-1: value1\r\nHeader-2\t: value2 \n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-2\x09: value2 \x0A") },

// Empty header name
new[] { ": value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@": value\x0D\x0A") },
new[] { ":a\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@":a\x0D\x0A") },
};

public static TheoryData<string, string> HostHeaderData
Expand Down
14 changes: 7 additions & 7 deletions src/Servers/Kestrel/shared/test/TestServiceContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,17 @@ internal class TestServiceContext : ServiceContext
{
public TestServiceContext()
{
Initialize(NullLoggerFactory.Instance, CreateLoggingTrace(NullLoggerFactory.Instance));
Initialize(NullLoggerFactory.Instance, CreateLoggingTrace(NullLoggerFactory.Instance), false);
}

public TestServiceContext(ILoggerFactory loggerFactory)
public TestServiceContext(ILoggerFactory loggerFactory, bool disableHttp1LineFeedTerminators = true)
{
Initialize(loggerFactory, CreateLoggingTrace(loggerFactory));
Initialize(loggerFactory, CreateLoggingTrace(loggerFactory), disableHttp1LineFeedTerminators);
}

public TestServiceContext(ILoggerFactory loggerFactory, KestrelTrace kestrelTrace)
public TestServiceContext(ILoggerFactory loggerFactory, KestrelTrace kestrelTrace, bool disableHttp1LineFeedTerminators = true)
{
Initialize(loggerFactory, kestrelTrace);
Initialize(loggerFactory, kestrelTrace, disableHttp1LineFeedTerminators);
}

private static KestrelTrace CreateLoggingTrace(ILoggerFactory loggerFactory)
Expand All @@ -49,7 +49,7 @@ public void InitializeHeartbeat()
SystemClock = heartbeatManager;
}

private void Initialize(ILoggerFactory loggerFactory, KestrelTrace kestrelTrace)
private void Initialize(ILoggerFactory loggerFactory, KestrelTrace kestrelTrace, bool disableHttp1LineFeedTerminators)
{
LoggerFactory = loggerFactory;
Log = kestrelTrace;
Expand All @@ -58,7 +58,7 @@ private void Initialize(ILoggerFactory loggerFactory, KestrelTrace kestrelTrace)
SystemClock = MockSystemClock;
DateHeaderValueManager = new DateHeaderValueManager();
ConnectionManager = new ConnectionManager(Log, ResourceCounter.Unlimited);
HttpParser = new HttpParser<Http1ParsingHandler>(Log.IsEnabled(LogLevel.Information));
HttpParser = new HttpParser<Http1ParsingHandler>(Log.IsEnabled(LogLevel.Information), disableHttp1LineFeedTerminators);
ServerOptions = new KestrelServerOptions
{
AddServerHeader = false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -524,5 +524,7 @@ public static TheoryData<string, string> InvalidRequestLineData

public static IEnumerable<object[]> InvalidRequestHeaderData => HttpParsingData.RequestHeaderInvalidData;

public static IEnumerable<object[]> InvalidRequestHeaderDataLineFeedTerminator => HttpParsingData.RequestHeaderInvalidDataLineFeedTerminator;

public static TheoryData<string, string> InvalidHostHeaderData => HttpParsingData.HostHeaderInvalidData;
}
43 changes: 43 additions & 0 deletions src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2284,6 +2284,49 @@ await connection.Receive(
}
}

[Fact]
public async Task SingleLineFeedIsSupportedAnywhere()
{
// Exercises all combinations of LF and CRLF as line separators.
// Uses a bit mask for all the possible combinations.

var lines = new[]
{
$"GET / HTTP/1.1",
"Content-Length: 0",
$"Host: localhost",
"",
};

await using (var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory, disableHttp1LineFeedTerminators: false)))
{
var mask = Math.Pow(2, lines.Length) - 1;

for (var m = 0; m <= mask; m++)
{
using (var client = server.CreateConnection())
{
var sb = new StringBuilder();

for (var pos = 0; pos < lines.Length; pos++)
{
sb.Append(lines[pos]);
var separator = (m & (1 << pos)) != 0 ? "\n" : "\r\n";
sb.Append(separator);
}

var text = sb.ToString();
var writer = new StreamWriter(client.Stream, Encoding.GetEncoding("iso-8859-1"));
await writer.WriteAsync(text).ConfigureAwait(false);
await writer.FlushAsync().ConfigureAwait(false);
await client.Stream.FlushAsync().ConfigureAwait(false);

await client.Receive("HTTP/1.1 200");
}
}
}
}

public static TheoryData<string, string> HostHeaderData => HttpParsingData.HostHeaderData;

private class IntAsClass
Expand Down