Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Commit a1c5987

Browse files
author
Cesar Blum Silveira
committed
Reject POST and PUT requests with no Content-Length or Transfer-Encoding (#1130).
1 parent 5b65117 commit a1c5987

File tree

6 files changed

+105
-22
lines changed

6 files changed

+105
-22
lines changed

‎src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,12 @@ internal static BadHttpRequestException GetException(RequestRejectionReason reas
102102
case RequestRejectionReason.FinalTransferCodingNotChunked:
103103
ex = new BadHttpRequestException($"Final transfer coding is not \"chunked\": \"{value}\"", 400);
104104
break;
105+
case RequestRejectionReason.LengthRequired:
106+
ex = new BadHttpRequestException($"{value} request contains no Content-Length or Transfer-Encoding header", 411);
107+
break;
108+
case RequestRejectionReason.LengthRequiredHttp10:
109+
ex = new BadHttpRequestException($"{value} request contains no Content-Length header", 400);
110+
break;
105111
default:
106112
ex = new BadHttpRequestException("Bad request.", 400);
107113
break;

‎src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/MessageBody.cs

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Numerics;
77
using System.Threading;
88
using System.Threading.Tasks;
9+
using Microsoft.AspNetCore.Http;
910
using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure;
1011
using Microsoft.Extensions.Internal;
1112
using Microsoft.Extensions.Primitives;
@@ -266,17 +267,29 @@ public static MessageBody For(
266267
return new ForChunkedEncoding(keepAlive, headers, context);
267268
}
268269

269-
var unparsedContentLength = headers.HeaderContentLength.ToString();
270-
if (unparsedContentLength.Length > 0)
270+
var unparsedContentLength = headers.HeaderContentLength;
271+
if (unparsedContentLength.Count > 0)
271272
{
272-
long contentLength;
273-
if (!long.TryParse(unparsedContentLength, out contentLength) || contentLength < 0)
273+
try
274+
{
275+
var contentLength = FrameHeaders.ParseContentLength(unparsedContentLength);
276+
return new ForContentLength(keepAlive, contentLength, context);
277+
}
278+
catch (InvalidOperationException)
274279
{
275280
context.RejectRequest(RequestRejectionReason.InvalidContentLength, unparsedContentLength);
276281
}
277-
else
282+
}
283+
284+
// Avoid slowing down most common case
285+
if (!object.ReferenceEquals(context.Method, HttpMethods.Get))
286+
{
287+
// If we got here, request contains no Content-Length or Transfer-Encoding header.
288+
// Reject with 411 Length Required.
289+
if (HttpMethods.IsPost(context.Method) || HttpMethods.IsPut(context.Method))
278290
{
279-
return new ForContentLength(keepAlive, contentLength, context);
291+
var requestRejectionReason = httpVersion == HttpVersion.Http11 ? RequestRejectionReason.LengthRequired : RequestRejectionReason.LengthRequiredHttp10;
292+
context.RejectRequest(requestRejectionReason, context.Method);
280293
}
281294
}
282295

‎src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/RequestRejectionReason.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ public enum RequestRejectionReason
2727
MissingCRInHeaderLine,
2828
TooManyHeaders,
2929
RequestTimeout,
30-
FinalTransferCodingNotChunked
30+
FinalTransferCodingNotChunked,
31+
LengthRequired,
32+
LengthRequiredHttp10,
3133
}
3234
}

