Skip to content

Commit

Permalink
Http.Sys: Clean up Request parsing errors (#57531)
Browse files Browse the repository at this point in the history 
* Http.Sys: Clean up Request parsing errors

* nit

* comment
  • Loading branch information
@BrennanConroy
BrennanConroy authored Sep 12, 2024
1 parent 296bdde commit 37dd88b
Show file tree
Hide file tree
Showing 10 changed files with 219 additions and 138 deletions.
1 change: 1 addition & 0 deletions src/Servers/HttpSys/src/LoggerEventIds.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,5 @@ internal static class LoggerEventIds
public const int AcceptSetExpectationMismatch = 51;
public const int AcceptCancelExpectationMismatch = 52;
public const int AcceptObserveExpectationMismatch = 53;
public const int RequestParsingError = 54;
}
237 changes: 122 additions & 115 deletions src/Servers/HttpSys/src/RequestProcessing/Request.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,145 +38,152 @@ internal Request(RequestContext requestContext)
{
// TODO: Verbose log
RequestContext = requestContext;
_contentBoundaryType = BoundaryType.None;

RequestId = requestContext.RequestId;
// For HTTP/2 Http.Sys assigns each request a unique connection id for use with API calls, but the RawConnectionId represents the real connection.
UConnectionId = requestContext.ConnectionId;
RawConnectionId = requestContext.RawConnectionId;
SslStatus = requestContext.SslStatus;
try
{
_contentBoundaryType = BoundaryType.None;

KnownMethod = requestContext.VerbId;
Method = requestContext.GetVerb()!;
RequestId = requestContext.RequestId;
// For HTTP/2 Http.Sys assigns each request a unique connection id for use with API calls, but the RawConnectionId represents the real connection.
UConnectionId = requestContext.ConnectionId;
RawConnectionId = requestContext.RawConnectionId;
SslStatus = requestContext.SslStatus;

RawUrl = requestContext.GetRawUrl()!;
KnownMethod = requestContext.VerbId;
Method = requestContext.GetVerb()!;

var cookedUrl = requestContext.GetCookedUrl();
QueryString = cookedUrl.GetQueryString() ?? string.Empty;
RawUrl = requestContext.GetRawUrl()!;

var rawUrlInBytes = requestContext.GetRawUrlInBytes();
var originalPath = RequestUriBuilder.DecodeAndUnescapePath(rawUrlInBytes);
var cookedUrl = requestContext.GetCookedUrl();
QueryString = cookedUrl.GetQueryString() ?? string.Empty;

PathBase = string.Empty;
Path = originalPath;
var prefix = requestContext.Server.Options.UrlPrefixes.GetPrefix((int)requestContext.UrlContext);
var rawUrlInBytes = requestContext.GetRawUrlInBytes();
var originalPath = RequestUriBuilder.DecodeAndUnescapePath(rawUrlInBytes);

// 'OPTIONS * HTTP/1.1'
if (KnownMethod == HTTP_VERB.HttpVerbOPTIONS && string.Equals(RawUrl, "*", StringComparison.Ordinal))
{
PathBase = string.Empty;
Path = string.Empty;
}
// Prefix may be null if the requested has been transferred to our queue
else if (prefix is not null)
{
var pathBase = prefix.PathWithoutTrailingSlash;
Path = originalPath;
var prefix = requestContext.Server.Options.UrlPrefixes.GetPrefix((int)requestContext.UrlContext);

// url: /base/path, prefix: /base/, base: /base, path: /path
// url: /, prefix: /, base: , path: /
if (originalPath.Equals(pathBase, StringComparison.Ordinal))
{
// Exact match, no need to preserve the casing
PathBase = pathBase;
Path = string.Empty;
}
else if (originalPath.Equals(pathBase, StringComparison.OrdinalIgnoreCase))
// 'OPTIONS * HTTP/1.1'
if (KnownMethod == HTTP_VERB.HttpVerbOPTIONS && string.Equals(RawUrl, "*", StringComparison.Ordinal))
{
// Preserve the user input casing
PathBase = originalPath;
PathBase = string.Empty;
Path = string.Empty;
}
else if (originalPath.StartsWith(prefix.Path, StringComparison.Ordinal))
{
// Exact match, no need to preserve the casing
PathBase = pathBase;
Path = originalPath[pathBase.Length..];
}
else if (originalPath.StartsWith(prefix.Path, StringComparison.OrdinalIgnoreCase))
// Prefix may be null if the requested has been transferred to our queue
else if (prefix is not null)
{
// Preserve the user input casing
PathBase = originalPath[..pathBase.Length];
Path = originalPath[pathBase.Length..];
}
else
{
// Http.Sys path base matching is based on the cooked url which applies some non-standard normalizations that we don't use
// like collapsing duplicate slashes "//", converting '\' to '/', and un-escaping "%2F" to '/'. Find the right split and
// ignore the normalizations.
var originalOffset = 0;
var baseOffset = 0;
while (originalOffset < originalPath.Length && baseOffset < pathBase.Length)
var pathBase = prefix.PathWithoutTrailingSlash;

// url: /base/path, prefix: /base/, base: /base, path: /path
// url: /, prefix: /, base: , path: /
if (originalPath.Equals(pathBase, StringComparison.Ordinal))
{
var baseValue = pathBase[baseOffset];
var offsetValue = originalPath[originalOffset];
if (baseValue == offsetValue
|| char.ToUpperInvariant(baseValue) == char.ToUpperInvariant(offsetValue))
{
// case-insensitive match, continue
originalOffset++;
baseOffset++;
}
else if (baseValue == '/' && offsetValue == '\\')
{
// Http.Sys considers these equivalent
originalOffset++;
baseOffset++;
}
else if (baseValue == '/' && originalPath.AsSpan(originalOffset).StartsWith("%2F", StringComparison.OrdinalIgnoreCase))
{
// Http.Sys un-escapes this
originalOffset += 3;
baseOffset++;
}
else if (baseOffset > 0 && pathBase[baseOffset - 1] == '/'
&& (offsetValue == '/' || offsetValue == '\\'))
{
// Duplicate slash, skip
originalOffset++;
}
else if (baseOffset > 0 && pathBase[baseOffset - 1] == '/'
&& originalPath.AsSpan(originalOffset).StartsWith("%2F", StringComparison.OrdinalIgnoreCase))
{
// Duplicate slash equivalent, skip
originalOffset += 3;
}
else
// Exact match, no need to preserve the casing
PathBase = pathBase;
Path = string.Empty;
}
else if (originalPath.Equals(pathBase, StringComparison.OrdinalIgnoreCase))
{
// Preserve the user input casing
PathBase = originalPath;
Path = string.Empty;
}
else if (originalPath.StartsWith(prefix.Path, StringComparison.Ordinal))
{
// Exact match, no need to preserve the casing
PathBase = pathBase;
Path = originalPath[pathBase.Length..];
}
else if (originalPath.StartsWith(prefix.Path, StringComparison.OrdinalIgnoreCase))
{
// Preserve the user input casing
PathBase = originalPath[..pathBase.Length];
Path = originalPath[pathBase.Length..];
}
else
{
// Http.Sys path base matching is based on the cooked url which applies some non-standard normalizations that we don't use
// like collapsing duplicate slashes "//", converting '\' to '/', and un-escaping "%2F" to '/'. Find the right split and
// ignore the normalizations.
var originalOffset = 0;
var baseOffset = 0;
while (originalOffset < originalPath.Length && baseOffset < pathBase.Length)
{
// Mismatch, fall back
// The failing test case here is "/base/call//../bat//path1//path2", reduced to "/base/call/bat//path1//path2",
// where http.sys collapses "//" before "../", but we do "../" first. We've lost the context that there were dot segments,
// or duplicate slashes, how do we figure out that "call/" can be eliminated?
originalOffset = 0;
break;
var baseValue = pathBase[baseOffset];
var offsetValue = originalPath[originalOffset];
if (baseValue == offsetValue
|| char.ToUpperInvariant(baseValue) == char.ToUpperInvariant(offsetValue))
{
// case-insensitive match, continue
originalOffset++;
baseOffset++;
}
else if (baseValue == '/' && offsetValue == '\\')
{
// Http.Sys considers these equivalent
originalOffset++;
baseOffset++;
}
else if (baseValue == '/' && originalPath.AsSpan(originalOffset).StartsWith("%2F", StringComparison.OrdinalIgnoreCase))
{
// Http.Sys un-escapes this
originalOffset += 3;
baseOffset++;
}
else if (baseOffset > 0 && pathBase[baseOffset - 1] == '/'
&& (offsetValue == '/' || offsetValue == '\\'))
{
// Duplicate slash, skip
originalOffset++;
}
else if (baseOffset > 0 && pathBase[baseOffset - 1] == '/'
&& originalPath.AsSpan(originalOffset).StartsWith("%2F", StringComparison.OrdinalIgnoreCase))
{
// Duplicate slash equivalent, skip
originalOffset += 3;
}
else
{
// Mismatch, fall back
// The failing test case here is "/base/call//../bat//path1//path2", reduced to "/base/call/bat//path1//path2",
// where http.sys collapses "//" before "../", but we do "../" first. We've lost the context that there were dot segments,
// or duplicate slashes, how do we figure out that "call/" can be eliminated?
originalOffset = 0;
break;
}
}
PathBase = originalPath[..originalOffset];
Path = originalPath[originalOffset..];
}
PathBase = originalPath[..originalOffset];
Path = originalPath[originalOffset..];
}
}
else if (requestContext.Server.Options.UrlPrefixes.TryMatchLongestPrefix(IsHttps, cookedUrl.GetHost()!, originalPath, out var pathBase, out var path))
{
PathBase = pathBase;
Path = path;
}
else if (requestContext.Server.Options.UrlPrefixes.TryMatchLongestPrefix(IsHttps, cookedUrl.GetHost()!, originalPath, out var pathBase, out var path))
{
PathBase = pathBase;
Path = path;
}

ProtocolVersion = RequestContext.GetVersion();
ProtocolVersion = RequestContext.GetVersion();

Headers = new RequestHeaders(RequestContext);
Headers = new RequestHeaders(RequestContext);

User = RequestContext.GetUser();
User = RequestContext.GetUser();

SniHostName = string.Empty;
if (IsHttps)
{
GetTlsHandshakeResults();
}
SniHostName = string.Empty;
if (IsHttps)
{
GetTlsHandshakeResults();
}

// GetTlsTokenBindingInfo(); TODO: https://github.com/aspnet/HttpSysServer/issues/231
// GetTlsTokenBindingInfo(); TODO: https://github.com/aspnet/HttpSysServer/issues/231

// Finished directly accessing the HTTP_REQUEST structure.
RequestContext.ReleasePins();
// TODO: Verbose log parameters
}
finally
{
// Finished directly accessing the HTTP_REQUEST structure.
RequestContext.ReleasePins();
// TODO: Verbose log parameters
}

RemoveContentLengthIfTransferEncodingContainsChunked();
}
Expand Down
21 changes: 19 additions & 2 deletions src/Servers/HttpSys/src/RequestProcessing/RequestContext.FeatureCollection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ internal partial class RequestContext :
private bool _bodyCompleted;
private IHeaderDictionary _responseHeaders = default!;
private IHeaderDictionary? _responseTrailers;
private ulong? _requestId;

