Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove connection-specific headeres for HTTP/2 and HTTP/3 #659

Merged
merged 3 commits into from
Jan 15, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,12 @@ public override Task TransformResponseTrailersAsync(HttpContext context, HttpRes
private static void CopyResponseHeaders(HttpResponseMessage response, HttpHeaders source, HttpContext context, IHeaderDictionary destination,
IReadOnlyDictionary<string, ResponseHeaderTransform> transforms, ref HashSet<string> transformsRun)
{
var isHttp2OrGreater = ProtocolHelper.IsHttp2OrGreater(context.Request.Protocol);

foreach (var header in source)
{
var headerName = header.Key;
if (RequestUtilities.ResponseHeadersToSkip.Contains(headerName))
if (RequestUtilities.ShouldSkipResponseHeader(headerName, isHttp2OrGreater))
{
continue;
}
Expand Down
23 changes: 19 additions & 4 deletions src/ReverseProxy/Utilities/RequestUtilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,26 @@ 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)
{
// This check can be dropped in .NET 6.0+ as these headers will be removed by Kestrel
MihaZupan marked this conversation as resolved.
Show resolved Hide resolved
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