‎test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxRequestLineSizeTests.cs

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,27 +11,27 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
1111
public class MaxRequestLineSizeTests
1212
{
1313
[Theory]
14-
[InlineData("GET / HTTP/1.1\r\n", 16)]
15-
[InlineData("GET / HTTP/1.1\r\n", 17)]
16-
[InlineData("GET / HTTP/1.1\r\n", 137)]
17-
[InlineData("POST /abc/de HTTP/1.1\r\n", 23)]
18-
[InlineData("POST /abc/de HTTP/1.1\r\n", 24)]
19-
[InlineData("POST /abc/de HTTP/1.1\r\n", 287)]
20-
[InlineData("PUT /abc/de?f=ghi HTTP/1.1\r\n", 28)]
21-
[InlineData("PUT /abc/de?f=ghi HTTP/1.1\r\n", 29)]
22-
[InlineData("PUT /abc/de?f=ghi HTTP/1.1\r\n", 589)]
23-
[InlineData("DELETE /a%20b%20c/d%20e?f=ghi HTTP/1.1\r\n", 40)]
24-
[InlineData("DELETE /a%20b%20c/d%20e?f=ghi HTTP/1.1\r\n", 41)]
25-
[InlineData("DELETE /a%20b%20c/d%20e?f=ghi HTTP/1.1\r\n", 1027)]
26-
public async Task ServerAcceptsRequestLineWithinLimit(string requestLine, int limit)
14+
[InlineData("GET / HTTP/1.1\r\n\r\n", 16)]
15+
[InlineData("GET / HTTP/1.1\r\n\r\n", 17)]
16+
[InlineData("GET / HTTP/1.1\r\n\r\n", 137)]
17+
[InlineData("POST /abc/de HTTP/1.1\r\nContent-Length: 0\r\n\r\n", 23)]
18+
[InlineData("POST /abc/de HTTP/1.1\r\nContent-Length: 0\r\n\r\n", 24)]
19+
[InlineData("POST /abc/de HTTP/1.1\r\nContent-Length: 0\r\n\r\n", 287)]
20+
[InlineData("PUT /abc/de?f=ghi HTTP/1.1\r\nContent-Length: 0\r\n\r\n", 28)]
21+
[InlineData("PUT /abc/de?f=ghi HTTP/1.1\r\nContent-Length: 0\r\n\r\n", 29)]
22+
[InlineData("PUT /abc/de?f=ghi HTTP/1.1\r\nContent-Length: 0\r\n\r\n", 589)]
23+
[InlineData("DELETE /a%20b%20c/d%20e?f=ghi HTTP/1.1\r\n\r\n", 40)]
24+
[InlineData("DELETE /a%20b%20c/d%20e?f=ghi HTTP/1.1\r\n\r\n", 41)]
25+
[InlineData("DELETE /a%20b%20c/d%20e?f=ghi HTTP/1.1\r\n\r\n", 1027)]
26+
public async Task ServerAcceptsRequestLineWithinLimit(string request, int limit)
2727
{
2828
var maxRequestLineSize = limit;
2929

3030
using (var server = CreateServer(limit))
3131
{
3232
using (var connection = new TestConnection(server.Port))
3333
{
34-
await connection.SendEnd($"{requestLine}\r\n");
34+
await connection.SendEnd(request);
3535
await connection.ReceiveEnd(
3636
"HTTP/1.1 200 OK",
3737
$"Date: {server.Context.DateHeaderValue}",

‎test/Microsoft.AspNetCore.Server.KestrelTests/BadHttpRequestTests.cs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,36 @@ public async Task BadRequestIfPathContainsNullCharacters(string path)
174174
}
175175
}
176176

177+
[Theory]
178+
[InlineData("POST")]
179+
[InlineData("PUT")]
180+
public async Task BadRequestIfMethodRequiresLengthButNoContentLengthOrTransferEncodingInRequest(string method)
181+
{
182+
using (var server = new TestServer(context => { return Task.FromResult(0); }))
183+
{
184+
using (var connection = server.CreateConnection())
185+
{
186+
await connection.SendEnd($"{method} / HTTP/1.1\r\n\r\n");
187+
await ReceiveBadRequestResponse(connection, "411 Length Required", server.Context.DateHeaderValue);
188+
}
189+
}
190+
}
191+
192+
[Theory]
193+
[InlineData("POST")]
194+
[InlineData("PUT")]
195+
public async Task BadRequestIfMethodRequiresLengthButNoContentLengthInHttp10Request(string method)
196+
{
197+
using (var server = new TestServer(context => { return Task.FromResult(0); }))
198+
{
199+
using (var connection = server.CreateConnection())
200+
{
201+
await connection.SendEnd($"{method} / HTTP/1.0\r\n\r\n");
202+
await ReceiveBadRequestResponse(connection, "400 Bad Request", server.Context.DateHeaderValue);
203+
}
204+
}
205+
}
206+
177207
private async Task ReceiveBadRequestResponse(TestConnection connection, string expectedResponseStatusCode, string expectedDateHeaderValue)
178208
{
179209
await connection.ReceiveForcedEnd(

‎test/Microsoft.AspNetCore.Server.KestrelTests/MessageBodyTests.cs

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,13 +196,45 @@ public void ForThrowsWhenFinalTransferCodingIsNotChunked()
196196
using (var input = new TestInput())
197197
{
198198
var ex = Assert.Throws<BadHttpRequestException>(() =>
199-
MessageBody.For(HttpVersion.Http10, new FrameRequestHeaders { HeaderTransferEncoding = "chunked, not-chunked" }, input.FrameContext));
199+
MessageBody.For(HttpVersion.Http11, new FrameRequestHeaders { HeaderTransferEncoding = "chunked, not-chunked" }, input.FrameContext));
200200

201201
Assert.Equal(400, ex.StatusCode);
202202
Assert.Equal("Final transfer coding is not \"chunked\": \"chunked, not-chunked\"", ex.Message);
203203
}
204204
}
205205

206+
[Theory]
207+
[InlineData("POST")]
208+
[InlineData("PUT")]
209+
public void ForThrowsWhenMethodRequiresLengthButNoContentLengthOrTransferEncodingIsSet(string method)
210+
{
211+
using (var input = new TestInput())
212+
{
213+
input.FrameContext.Method = method;
214+
var ex = Assert.Throws<BadHttpRequestException>(() =>
215+
MessageBody.For(HttpVersion.Http11, new FrameRequestHeaders(), input.FrameContext));
216+
217+
Assert.Equal(411, ex.StatusCode);
218+
Assert.Equal($"{method} request contains no Content-Length or Transfer-Encoding header", ex.Message);
219+
}
220+
}
221+
222+
[Theory]
223+
[InlineData("POST")]
224+
[InlineData("PUT")]
225+
public void ForThrowsWhenMethodRequiresLengthButNoContentLengthSetHttp10(string method)
226+
{
227+
using (var input = new TestInput())
228+
{
229+
input.FrameContext.Method = method;
230+
var ex = Assert.Throws<BadHttpRequestException>(() =>
231+
MessageBody.For(HttpVersion.Http10, new FrameRequestHeaders(), input.FrameContext));
232+
233+
Assert.Equal(400, ex.StatusCode);
234+
Assert.Equal($"{method} request contains no Content-Length header", ex.Message);
235+
}
236+
}
237+
206238
public static IEnumerable<object[]> StreamData => new[]
207239
{
208240
new object[] { new ThrowOnWriteSynchronousStream() },

0 commit comments

Comments
 (0)