Skip to content

Commit

Permalink
Streamline SocketHttpHandler's ParseStatusLine validation (dotnet#27163)
Browse files Browse the repository at this point in the history 
* Streamline SocketHttpHandler's ParseStatusLine validation

For a typical status line like "HTTP/1.1 200 OK", cuts the time of ParseStatusLine almost in half.

* Address PR feedback
  • Loading branch information
@stephentoub @A-And
stephentoub authored and A-And committed Feb 20, 2018
1 parent 7520609 commit fe2ae36
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 37 deletions.
6 changes: 6 additions & 0 deletions src/System.Net.Http/src/System/Net/Http/HttpResponseMessage.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ public Version Version
}
}

internal void SetVersionWithoutValidation(Version value) => _version = value;

public HttpContent Content
{
get { return _content; }
Expand Down Expand Up @@ -74,6 +76,8 @@ public HttpStatusCode StatusCode
}
}

internal void SetStatusCodeWithoutValidation(HttpStatusCode value) => _statusCode = value;

public string ReasonPhrase
{
get
Expand All @@ -97,6 +101,8 @@ public string ReasonPhrase
}
}

internal void SetReasonPhraseWithoutValidation(string value) => _reasonPhrase = value;

public HttpResponseHeaders Headers
{
get
Expand Down
86 changes: 53 additions & 33 deletions src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ internal partial class HttpConnection : IDisposable
private static readonly byte[] s_spaceHttp11NewlineAsciiBytes = Encoding.ASCII.GetBytes(" HTTP/1.1\r\n");
private static readonly byte[] s_hostKeyAndSeparator = Encoding.ASCII.GetBytes(HttpKnownHeaderNames.Host + ": ");
private static readonly byte[] s_httpSchemeAndDelimiter = Encoding.ASCII.GetBytes(Uri.UriSchemeHttp + Uri.SchemeDelimiter);
private static readonly byte[] s_http1DotBytes = Encoding.ASCII.GetBytes("HTTP/1.");
private static readonly ulong s_http10Bytes = BitConverter.ToUInt64(Encoding.ASCII.GetBytes("HTTP/1.0"));
private static readonly ulong s_http11Bytes = BitConverter.ToUInt64(Encoding.ASCII.GetBytes("HTTP/1.1"));
private static readonly string s_cancellationMessage = new OperationCanceledException().Message; // use same message as the default ctor

private readonly HttpConnectionPool _pool;
Expand Down Expand Up @@ -635,73 +638,90 @@ private async Task SendRequestContentWithExpect100ContinueAsync(
// TODO: Remove this overload once https://github.com/dotnet/roslyn/issues/17287 is addressed
// and the compiler doesn't lift the span temporary from the call site into the async state
// machine in debug builds.
private void ParseStatusLine(ArraySegment<byte> line, HttpResponseMessage response) =>
private static void ParseStatusLine(ArraySegment<byte> line, HttpResponseMessage response) =>
ParseStatusLine((Span<byte>)line, response);

private void ParseStatusLine(Span<byte> line, HttpResponseMessage response)
private static void ParseStatusLine(Span<byte> line, HttpResponseMessage response)
{
// We sent the request version as either 1.0 or 1.1.
// We expect a response version of the form 1.X, where X is a single digit as per RFC.

const int MinStatusLineLength = 12; // "HTTP/1.1 123"

if (line.Length < MinStatusLineLength ||
line[0] != 'H' ||
line[1] != 'T' ||
line[2] != 'T' ||
line[3] != 'P' ||
line[4] != '/' ||
line[5] != '1' ||
line[6] != '.' ||
line[8] != ' ')
// Validate the beginning of the status line and set the response version.
const int MinStatusLineLength = 12; // "HTTP/1.x 123"
if (line.Length < MinStatusLineLength || line[8] != ' ')
{
ThrowInvalidHttpResponse();
}

// Set the response HttpVersion
byte minorVersion = line[7];
response.Version =
minorVersion == '1' ? HttpVersion.Version11 :
minorVersion == '0' ? HttpVersion.Version10 :
!IsDigit(minorVersion) ? throw new HttpRequestException(SR.net_http_invalid_response) :
new Version(1, minorVersion - '0');
ulong first8Bytes = BitConverter.ToUInt64(line);
if (first8Bytes == s_http11Bytes)
{
response.SetVersionWithoutValidation(HttpVersion.Version11);
}
else if (first8Bytes == s_http10Bytes)
{
response.SetVersionWithoutValidation(HttpVersion.Version10);
}
else
{
byte minorVersion = line[7];
if (IsDigit(minorVersion) &&
line.Slice(0, 7).SequenceEqual(s_http1DotBytes))
{
response.SetVersionWithoutValidation(new Version(1, minorVersion - '0'));
}
else
{
ThrowInvalidHttpResponse();
}
}

// Set the status code
byte status1 = line[9], status2 = line[10], status3 = line[11];
if (!IsDigit(status1) || !IsDigit(status2) || !IsDigit(status3))
{
ThrowInvalidHttpResponse();
}

response.StatusCode =
(HttpStatusCode)(100 * (status1 - '0') + 10 * (status2 - '0') + (status3 - '0'));
response.SetStatusCodeWithoutValidation((HttpStatusCode)(100 * (status1 - '0') + 10 * (status2 - '0') + (status3 - '0')));

// Parse (optional) reason phrase
if (line.Length == MinStatusLineLength)
{
response.ReasonPhrase = string.Empty;
response.SetReasonPhraseWithoutValidation(string.Empty);
}
else if (line[MinStatusLineLength] != ' ')
else if (line[MinStatusLineLength] == ' ')
{
ThrowInvalidHttpResponse();
Span<byte> reasonBytes = line.Slice(MinStatusLineLength + 1);
string knownReasonPhrase = HttpStatusDescription.Get(response.StatusCode);
if (knownReasonPhrase != null && EqualsOrdinal(knownReasonPhrase, reasonBytes))
{
response.SetReasonPhraseWithoutValidation(knownReasonPhrase);
}
else
{
try
{
response.ReasonPhrase = HttpRuleParser.DefaultHttpEncoding.GetString(reasonBytes);
}
catch (FormatException error)
{
ThrowInvalidHttpResponse(error);
}
}
}
else
{
Span<byte> reasonBytes = line.Slice(MinStatusLineLength + 1);
string knownReasonPhrase = HttpStatusDescription.Get(response.StatusCode);
response.ReasonPhrase = knownReasonPhrase != null && EqualsOrdinal(knownReasonPhrase, reasonBytes) ?
knownReasonPhrase :
HttpRuleParser.DefaultHttpEncoding.GetString(reasonBytes);
ThrowInvalidHttpResponse();
}
}

// TODO: Remove this overload once https://github.com/dotnet/roslyn/issues/17287 is addressed
// and the compiler doesn't lift the span temporary from the call site into the async state
// machine in debug builds.
private void ParseHeaderNameValue(ArraySegment<byte> line, HttpResponseMessage response) =>
private static void ParseHeaderNameValue(ArraySegment<byte> line, HttpResponseMessage response) =>
ParseHeaderNameValue((Span<byte>)line, response);

private void ParseHeaderNameValue(Span<byte> line, HttpResponseMessage response)
private static void ParseHeaderNameValue(Span<byte> line, HttpResponseMessage response)
{
Debug.Assert(line.Length > 0);

Expand Down
7 changes: 3 additions & 4 deletions src/System.Net.Http/tests/FunctionalTests/HttpProtocolTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -403,13 +403,14 @@ public static IEnumerable<string> GetInvalidStatusLine()
yield return "HTTP/A.1 200 OK";
yield return "HTTP/X.Y.Z 200 OK";

// Only pass on .NET Core Windows & ManagedHandler.
// Only pass on .NET Core Windows & SocketsHttpHandler.
if (PlatformDetection.IsNetCore && PlatformDetection.IsWindows)
{
yield return "HTTP/0.1 200 OK";
yield return "HTTP/3.5 200 OK";
yield return "HTTP/1.12 200 OK";
yield return "HTTP/12.1 200 OK";
yield return "HTTP/1.1 200 O\rK";
}

// Skip these test cases on CurlHandler since the behavior is different.
Expand Down Expand Up @@ -442,8 +443,6 @@ public static IEnumerable<string> GetInvalidStatusLine()
yield return "ABCD/1.1 200 OK";
yield return "HTTP/1.1";
yield return "HTTP\\1.1 200 OK";
// ActiveIssue: #26946
// yield return "HTTP/1.1 200 O\rK";
yield return "NOTHTTP/1.1 200 OK";
}
}
Expand Down Expand Up @@ -471,7 +470,7 @@ public async Task GetAsync_ReasonPhraseHasLF_BehaviorDifference()
}
else
{
// UAP and ManagedHandler will allow LF ending.
// UAP and SocketsHttpHandler will allow LF ending.
await GetAsyncSuccessHelper(responseString, expectedStatusCode, expectedReason);
}
}
Expand Down

0 comments on commit fe2ae36

Please sign in to comment.