Skip to content

Commit ce84e1a

Browse files
committed
Add LineFeed line terminator switch
1 parent ddc6da1 commit ce84e1a

File tree

8 files changed

+499
-115
lines changed

8 files changed

+499
-115
lines changed

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

Lines changed: 107 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
1515
public class HttpParser<TRequestHandler> : IHttpParser<TRequestHandler> where TRequestHandler : IHttpHeadersHandler, IHttpRequestLineHandler
1616
{
1717
private readonly bool _showErrorDetails;
18+
private readonly bool _enableLineFeedTerminator;
1819

19-
public HttpParser() : this(showErrorDetails: true)
20+
public HttpParser() : this(showErrorDetails: true, enableLineFeedTerminator: true)
2021
{
2122
}
2223

23-
public HttpParser(bool showErrorDetails)
24+
public HttpParser(bool showErrorDetails) : this(showErrorDetails, enableLineFeedTerminator: false)
25+
{
26+
}
27+
28+
internal HttpParser(bool showErrorDetails, bool enableLineFeedTerminator)
2429
{
2530
_showErrorDetails = showErrorDetails;
31+
_enableLineFeedTerminator = enableLineFeedTerminator;
2632
}
2733

2834
// byte types don't have a data type annotation so we pre-cast them; to avoid in-place casts
@@ -115,11 +121,35 @@ private void ParseRequestLine(TRequestHandler handler, ReadOnlySpan<byte> reques
115121
// Consume space
116122
offset++;
117123

118-
// Version + CR is 9 bytes which should take us to .Length
119-
// LF should have been dropped prior to method call
120-
if ((uint)offset + 9 != (uint)requestLine.Length || requestLine[offset + sizeof(ulong)] != ByteCR)
124+
if (!_enableLineFeedTerminator)
121125
{
122-
RejectRequestLine(requestLine);
126+
// Version + CR is 9 bytes which should take us to .Length
127+
// LF should have been dropped prior to method call
128+
if ((uint)offset + 9 != (uint)requestLine.Length || requestLine[offset + 8] != ByteCR)
129+
{
130+
RejectRequestLine(requestLine);
131+
}
132+
}
133+
else
134+
{
135+
// LF should have been dropped prior to method call
136+
// If offset + 8 is .Length then requestLine is valid since it mean LF was the next char
137+
if ((uint)offset + 8 != (uint)requestLine.Length)
138+
{
139+
// Version + CR is 9 bytes which should take us to .Length
140+
if ((uint)offset + 9 != (uint)requestLine.Length || requestLine[offset + 8] != ByteCR)
141+
{
142+
RejectRequestLine(requestLine);
143+
}
144+
}
145+
else
146+
{
147+
// e.g., GET / HTTP1.\r\n
148+
if (requestLine[offset + 7] == ByteCR)
149+
{
150+
RejectRequestLine(requestLine);
151+
}
152+
}
123153
}
124154

125155
// Version
@@ -201,44 +231,59 @@ public bool ParseHeaders(TRequestHandler handler, ref SequenceReader<byte> reade
201231
// the bounds check for the next lookup of span[length]
202232
if ((uint)length < (uint)span.Length)
203233
{
204-
// Early memory read to hide latency
205-
var expectedCR = span[length];
206-
// Correctly has a CR, move to next
207-
length++;
234+
var headerSpan = span[..length];
208235

209-
if (expectedCR != ByteCR)
236+
if (length < 4)
210237
{
211-
// Sequence needs to be CRLF not LF first.
212-
RejectRequestHeader(span[..length]);
238+
// Less than min possible headerSpan of 3 bytes a:b
239+
RejectRequestHeader(headerSpan);
213240
}
241+
242+
// Early memory read to hide latency
243+
var lineTerminator = span[length];
244+
// Correctly has a CR/LF, move to next
245+
length++;
214246

215-
if ((uint)length < (uint)span.Length)
247+
if (lineTerminator == ByteLF)
216248
{
217-
// Early memory read to hide latency
218-
var expectedLF = span[length];
219-
// Correctly has a LF, move to next
220-
length++;
221-
222-
if (expectedLF != ByteLF ||
223-
length < 5 ||
224-
// Exclude the CRLF from the headerLine and parse the header name:value pair
225-
!TryTakeSingleHeader(handler, span[..(length - 2)]))
249+
if (!_enableLineFeedTerminator)
226250
{
227-
// Sequence needs to be CRLF and not contain an inner CR not part of terminator.
228-
// Less than min possible headerSpan of 5 bytes a:b\r\n
229-
// Not parsable as a valid name:value header pair.
251+
// Sequence needs to be CRLF not LF first.
230252
RejectRequestHeader(span[..length]);
231253
}
232-
233-
// Read the header successfully, skip the reader forward past the headerSpan.
234-
span = span.Slice(length);
235-
reader.Advance(length);
236254
}
237255
else
238256
{
239-
// No enough data, set length to 0.
240-
length = 0;
257+
if ((uint)length < (uint)span.Length)
258+
{
259+
// Early memory read to hide latency
260+
var expectedLF = span[length];
261+
// Correctly has a LF, move to next
262+
length++;
263+
264+
if (expectedLF != ByteLF)
265+
{
266+
// Sequence needs to be CRLF not LF first.
267+
RejectRequestHeader(span[..length]);
268+
}
269+
}
270+
else
271+
{
272+
// No enough data, set length to 0.
273+
length = 0;
274+
}
275+
}
276+
277+
if (length != 0 && !TryTakeSingleHeader(handler, headerSpan))
278+
{
279+
// Sequence needs to be CRLF and not contain an inner CR not part of terminator.
280+
// Not parsable as a valid name:value header pair.
281+
RejectRequestHeader(span[..length]);
241282
}
283+
284+
// Read the header successfully, skip the reader forward past the headerSpan.
285+
span = span[length..];
286+
reader.Advance(length);
242287
}
243288
}
244289

@@ -287,14 +332,17 @@ private int ParseMultiSpanHeader(TRequestHandler handler, ref SequenceReader<byt
287332
if (currentSlice.Slice(reader.Position, lineEndPosition.Value).Length == currentSlice.Length - 1)
288333
{
289334
// No enough data, so CRLF can't currently be there.
290-
// However, we need to check the found char is CR and not LF
335+
// However, we need to check the found char is CR and not LF (unless quirk mode)
291336

292337
// Advance 1 to include CR/LF in lineEnd
293338
lineEnd = currentSlice.GetPosition(1, lineEndPosition.Value);
294339
headerSpan = currentSlice.Slice(reader.Position, lineEnd).ToSpan();
295340
if (headerSpan[^1] != ByteCR)
296341
{
297-
RejectRequestHeader(headerSpan);
342+
if (!_enableLineFeedTerminator || headerSpan[^1] != ByteLF)
343+
{
344+
RejectRequestHeader(headerSpan);
345+
}
298346
}
299347
return -1;
300348
}
@@ -305,14 +353,35 @@ private int ParseMultiSpanHeader(TRequestHandler handler, ref SequenceReader<byt
305353

306354
if (headerSpan.Length < 5)
307355
{
308-
// Less than min possible headerSpan is 5 bytes a:b\r\n
309-
RejectRequestHeader(headerSpan);
356+
if (!_enableLineFeedTerminator || headerSpan.Length < 4)
357+
{
358+
// Less than min possible headerSpan is 4 bytes a:b\n or 5 bytes a:b\r\n
359+
RejectRequestHeader(headerSpan);
360+
}
310361
}
311362

312363
if (headerSpan[^2] != ByteCR)
313364
{
314-
// Sequence needs to be CRLF not LF first.
315-
RejectRequestHeader(headerSpan[..^1]);
365+
if (!_enableLineFeedTerminator)
366+
{
367+
// Sequence needs to be CRLF.
368+
RejectRequestHeader(headerSpan[..^1]);
369+
}
370+
else if (headerSpan[^2] != ByteLF)
371+
{
372+
RejectRequestHeader(headerSpan[..^1]);
373+
}
374+
else
375+
{
376+
// Line ends with LF and quirk mode is enabled, try to parse the header
377+
if (!TryTakeSingleHeader(handler, headerSpan[..^1]))
378+
{
379+
// Not parsable as a valid name:value header pair.
380+
RejectRequestHeader(headerSpan);
381+
}
382+
383+
return headerSpan.Length;
384+
}
316385
}
317386

318387
if (headerSpan[^1] != ByteLF ||

src/Servers/Kestrel/Core/src/Internal/KestrelServerImpl.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ private static ServiceContext CreateServiceContext(IOptions<KestrelServerOptions
129129
{
130130
Log = trace,
131131
Scheduler = PipeScheduler.ThreadPool,
132-
HttpParser = new HttpParser<Http1ParsingHandler>(trace.IsEnabled(LogLevel.Information)),
132+
HttpParser = new HttpParser<Http1ParsingHandler>(trace.IsEnabled(LogLevel.Information), serverOptions.EnableLineFeedTerminator),
133133
SystemClock = heartbeatManager,
134134
DateHeaderValueManager = dateHeaderValueManager,
135135
ConnectionManager = connectionManager,

src/Servers/Kestrel/Core/src/KestrelServerOptions.cs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,21 @@ public class KestrelServerOptions
161161
/// </summary>
162162
internal bool IsDevCertLoaded { get; set; }
163163

164+
private bool? _enableLineFeedTerminator;
165+
internal bool EnableLineFeedTerminator
166+
{
167+
get
168+
{
169+
if (!_enableLineFeedTerminator.HasValue)
170+
{
171+
_enableLineFeedTerminator = AppContext.TryGetSwitch("Microsoft.AspNetCore.Server.Kestrel.EnableLineFeedTerminator", out var enabled) && enabled;
172+
}
173+
174+
return _enableLineFeedTerminator.Value;
175+
}
176+
set => _enableLineFeedTerminator = value;
177+
}
178+
164179
/// <summary>
165180
/// Specifies a configuration Action to run for each newly created endpoint. Calling this again will replace
166181
/// the prior action.

0 commit comments

Comments
 (0)