private Fields _initializedFields;

Expand Down Expand Up @@ -98,11 +99,26 @@ private enum Fields
TraceIdentifier = 0x200,
}

protected internal void InitializeFeatures()
protected internal bool InitializeFeatures()
{
_initialized = true;

Request = new Request(this);
// Get the ID before creating the Request object as the Request ctor releases the native memory
// We want the ID in case request processing fails and we need the ID to cancel the native request
_requestId = RequestId;
try
{
Request = new Request(this);
}
catch (Exception ex)
{
Log.RequestParsingError(Logger, ex);
// Synchronously calls Http.Sys and tells it to send an http response
// No one has written to the response yet (haven't even created the response object below)
Server.SendError(_requestId.Value, StatusCodes.Status400BadRequest, authChallenges: null);
return false;
}

Response = new Response(this);

_features = new FeatureCollection(new StandardFeatureCollection(this));
Expand All @@ -124,6 +140,7 @@ protected internal void InitializeFeatures()

_responseStream = new ResponseStream(Response.Body, OnResponseStart);
_responseHeaders = Response.Headers;
return true;
}

private bool IsNotInitialized(Fields field)
Expand Down
3 changes: 3 additions & 0 deletions src/Servers/HttpSys/src/RequestProcessing/RequestContext.Log.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,8 @@ private static partial class Log

