Chunked request parsing does not properly update examined (#8318)

- The chunked parsing logic didn't properly update the examined position when parsing the chunked prefix. This started to throw because Pipe now throws if examined is set to the position before the previous.
This commit is contained in:
David Fowler 2019-03-08 09:42:02 -08:00 committed by GitHub
parent ce04a1a723
commit bd6faa5ca1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 96 additions and 2 deletions

View File

@ -305,7 +305,7 @@
<Uri>https://github.com/dotnet/corefx</Uri>
<Sha>00000</Sha>
</Dependency>
<Dependency Name="System.IO.Pipelines" Version="4.6.0-preview4.19127.11">
<Dependency Name="System.IO.Pipelines" Version="4.6.0-preview4.19156.2">
<Uri>https://github.com/dotnet/corefx</Uri>
<Sha>00000</Sha>
</Dependency>

View File

@ -29,7 +29,7 @@
<SystemComponentModelAnnotationsPackageVersion>4.6.0-preview4.19156.2</SystemComponentModelAnnotationsPackageVersion>
<SystemDataSqlClientPackageVersion>4.7.0-preview4.19156.2</SystemDataSqlClientPackageVersion>
<SystemDiagnosticsEventLogPackageVersion>4.6.0-preview4.19156.2</SystemDiagnosticsEventLogPackageVersion>
<SystemIOPipelinesPackageVersion>4.6.0-preview4.19127.11</SystemIOPipelinesPackageVersion>
<SystemIOPipelinesPackageVersion>4.6.0-preview4.19156.2</SystemIOPipelinesPackageVersion>
<SystemNetHttpWinHttpHandlerPackageVersion>4.6.0-preview4.19156.2</SystemNetHttpWinHttpHandlerPackageVersion>
<SystemNetWebSocketsWebSocketProtocolPackageVersion>4.6.0-preview4.19156.2</SystemNetWebSocketsWebSocketProtocolPackageVersion>
<SystemReflectionMetadataPackageVersion>1.7.0-preview4.19156.2</SystemReflectionMetadataPackageVersion>

View File

@ -323,6 +323,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
return;
}
// Assigned this before calculating the chunk size since that can throw
examined = reader.Position;
var chunkSize = CalculateChunkSize(ch1, 0);
ch1 = ch2;
@ -360,6 +363,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
ch1 = ch2;
}
// Set examined so that we capture the progress that way made
examined = reader.Position;
// At this point, 10 bytes have been consumed which is enough to parse the max value "7FFFFFFF\r\n".
BadHttpRequestException.Throw(RequestRejectionReason.BadChunkSizeData);
}

View File

@ -198,6 +198,94 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
}
}
[Fact]
public async Task BadChunkPrefixThrowsBadRequestException()
{
using (var input = new TestInput())
{
var body = Http1MessageBody.For(HttpVersion.Http11, new HttpRequestHeaders { HeaderTransferEncoding = "chunked" }, input.Http1Connection);
var mockBodyControl = new Mock<IHttpBodyControlFeature>();
mockBodyControl.Setup(m => m.AllowSynchronousIO).Returns(true);
var reader = new HttpRequestPipeReader();
var stream = new HttpRequestStream(mockBodyControl.Object, reader);
reader.StartAcceptingReads(body);
var buffer = new byte[1024];
var task = stream.ReadAsync(buffer, 0, buffer.Length);
input.Add("g");
input.Add("g");
await Assert.ThrowsAsync<BadHttpRequestException>(() => task);
await body.StopAsync();
}
}
[Fact]
public async Task WritingChunkOverMaxChunkSizeThrowsBadRequest()
{
using (var input = new TestInput())
{
var body = Http1MessageBody.For(HttpVersion.Http11, new HttpRequestHeaders { HeaderTransferEncoding = "chunked" }, input.Http1Connection);
var mockBodyControl = new Mock<IHttpBodyControlFeature>();
mockBodyControl.Setup(m => m.AllowSynchronousIO).Returns(true);
var reader = new HttpRequestPipeReader();
var stream = new HttpRequestStream(mockBodyControl.Object, reader);
reader.StartAcceptingReads(body);
var buffer = new byte[1024];
var task = stream.ReadAsync(buffer, 0, buffer.Length);
// Max is 10 bytes
for (int i = 0; i < 11; i++)
{
input.Add(i.ToString());
}
await Assert.ThrowsAsync<BadHttpRequestException>(() => task);
await body.StopAsync();
}
}
[Fact]
public async Task InvalidChunkSuffixThrowsBadRequest()
{
using (var input = new TestInput())
{
var body = Http1MessageBody.For(HttpVersion.Http11, new HttpRequestHeaders { HeaderTransferEncoding = "chunked" }, input.Http1Connection);
var mockBodyControl = new Mock<IHttpBodyControlFeature>();
mockBodyControl.Setup(m => m.AllowSynchronousIO).Returns(true);
var reader = new HttpRequestPipeReader();
var stream = new HttpRequestStream(mockBodyControl.Object, reader);
reader.StartAcceptingReads(body);
var buffer = new byte[1024];
async Task ReadAsync()
{
while (true)
{
await stream.ReadAsync(buffer, 0, buffer.Length);
}
}
var task = ReadAsync();
input.Add("1");
input.Add("\r");
input.Add("\n");
input.Add("h");
input.Add("0");
input.Add("\r");
input.Add("\n");
input.Add("\r");
input.Add("n");
await Assert.ThrowsAsync<BadHttpRequestException>(() => task);
await body.StopAsync();
}
}
[Fact]
public async Task CanReadAsyncFromChunkedEncoding()
{