Skip to content

Commit

Permalink
Remove connection-specific headeres for HTTP/2 and HTTP/3 (#659)
Browse files Browse the repository at this point in the history
* Drop invalid response headers

* Remove comment about .NET 6+
  • Loading branch information
MihaZupan authored Jan 15, 2021
1 parent b7e79d5 commit 5b48e37
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 9 deletions.
13 changes: 8 additions & 5 deletions src/ReverseProxy/Service/Proxy/HttpTransformer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,10 @@ public virtual Task TransformRequestAsync(HttpContext httpContext, HttpRequestMe
public virtual Task TransformResponseAsync(HttpContext httpContext, HttpResponseMessage proxyResponse)
{
var responseHeaders = httpContext.Response.Headers;
CopyResponseHeaders(proxyResponse.Headers, responseHeaders);
CopyResponseHeaders(httpContext, proxyResponse.Headers, responseHeaders);
if (proxyResponse.Content != null)
{
CopyResponseHeaders(proxyResponse.Content.Headers, responseHeaders);
CopyResponseHeaders(httpContext, proxyResponse.Content.Headers, responseHeaders);
}

return Task.CompletedTask;
Expand All @@ -93,22 +93,25 @@ public virtual Task TransformResponseTrailersAsync(HttpContext httpContext, Http
{
// Note that trailers, if any, should already have been declared in Proxy's response
// by virtue of us having proxied all response headers in step 6.
CopyResponseHeaders(proxyResponse.TrailingHeaders, outgoingTrailers);
CopyResponseHeaders(httpContext, proxyResponse.TrailingHeaders, outgoingTrailers);
}

return Task.CompletedTask;
}


private static void CopyResponseHeaders(HttpHeaders source, IHeaderDictionary destination)
private static void CopyResponseHeaders(HttpContext httpContext, HttpHeaders source, IHeaderDictionary destination)
{
var isHttp2OrGreater = ProtocolHelper.IsHttp2OrGreater(httpContext.Request.Protocol);

foreach (var header in source)
{
var headerName = header.Key;
if (RequestUtilities.ResponseHeadersToSkip.Contains(headerName))
if (RequestUtilities.ShouldSkipResponseHeader(headerName, isHttp2OrGreater))
{
continue;
}

destination.Append(headerName, header.Value.ToArray());
}
}
Expand Down
22 changes: 18 additions & 4 deletions src/ReverseProxy/Utilities/RequestUtilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,25 @@ namespace Microsoft.ReverseProxy.Utilities
{
internal static class RequestUtilities
{
// TODO: this list only contains "Transfer-Encoding" because that messes up Kestrel. If we don't need to add any more here then it would be more efficient to
// check for the single value directly. What about connection headers?
internal static readonly HashSet<string> ResponseHeadersToSkip = new HashSet<string>(StringComparer.OrdinalIgnoreCase)
internal static bool ShouldSkipResponseHeader(string headerName, bool isHttp2OrGreater)
{
HeaderNames.TransferEncoding
if (isHttp2OrGreater)
{
return _invalidH2H3ResponseHeaders.Contains(headerName);
}
else
{
return headerName.Equals(HeaderNames.TransferEncoding, StringComparison.OrdinalIgnoreCase);
}
}

private static readonly HashSet<string> _invalidH2H3ResponseHeaders = new(StringComparer.OrdinalIgnoreCase)
{
HeaderNames.Connection,
HeaderNames.TransferEncoding,
HeaderNames.KeepAlive,
HeaderNames.Upgrade,
"Proxy-Connection"
};

/// <summary>
Expand Down
41 changes: 41 additions & 0 deletions test/ReverseProxy.Tests/Service/Proxy/HttpProxyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1538,6 +1538,47 @@ await Assert.ThrowsAsync<ArgumentException>(() => proxy.ProxyAsync(httpContext,
destinationPrefix, httpClient, requestOptions, transforms));
}

[Theory]
[InlineData("Foo", "HTTP/1.1", false)]
[InlineData("Foo", "HTTP/2", false)]
[InlineData("Transfer-Encoding", "HTTP/1.1", true)]
[InlineData("Transfer-Encoding", "HTTP/2", true)]
[InlineData("Connection", "HTTP/1.1", false)]
[InlineData("Connection", "HTTP/2", true)]
[InlineData("Keep-Alive", "HTTP/1.1", false)]
[InlineData("Keep-Alive", "HTTP/2", true)]
[InlineData("Upgrade", "HTTP/1.1", false)]
[InlineData("Upgrade", "HTTP/2", true)]
[InlineData("Proxy-Connection", "HTTP/1.1", false)]
[InlineData("Proxy-Connection", "HTTP/2", true)]
public async Task ProxyAsync_DropsInvalidResponseHeaders(string responseHeaderName, string requestProtocol, bool shouldDrop)
{
var events = TestEventListener.Collect();

var httpContext = new DefaultHttpContext();
httpContext.Request.Method = "GET";
httpContext.Request.Protocol = requestProtocol;

var destinationPrefix = "https://localhost:123/a/b/";
var sut = CreateProxy();
var client = MockHttpHandler.CreateClient(
async (HttpRequestMessage request, CancellationToken cancellationToken) =>
{
await Task.Yield();
var response = new HttpResponseMessage(HttpStatusCode.OK);
response.Content = new StringContent("Foo");
response.Headers.Add(responseHeaderName, "Bar");
return response;
});

await sut.ProxyAsync(httpContext, destinationPrefix, client);

Assert.Equal(!shouldDrop, httpContext.Response.Headers.ContainsKey(responseHeaderName));

AssertProxyStartStop(events, destinationPrefix, httpContext.Response.StatusCode);
events.AssertContainProxyStages(hasRequestContent: false);
}

private static void AssertProxyStartStop(List<EventWrittenEventArgs> events, string destinationPrefix, int statusCode)
{
AssertProxyStartFailedStop(events, destinationPrefix, statusCode, error: null);
Expand Down

0 comments on commit 5b48e37

Please sign in to comment.