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

Commit 9c4f978

Browse files
author
Cesar Blum Silveira
committed
Handle response content length mismatches (#175).
1 parent 663b825 commit 9c4f978

File tree

8 files changed

+269
-67
lines changed

8 files changed

+269
-67
lines changed

src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,8 @@ public abstract partial class Frame : IFrameControl
7575
protected readonly long _keepAliveMilliseconds;
7676
private readonly long _requestHeadersTimeoutMilliseconds;
7777

78-
private int _responseBytesWritten;
78+
protected long? _responseContentLength;
79+
protected long _responseBytesWritten;
7980

8081
public Frame(ConnectionContext context)
8182
{
@@ -116,7 +117,7 @@ public Action<IFeatureCollection> PrepareRequest
116117
private KestrelServerOptions ServerOptions { get; }
117118
private IPEndPoint LocalEndPoint => ConnectionContext.LocalEndPoint;
118119
private IPEndPoint RemoteEndPoint => ConnectionContext.RemoteEndPoint;
119-
private string ConnectionId => ConnectionContext.ConnectionId;
120+
protected string ConnectionId => ConnectionContext.ConnectionId;
120121

121122
public string ConnectionIdFeature { get; set; }
122123
public IPAddress RemoteIpAddress { get; set; }
@@ -350,6 +351,7 @@ public void Reset()
350351
_remainingRequestHeadersBytesAllowed = ServerOptions.Limits.MaxRequestHeadersTotalSize;
351352
_requestHeadersParsed = 0;
352353

354+
_responseContentLength = null;
353355
_responseBytesWritten = 0;
354356
}
355357

@@ -516,8 +518,9 @@ public async Task FlushAsync(CancellationToken cancellationToken)
516518

517519
public void Write(ArraySegment<byte> data)
518520
{
519-
ProduceStartAndFireOnStarting().GetAwaiter().GetResult();
520521
_responseBytesWritten += data.Count;
522+
VerifyWrite();
523+
ProduceStartAndFireOnStarting().GetAwaiter().GetResult();
521524

522525
if (_canHaveBody)
523526
{
@@ -548,6 +551,7 @@ public Task WriteAsync(ArraySegment<byte> data, CancellationToken cancellationTo
548551
}
549552

550553
_responseBytesWritten += data.Count;
554+
VerifyWrite();
551555

552556
if (_canHaveBody)
553557
{
@@ -573,8 +577,10 @@ public Task WriteAsync(ArraySegment<byte> data, CancellationToken cancellationTo
573577

574578
public async Task WriteAsyncAwaited(ArraySegment<byte> data, CancellationToken cancellationToken)
575579
{
576-
await ProduceStartAndFireOnStarting();
577580
_responseBytesWritten += data.Count;
581+
VerifyWrite();
582+
583+
await ProduceStartAndFireOnStarting();
578584

579585
if (_canHaveBody)
580586
{
@@ -598,6 +604,34 @@ public async Task WriteAsyncAwaited(ArraySegment<byte> data, CancellationToken c
598604
}
599605
}
600606

607+
private void VerifyWrite()
608+
{
609+
TryInitializeResponseContentLength();
610+
611+
if (_responseContentLength.HasValue && _responseBytesWritten > _responseContentLength.Value)
612+
{
613+
ThrowContentLengthMismatchException();
614+
}
615+
}
616+
617+
protected void TryInitializeResponseContentLength()
618+
{
619+
if (!_responseContentLength.HasValue)
620+
{
621+
var responseHeaders = FrameResponseHeaders;
622+
if (responseHeaders != null && responseHeaders.HasContentLength)
623+
{
624+
_responseContentLength = int.Parse(responseHeaders.HeaderContentLength.ToString());
625+
}
626+
}
627+
}
628+
629+
protected void ThrowContentLengthMismatchException()
630+
{
631+
_keepAlive = false;
632+
throw new InvalidOperationException($"Response content length mismatch: wrote {_responseBytesWritten} bytes to {_responseContentLength.Value}-byte response.");
633+
}
634+
601635
private void WriteChunked(ArraySegment<byte> data)
602636
{
603637
SocketOutput.Write(data, chunk: true);
@@ -684,7 +718,7 @@ private void ProduceStart(bool appCompleted)
684718

685719
protected Task TryProduceInvalidRequestResponse()
686720
{
687-
if (_requestRejected)
721+
if (_requestRejected || _applicationException != null)
688722
{
689723
if (FrameRequestHeaders == null || FrameResponseHeaders == null)
690724
{

src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/FrameOfT.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,13 @@ public override async Task RequestProcessingAsync()
9292
try
9393
{
9494
await _application.ProcessRequestAsync(context).ConfigureAwait(false);
95+
96+
TryInitializeResponseContentLength();
97+
98+
if (_responseContentLength.HasValue && _responseBytesWritten < _responseContentLength.Value)
99+
{
100+
ThrowContentLengthMismatchException();
101+
}
95102
}
96103
catch (Exception ex)
97104
{

src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/IKestrelTrace.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public interface IKestrelTrace : ILogger
3333

3434
void ConnectionDisconnectedWrite(string connectionId, int count, Exception ex);
3535

36-
void ConnectionHeadResponseBodyWrite(string connectionId, int count);
36+
void ConnectionHeadResponseBodyWrite(string connectionId, long count);
3737

3838
void ConnectionBadRequest(string connectionId, BadHttpRequestException ex);
3939

src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/KestrelTrace.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public class KestrelTrace : IKestrelTrace
2424
private static readonly Action<ILogger, string, Exception> _applicationError;
2525
private static readonly Action<ILogger, string, Exception> _connectionError;
2626
private static readonly Action<ILogger, string, int, Exception> _connectionDisconnectedWrite;
27-
private static readonly Action<ILogger, string, int, Exception> _connectionHeadResponseBodyWrite;
27+
private static readonly Action<ILogger, string, long, Exception> _connectionHeadResponseBodyWrite;
2828
private static readonly Action<ILogger, Exception> _notAllConnectionsClosedGracefully;
2929
private static readonly Action<ILogger, string, string, Exception> _connectionBadRequest;
3030

@@ -49,7 +49,7 @@ static KestrelTrace()
4949
_connectionDisconnectedWrite = LoggerMessage.Define<string, int>(LogLevel.Debug, 15, @"Connection id ""{ConnectionId}"" write of ""{count}"" bytes to disconnected client.");
5050
_notAllConnectionsClosedGracefully = LoggerMessage.Define(LogLevel.Debug, 16, "Some connections failed to close gracefully during server shutdown.");
5151
_connectionBadRequest = LoggerMessage.Define<string, string>(LogLevel.Information, 17, @"Connection id ""{ConnectionId}"" bad request data: ""{message}""");
52-
_connectionHeadResponseBodyWrite = LoggerMessage.Define<string, int>(LogLevel.Debug, 18, @"Connection id ""{ConnectionId}"" write of ""{count}"" body bytes to non-body HEAD response.");
52+
_connectionHeadResponseBodyWrite = LoggerMessage.Define<string, long>(LogLevel.Debug, 18, @"Connection id ""{ConnectionId}"" write of ""{count}"" body bytes to non-body HEAD response.");
5353
}
5454

5555
public KestrelTrace(ILogger logger)
@@ -135,7 +135,7 @@ public virtual void ConnectionDisconnectedWrite(string connectionId, int count,
135135
_connectionDisconnectedWrite(_logger, connectionId, count, ex);
136136
}
137137

138-
public virtual void ConnectionHeadResponseBodyWrite(string connectionId, int count)
138+
public virtual void ConnectionHeadResponseBodyWrite(string connectionId, long count)
139139
{
140140
_connectionHeadResponseBodyWrite(_logger, connectionId, count, null);
141141
}

test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/ResponseTests.cs

Lines changed: 205 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
5+
using System.IO;
56
using System.Linq;
67
using System.Net;
78
using System.Net.Http;
@@ -14,6 +15,7 @@
1415
using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure;
1516
using Microsoft.AspNetCore.Testing;
1617
using Microsoft.Extensions.Internal;
18+
using Microsoft.Extensions.Logging;
1719
using Microsoft.Extensions.Primitives;
1820
using Moq;
1921
using Xunit;
@@ -85,7 +87,7 @@ public async Task IgnoreNullHeaderValues(string headerName, StringValues headerV
8587
app.Run(async context =>
8688
{
8789
context.Response.Headers.Add(headerName, headerValue);
88-
90+
8991
await context.Response.WriteAsync("");
9092
});
9193
});
@@ -299,7 +301,7 @@ await connection.Receive(
299301
}
300302

301303
[Fact]
302-
public async Task ResponseBodyNotWrittenOnHeadResponse()
304+
public async Task ResponseBodyNotWrittenOnHeadResponseAndLoggedOnlyOnce()
303305
{
304306
var mockKestrelTrace = new Mock<IKestrelTrace>();
305307

@@ -324,7 +326,207 @@ await connection.Receive(
324326
}
325327

326328
mockKestrelTrace.Verify(kestrelTrace =>
327-
kestrelTrace.ConnectionHeadResponseBodyWrite(It.IsAny<string>(), "hello, world".Length));
329+
kestrelTrace.ConnectionHeadResponseBodyWrite(It.IsAny<string>(), "hello, world".Length), Times.Once);
330+
}
331+
332+
[Fact]
333+
public async Task WhenAppWritesMoreThanContentLengthWriteThrowsAndConnectionCloses()
334+
{
335+
var testLogger = new TestApplicationErrorLogger();
336+
var serviceContext = new TestServiceContext { Log = new TestKestrelTrace(testLogger) };
337+
338+
using (var server = new TestServer(httpContext =>
339+
{
340+
httpContext.Response.ContentLength = 11;
341+
httpContext.Response.Body.Write(Encoding.ASCII.GetBytes("hello,"), 0, 6);
342+
httpContext.Response.Body.Write(Encoding.ASCII.GetBytes(" world"), 0, 6);
343+
return TaskCache.CompletedTask;
344+
}, serviceContext))
345+
{
346+
using (var connection = server.CreateConnection())
347+
{
348+
await connection.Send(
349+
"GET / HTTP/1.1",
350+
"",
351+
"");
352+
await connection.ReceiveEnd(
353+
$"HTTP/1.1 200 OK",
354+
$"Date: {server.Context.DateHeaderValue}",
355+
"Content-Length: 11",
356+
"",
357+
"hello,");
358+
}
359+
}
360+
361+
var logMessage = Assert.Single(testLogger.Messages, message => message.LogLevel == LogLevel.Error);
362+
Assert.Equal(
363+
$"Response content length mismatch: wrote 12 bytes to 11-byte response.",
364+
logMessage.Exception.Message);
365+
}
366+
367+
[Fact]
368+
public async Task WhenAppWritesMoreThanContentLengthWriteAsyncThrowsAndConnectionCloses()
369+
{
370+
var testLogger = new TestApplicationErrorLogger();
371+
var serviceContext = new TestServiceContext { Log = new TestKestrelTrace(testLogger) };
372+
373+
using (var server = new TestServer(async httpContext =>
374+
{
375+
httpContext.Response.ContentLength = 11;
376+
await httpContext.Response.WriteAsync("hello,");
377+
await httpContext.Response.WriteAsync(" world");
378+
}, serviceContext))
379+
{
380+
using (var connection = server.CreateConnection())
381+
{
382+
await connection.Send(
383+
"GET / HTTP/1.1",
384+
"",
385+
"");
386+
await connection.ReceiveEnd(
387+
$"HTTP/1.1 200 OK",
388+
$"Date: {server.Context.DateHeaderValue}",
389+
"Content-Length: 11",
390+
"",
391+
"hello,");
392+
}
393+
}
394+
395+
var logMessage = Assert.Single(testLogger.Messages, message => message.LogLevel == LogLevel.Error);
396+
Assert.Equal(
397+
$"Response content length mismatch: wrote 12 bytes to 11-byte response.",
398+
logMessage.Exception.Message);
399+
}
400+
401+
[Fact]
402+
public async Task WhenAppWritesMoreThanContentLengthAndResponseNotStarted500ResponseSentAndConnectionCloses()
403+
{
404+
var testLogger = new TestApplicationErrorLogger();
405+
var serviceContext = new TestServiceContext { Log = new TestKestrelTrace(testLogger) };
406+
407+
using (var server = new TestServer(async httpContext =>
408+
{
409+
httpContext.Response.ContentLength = 5;
410+
await httpContext.Response.WriteAsync("hello, world");
411+
}, serviceContext))
412+
{
413+
using (var connection = server.CreateConnection())
414+
{
415+
await connection.Send(
416+
"GET / HTTP/1.1",
417+
"",
418+
"");
419+
await connection.ReceiveEnd(
420+
$"HTTP/1.1 500 Internal Server Error",
421+
"Connection: close",
422+
$"Date: {server.Context.DateHeaderValue}",
423+
"Content-Length: 0",
424+
"",
425+
"");
426+
}
427+
}
428+
429+
var logMessage = Assert.Single(testLogger.Messages, message => message.LogLevel == LogLevel.Error);
430+
Assert.Equal(
431+
$"Response content length mismatch: wrote 12 bytes to 5-byte response.",
432+
logMessage.Exception.Message);
433+
}
434+
435+
[Fact]
436+
public async Task WhenAppWritesLessThanContentLengthErrorLogged()
437+
{
438+
var testLogger = new TestApplicationErrorLogger();
439+
var serviceContext = new TestServiceContext { Log = new TestKestrelTrace(testLogger) };
440+
441+
using (var server = new TestServer(async httpContext =>
442+
{
443+
httpContext.Response.ContentLength = 13;
444+
await httpContext.Response.WriteAsync("hello, world");
445+
}, serviceContext))
446+
{
447+
using (var connection = server.CreateConnection())
448+
{
449+
await connection.Send(
450+
"GET / HTTP/1.1",
451+
"",
452+
"");
453+
await connection.ReceiveEnd(
454+
$"HTTP/1.1 200 OK",
455+
$"Date: {server.Context.DateHeaderValue}",
456+
"Content-Length: 13",
457+
"",
458+
"hello, world");
459+
}
460+
}
461+
462+
var errorMessage = Assert.Single(testLogger.Messages, message => message.LogLevel == LogLevel.Error);
463+
Assert.Equal(
464+
$"Response content length mismatch: wrote 12 bytes to 13-byte response.",
465+
errorMessage.Exception.Message);
466+
}
467+
468+
[Fact]
469+
public async Task WhenAppSetsContentLengthButDoesNotWriteBody500ResponseSentAndConnectionCloses()
470+
{
471+
var testLogger = new TestApplicationErrorLogger();
472+
var serviceContext = new TestServiceContext { Log = new TestKestrelTrace(testLogger) };
473+
474+
using (var server = new TestServer(httpContext =>
475+
{
476+
httpContext.Response.ContentLength = 5;
477+
return TaskCache.CompletedTask;
478+
}, serviceContext))
479+
{
480+
using (var connection = server.CreateConnection())
481+
{
482+
await connection.Send(
483+
"GET / HTTP/1.1",
484+
"",
485+
"");
486+
await connection.ReceiveEnd(
487+
$"HTTP/1.1 500 Internal Server Error",
488+
"Connection: close",
489+
$"Date: {server.Context.DateHeaderValue}",
490+
"Content-Length: 0",
491+
"",
492+
"");
493+
}
494+
}
495+
496+
var errorMessage = Assert.Single(testLogger.Messages, message => message.LogLevel == LogLevel.Error);
497+
Assert.Equal(
498+
$"Response content length mismatch: wrote 0 bytes to 5-byte response.",
499+
errorMessage.Exception.Message);
500+
}
501+
502+
[Fact]
503+
public async Task WhenAppSetsContentLengthToZeroAndDoesNotWriteNoErrorIsThrown()
504+
{
505+
var testLogger = new TestApplicationErrorLogger();
506+
var serviceContext = new TestServiceContext { Log = new TestKestrelTrace(testLogger) };
507+
508+
using (var server = new TestServer(httpContext =>
509+
{
510+
httpContext.Response.ContentLength = 0;
511+
return TaskCache.CompletedTask;
512+
}, serviceContext))
513+
{
514+
using (var connection = server.CreateConnection())
515+
{
516+
await connection.Send(
517+
"GET / HTTP/1.1",
518+
"",
519+
"");
520+
await connection.Receive(
521+
$"HTTP/1.1 200 OK",
522+
$"Date: {server.Context.DateHeaderValue}",
523+
"Content-Length: 0",
524+
"",
525+
"");
526+
}
527+
}
528+
529+
Assert.DoesNotContain(testLogger.Messages, message => message.LogLevel == LogLevel.Error);
328530
}
329531

330532
public static TheoryData<string, StringValues, string> NullHeaderData

0 commit comments

Comments
 (0)