Skip to content

Commit

Permalink
Give SocketsHttpHandler more descriptive error messages (dotnet/coref…
Browse files Browse the repository at this point in the history 
…x#37251)

Currently lots of parsing failures just result in an exception stating "The server returned an invalid or unrecognized response."  We can instead be more descriptive about what the problem is in most cases.

Commit migrated from dotnet/corefx@8e551d4
  • Loading branch information
@stephentoub
stephentoub authored Apr 27, 2019
1 parent 3219126 commit 7058903
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 43 deletions.
42 changes: 42 additions & 0 deletions src/libraries/System.Net.Http/src/Resources/Strings.resx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,45 @@
<data name="net_http_invalid_response" xml:space="preserve">
<value>The server returned an invalid or unrecognized response.</value>
</data>
<data name="net_http_invalid_response_premature_eof" xml:space="preserve">
<value>The response ended prematurely.</value>
</data>
<data name="net_http_invalid_response_premature_eof_bytecount" xml:space="preserve">
<value>The response ended prematurely, with at least {0} additional bytes expected.</value>
</data>
<data name="net_http_invalid_response_chunk_header_invalid" xml:space="preserve">
<value>Received chunk header length could not be parsed: '{0}'.</value>
</data>
<data name="net_http_invalid_response_chunk_extension_invalid" xml:space="preserve">
<value>Received an invalid chunk extension: '{0}'.</value>
</data>
<data name="net_http_invalid_response_chunk_terminator_invalid" xml:space="preserve">
<value>Received an invalid chunk terminator: '{0}'.</value>
</data>
<data name="net_http_invalid_response_status_line" xml:space="preserve">
<value>Received an invalid status line: '{0}'.</value>
</data>
<data name="net_http_invalid_response_status_code" xml:space="preserve">
<value>Received an invalid status code: '{0}'.</value>
</data>
<data name="net_http_invalid_response_status_reason" xml:space="preserve">
<value>Received status phrase could not be decoded with iso-8859-1: '{0}'.</value>
</data>
<data name="net_http_invalid_response_header_folder" xml:space="preserve">
<value>Received an invalid folded header.</value>
</data>
<data name="net_http_invalid_response_header_line" xml:space="preserve">
<value>Received an invalid header line: '{0}'.</value>
</data>
<data name="net_http_invalid_response_header_name" xml:space="preserve">
<value>Received an invalid header name: '{0}'.</value>
</data>
<data name="net_http_request_aborted" xml:space="preserve">
<value>The request was aborted.</value>
</data>
<data name="net_http_invalid_response_pseudo_header_in_trailer" xml:space="preserve">
<value>Received an HTTP/2 pseudo-header as a trailing header.</value>
</data>
<data name="net_http_unix_handler_disposed" xml:space="preserve">
<value>The handler was disposed of while active operations were in progress.</value>
</data>
Expand Down Expand Up @@ -392,6 +431,9 @@
<data name="net_ssl_app_protocols_invalid" xml:space="preserve">
<value>The application protocol list is invalid.</value>
</data>
<data name="net_ssl_http2_requires_tls12" xml:space="preserve">
<value>HTTP/2 requires TLS 1.2 or newer, but '{0}' was negotiated.</value>
</data>
<data name="IO_PathTooLong_Path" xml:space="preserve">
<value>The path '{0}' is too long, or a component of the specified path is too long.</value>
</data>
Expand Down
11 changes: 6 additions & 5 deletions ...aries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ChunkedEncodingReadStream.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Buffers.Text;
using System.Diagnostics;
using System.IO;
using System.Text;
using System.Threading;
using System.Threading.Tasks;

Expand Down Expand Up @@ -71,7 +72,7 @@ public override int Read(Span<byte> buffer)
bytesRead = _connection.Read(buffer.Slice(0, (int)Math.Min((ulong)buffer.Length, _chunkBytesRemaining)));
if (bytesRead == 0)
{
throw new IOException(SR.net_http_invalid_response);
throw new IOException(SR.Format(SR.net_http_invalid_response_premature_eof_bytecount, _chunkBytesRemaining));
}
_chunkBytesRemaining -= (ulong)bytesRead;
if (_chunkBytesRemaining == 0)
Expand Down Expand Up @@ -157,7 +158,7 @@ private async ValueTask<int> ReadAsyncCore(Memory<byte> buffer, CancellationToke
int bytesRead = await _connection.ReadAsync(buffer.Slice(0, (int)Math.Min((ulong)buffer.Length, _chunkBytesRemaining))).ConfigureAwait(false);
if (bytesRead == 0)
{
throw new IOException(SR.net_http_invalid_response);
throw new IOException(SR.Format(SR.net_http_invalid_response_premature_eof_bytecount, _chunkBytesRemaining));
}
_chunkBytesRemaining -= (ulong)bytesRead;
if (_chunkBytesRemaining == 0)
Expand Down Expand Up @@ -277,7 +278,7 @@ private ReadOnlyMemory<byte> ReadChunkFromConnectionBuffer(int maxBytesToRead, C
// Parse the hex value from it.
if (!Utf8Parser.TryParse(currentLine, out ulong chunkSize, out int bytesConsumed, 'X'))
{
throw new IOException(SR.net_http_invalid_response);
throw new IOException(SR.Format(SR.net_http_invalid_response_chunk_header_invalid, BitConverter.ToString(currentLine.ToArray())));
}
_chunkBytesRemaining = chunkSize;

Expand Down Expand Up @@ -332,7 +333,7 @@ private ReadOnlyMemory<byte> ReadChunkFromConnectionBuffer(int maxBytesToRead, C

if (currentLine.Length != 0)
{
ThrowInvalidHttpResponse();
throw new HttpRequestException(SR.Format(SR.net_http_invalid_response_chunk_terminator_invalid, Encoding.ASCII.GetString(currentLine)));
}

_state = ParsingState.ExpectChunkHeader;
Expand Down Expand Up @@ -412,7 +413,7 @@ private static void ValidateChunkExtension(ReadOnlySpan<byte> lineAfterChunkSize
}
else if (c != ' ' && c != '\t') // not called out in the RFC, but WinHTTP allows it
{
throw new IOException(SR.net_http_invalid_response);
throw new IOException(SR.Format(SR.net_http_invalid_response_chunk_extension_invalid, BitConverter.ToString(lineAfterChunkSize.ToArray())));
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions ...braries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ContentLengthReadStream.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public override int Read(Span<byte> buffer)
if (bytesRead <= 0)
{
// Unexpected end of response stream.
throw new IOException(SR.net_http_invalid_response);
throw new IOException(SR.Format(SR.net_http_invalid_response_premature_eof_bytecount, _contentBytesRemaining));
}

Debug.Assert((ulong)bytesRead <= _contentBytesRemaining);
Expand Down Expand Up @@ -101,7 +101,7 @@ public override async ValueTask<int> ReadAsync(Memory<byte> buffer, Cancellation
CancellationHelper.ThrowIfCancellationRequested(cancellationToken);

// Unexpected end of response stream.
throw new IOException(SR.net_http_invalid_response);
throw new IOException(SR.Format(SR.net_http_invalid_response_premature_eof_bytecount, _contentBytesRemaining));
}

Debug.Assert((ulong)bytesRead <= _contentBytesRemaining);
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,7 @@ private async Task AcquireWriteLockAsync(CancellationToken cancellationToken)
// If the connection has been aborted, then fail now instead of trying to send more data.
if (IsAborted())
{
throw new IOException(SR.net_http_invalid_response);
throw new IOException(SR.net_http_request_aborted);
}
}

Expand Down Expand Up @@ -1486,7 +1486,7 @@ internal static async ValueTask<int> ReadAtLeastAsync(Stream stream, Memory<byte
int bytesRead = await stream.ReadAsync(buffer.Slice(totalBytesRead)).ConfigureAwait(false);
if (bytesRead == 0)
{
throw new IOException(SR.net_http_invalid_response);
throw new IOException(SR.Format(SR.net_http_invalid_response_premature_eof_bytecount, minReadBytes));
}

totalBytesRead += bytesRead;
Expand Down
21 changes: 10 additions & 11 deletions src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,17 +135,16 @@ public void OnResponseHeader(ReadOnlySpan<byte> name, ReadOnlySpan<byte> value)
{
// Pseudo-headers not allowed in trailers.
if (NetEventSource.IsEnabled) _connection.Trace("Pseudo-header in trailer headers.");
throw new HttpRequestException(SR.net_http_invalid_response);
throw new HttpRequestException(SR.net_http_invalid_response_pseudo_header_in_trailer);
}

if (value.Length != 3)
throw new Exception("Invalid status code");

// Copied from HttpConnection
byte status1 = value[0], status2 = value[1], status3 = value[2];
if (!IsDigit(status1) || !IsDigit(status2) || !IsDigit(status3))
byte status1, status2, status3;
if (value.Length != 3 ||
!IsDigit(status1 = value[0]) ||
!IsDigit(status2 = value[1]) ||
!IsDigit(status3 = value[2]))
{
throw new HttpRequestException(SR.net_http_invalid_response);
throw new HttpRequestException(SR.Format(SR.net_http_invalid_response_status_code, Encoding.ASCII.GetString(value)));
}

_response.SetStatusCodeWithoutValidation((HttpStatusCode)(100 * (status1 - '0') + 10 * (status2 - '0') + (status3 - '0')));
Expand All @@ -155,7 +154,7 @@ public void OnResponseHeader(ReadOnlySpan<byte> name, ReadOnlySpan<byte> value)
if (!HeaderDescriptor.TryGet(name, out HeaderDescriptor descriptor))
{
// Invalid header name
throw new HttpRequestException(SR.net_http_invalid_response);
throw new HttpRequestException(SR.Format(SR.net_http_invalid_response_header_name, Encoding.ASCII.GetString(name)));
}

string headerValue = descriptor.GetHeaderValue(value);
Expand Down Expand Up @@ -301,7 +300,7 @@ public void OnResponseAbort()

if (_state == StreamState.Aborted)
{
throw new IOException(SR.net_http_invalid_response);
throw new IOException(SR.net_http_request_aborted);
}
else if (_state == StreamState.ExpectingHeaders)
{
Expand Down Expand Up @@ -394,7 +393,7 @@ private void ExtendWindow(int amount)
}
else if (_state == StreamState.Aborted)
{
throw new IOException(SR.net_http_invalid_response);
throw new IOException(SR.net_http_request_aborted);
}

Debug.Assert(_state == StreamState.ExpectingData || _state == StreamState.ExpectingTrailingHeaders);
Expand Down
36 changes: 16 additions & 20 deletions src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,7 @@ public async Task<HttpResponseMessage> SendAsyncCore(HttpRequestMessage request,

if (bytesRead == 0)
{
throw new IOException(SR.net_http_invalid_response);
throw new IOException(SR.net_http_invalid_response_premature_eof);
}

_readOffset = 0;
Expand Down Expand Up @@ -824,7 +824,7 @@ private static void ParseStatusLine(Span<byte> line, HttpResponseMessage respons
const int MinStatusLineLength = 12; // "HTTP/1.x 123"
if (line.Length < MinStatusLineLength || line[8] != ' ')
{
ThrowInvalidHttpResponse();
throw new HttpRequestException(SR.Format(SR.net_http_invalid_response_status_line, Encoding.ASCII.GetString(line)));
}

ulong first8Bytes = BitConverter.ToUInt64(line);
Expand All @@ -846,15 +846,15 @@ private static void ParseStatusLine(Span<byte> line, HttpResponseMessage respons
}
else
{
ThrowInvalidHttpResponse();
throw new HttpRequestException(SR.Format(SR.net_http_invalid_response_status_line, Encoding.ASCII.GetString(line)));
}
}

// Set the status code
byte status1 = line[9], status2 = line[10], status3 = line[11];
if (!IsDigit(status1) || !IsDigit(status2) || !IsDigit(status3))
{
ThrowInvalidHttpResponse();
throw new HttpRequestException(SR.Format(SR.net_http_invalid_response_status_code, Encoding.ASCII.GetString(line.Slice(9, 3))));
}
response.SetStatusCodeWithoutValidation((HttpStatusCode)(100 * (status1 - '0') + 10 * (status2 - '0') + (status3 - '0')));

Expand All @@ -879,13 +879,13 @@ private static void ParseStatusLine(Span<byte> line, HttpResponseMessage respons
}
catch (FormatException error)
{
ThrowInvalidHttpResponse(error);
throw new HttpRequestException(SR.Format(SR.net_http_invalid_response_status_reason, Encoding.ASCII.GetString(reasonBytes.ToArray())), error);
}
}
}
else
{
ThrowInvalidHttpResponse();
throw new HttpRequestException(SR.Format(SR.net_http_invalid_response_status_line, Encoding.ASCII.GetString(line)));
}
}

Expand All @@ -905,20 +905,20 @@ private static void ParseHeaderNameValue(HttpConnection connection, ReadOnlySpan
if (pos == line.Length)
{
// Invalid header line that doesn't contain ':'.
ThrowInvalidHttpResponse();
throw new HttpRequestException(SR.Format(SR.net_http_invalid_response_header_line, Encoding.ASCII.GetString(line)));
}
}

if (pos == 0)
{
// Invalid empty header name.
ThrowInvalidHttpResponse();
throw new HttpRequestException(SR.Format(SR.net_http_invalid_response_header_name, ""));
}

if (!HeaderDescriptor.TryGet(line.Slice(0, pos), out HeaderDescriptor descriptor))
{
// Invalid header name.
ThrowInvalidHttpResponse();
throw new HttpRequestException(SR.Format(SR.net_http_invalid_response_header_name, Encoding.ASCII.GetString(line.Slice(0, pos))));
}

if (isFromTrailer && descriptor.KnownHeader != null && s_disallowedTrailers.Contains(descriptor.KnownHeader))
Expand All @@ -936,14 +936,14 @@ private static void ParseHeaderNameValue(HttpConnection connection, ReadOnlySpan
if (pos == line.Length)
{
// Invalid header line that doesn't contain ':'.
ThrowInvalidHttpResponse();
throw new HttpRequestException(SR.Format(SR.net_http_invalid_response_header_line, Encoding.ASCII.GetString(line)));
}
}

if (line[pos++] != ':')
{
// Invalid header line that doesn't contain ':'.
ThrowInvalidHttpResponse();
throw new HttpRequestException(SR.Format(SR.net_http_invalid_response_header_line, Encoding.ASCII.GetString(line)));
}

// Skip whitespace after colon
Expand Down Expand Up @@ -1243,7 +1243,7 @@ private bool TryReadNextLine(out ReadOnlySpan<byte> line)
{
if (_allowedReadLineBytes < buffer.Length)
{
ThrowInvalidHttpResponse();
throw new HttpRequestException(SR.Format(SR.net_http_response_headers_exceeded_length, _pool.Settings._maxResponseHeadersLength));
}

line = default;
Expand Down Expand Up @@ -1311,7 +1311,7 @@ private async ValueTask<ArraySegment<byte>> ReadNextResponseHeaderLineAsync(bool
// so if we haven't seen a colon, this is invalid.
if (Array.IndexOf(_readBuffer, (byte)':', _readOffset, lfIndex - _readOffset) == -1)
{
ThrowInvalidHttpResponse();
throw new HttpRequestException(SR.net_http_invalid_response_header_folder);
}

// When we return the line, we need the interim newlines filtered out. According
Expand Down Expand Up @@ -1354,7 +1354,7 @@ private void ThrowIfExceededAllowedReadLineBytes()
{
if (_allowedReadLineBytes < 0)
{
ThrowInvalidHttpResponse();
throw new HttpRequestException(SR.Format(SR.net_http_response_headers_exceeded_length, _pool.Settings._maxResponseHeadersLength));
}
}

Expand Down Expand Up @@ -1398,7 +1398,7 @@ private void Fill()
if (NetEventSource.IsEnabled) Trace($"Received {bytesRead} bytes.");
if (bytesRead == 0)
{
throw new IOException(SR.net_http_invalid_response);
throw new IOException(SR.net_http_invalid_response_premature_eof);
}

_readLength += bytesRead;
Expand Down Expand Up @@ -1445,7 +1445,7 @@ private async Task FillAsync()
if (NetEventSource.IsEnabled) Trace($"Received {bytesRead} bytes.");
if (bytesRead == 0)
{
throw new IOException(SR.net_http_invalid_response);
throw new IOException(SR.net_http_invalid_response_premature_eof);
}

_readLength += bytesRead;
Expand Down Expand Up @@ -1846,10 +1846,6 @@ private static bool EqualsOrdinal(string left, Span<byte> right)

public sealed override string ToString() => $"{nameof(HttpConnection)}({_pool})"; // Description for diagnostic purposes

private static void ThrowInvalidHttpResponse() => throw new HttpRequestException(SR.net_http_invalid_response);

private static void ThrowInvalidHttpResponse(Exception innerException) => throw new HttpRequestException(SR.net_http_invalid_response, innerException);

internal sealed override void Trace(string message, [CallerMemberName] string memberName = null) =>
NetEventSource.Log.HandlerMessage(
_pool?.GetHashCode() ?? 0, // pool ID
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ private ValueTask<HttpConnection> GetOrReserveHttp11ConnectionAsync(Cancellation

if (sslStream.SslProtocol < SslProtocols.Tls12)
{
throw new HttpRequestException(SR.net_http_invalid_response);
throw new HttpRequestException(SR.Format(SR.net_ssl_http2_requires_tls12, sslStream.SslProtocol));
}

http2Connection = new Http2Connection(this, sslStream);
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RawConnectionStream.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ public override void Write(ReadOnlySpan<byte> buffer)
{
if (_connection == null)
{
throw new IOException(SR.net_http_io_write);
throw new IOException(SR.ObjectDisposed_StreamClosed);
}

if (buffer.Length != 0)
Expand All @@ -170,7 +170,7 @@ public override ValueTask WriteAsync(ReadOnlyMemory<byte> buffer, CancellationTo

if (_connection == null)
{
return new ValueTask(Task.FromException(new IOException(SR.net_http_io_write)));
return new ValueTask(Task.FromException(new IOException(SR.ObjectDisposed_StreamClosed)));
}

if (buffer.Length == 0)
Expand Down

0 comments on commit 7058903

Please sign in to comment.