[LoggerMessage(LoggerEventIds.ChannelBindingRetrieved, LogLevel.Debug, "Channel binding retrieved.", EventName = "ChannelBindingRetrieved")]
public static partial void ChannelBindingRetrieved(ILogger logger);

[LoggerMessage(LoggerEventIds.RequestParsingError, LogLevel.Debug, "Failed to parse request.", EventName = "RequestParsingError")]
public static partial void RequestParsingError(ILogger logger, Exception exception);
}
}
17 changes: 13 additions & 4 deletions src/Servers/HttpSys/src/RequestProcessing/RequestContext.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,11 @@ public void Abort()
_disconnectToken = new CancellationToken(canceled: true);
}
ForceCancelRequest();
Request.Dispose();
// Request and/or Response can be null (even though the property doesn't say it can)
// if the constructor throws (can happen for invalid path format)
Request?.Dispose();
// Only Abort, Response.Dispose() tries a graceful flush
Response.Abort();
Response?.Abort();
}

private static void Abort(object? state)
Expand All @@ -192,15 +194,22 @@ internal void ForceCancelRequest()
{
try
{
// Shouldn't be able to get here when this is null, but just in case we'll noop
if (_requestId is null)
{
return;
}

var statusCode = PInvoke.HttpCancelHttpRequest(Server.RequestQueue.Handle,
Request.RequestId, default);
_requestId.Value, default);

// Either the connection has already dropped, or the last write is in progress.
// The requestId becomes invalid as soon as the last Content-Length write starts.
// The only way to cancel now is with CancelIoEx.
if (statusCode == ErrorCodes.ERROR_CONNECTION_INVALID)
{
Response.CancelLastWrite();
// Can be null if processing the request threw and the response object was never created.
Response?.CancelLastWrite();
}
}
catch (ObjectDisposedException)
Expand Down
6 changes: 5 additions & 1 deletion src/Servers/HttpSys/src/RequestProcessing/RequestContextOfT.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,11 @@ public override async Task ExecuteAsync()

try
{
InitializeFeatures();
if (!InitializeFeatures())
{
Abort();
return;
}

if (messagePump.Stopping)
{
Expand Down
Loading

0 comments on commit 37dd88b

Please sign in to comment.