Conditionally copy Strict-Transport-Security (#2306)

docs/docfx/articles/header-guidelines.md

@@ -45,6 +45,10 @@ services.AddReverseProxy()
     .ConfigureHttpClient((_, handler) => handler.ActivityHeadersPropagator = null);
 ```
 
+### Strict-Transport-Security
+
+This header instructs clients to always use HTTPS, but there may be a conflict between values provided by the proxy and destination. To avoid confusion, the destination's value is not copied to the response if one was already added to the response by the proxy application.
+
 ## Other Header Guidelines
 
 ### Host
@@ -74,4 +78,3 @@ This response header indicates what server technology was used to generate the r
 ### X-Powered-By
 
 This response header indicates what web framework was used to generate the response (ASP.NET, etc.). ASP.NET Core does not generate this header but IIS can. This header is proxied from the destination by default. Applications that want to remove it can use the [ResponseHeaderRemove](transforms.md#responseheaderremove) transform.
-

src/ReverseProxy/Forwarder/HttpTransformer.cs

@@ -11,6 +11,7 @@
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Http.Features;
+using Microsoft.Extensions.Primitives;
 using Microsoft.Net.Http.Headers;
 using Yarp.ReverseProxy.Transforms.Builder;
 
@@ -258,7 +259,18 @@ private static void CopyResponseHeaders(HttpHeaders source, IHeaderDictionary de
                 continue;
             }
 
-            destination[headerName] = RequestUtilities.Concat(destination[headerName], header.Value);
+            var currentValue = destination[headerName];
+
+            // https://github.com/microsoft/reverse-proxy/issues/2269
+            // The Strict-Transport-Security may be added by the proxy before forwarding. Only copy the header
+            // if it's not already present.
+            if (!StringValues.IsNullOrEmpty(currentValue)
+                && string.Equals(headerName, HeaderNames.StrictTransportSecurity, StringComparison.OrdinalIgnoreCase))
+            {
+                continue;
+            }
+
+            destination[headerName] = RequestUtilities.Concat(currentValue, header.Value);
         }
     }
 }

src/ReverseProxy/Forwarder/RequestUtilities.cs

@@ -68,7 +68,7 @@ internal static bool ShouldSkipResponseHeader(string headerName)
         return _headersToExclude.Contains(headerName);
     }
 
-    private static readonly FrozenSet<string> _headersToExclude = new HashSet<string>(18, StringComparer.OrdinalIgnoreCase)
+    private static readonly FrozenSet<string> _headersToExclude = new HashSet<string>(17, StringComparer.OrdinalIgnoreCase)
     {
         HeaderNames.Connection,
         HeaderNames.TransferEncoding,
@@ -87,7 +87,6 @@ internal static bool ShouldSkipResponseHeader(string headerName)
         HeaderNames.UpgradeInsecureRequests,
         HeaderNames.TE,
         HeaderNames.AltSvc,
-        HeaderNames.StrictTransportSecurity,
     }.ToFrozenSet(StringComparer.OrdinalIgnoreCase);
 
     // Headers marked as HttpHeaderType.Content in

test/ReverseProxy.Tests/Forwarder/HttpTransformerTests.cs

@@ -39,7 +39,6 @@ public class HttpTransformerTests
         HeaderNames.UpgradeInsecureRequests,
         HeaderNames.TE,
         HeaderNames.AltSvc,
-        HeaderNames.StrictTransportSecurity,
     };
 
     [Fact]
@@ -163,6 +162,38 @@ public async Task TransformResponseAsync_RemovesRestrictedHeaders()
         }
     }
 
+    [Theory]
+    [InlineData(true)]
+    [InlineData(false)]
+    public async Task TransformResponseAsync_StrictTransportSecurity_CopiedIfNotPresent(bool alreadyPresent)
+    {
+        var transformer = HttpTransformer.Default;
+        var httpContext = new DefaultHttpContext();
+        var proxyResponse = new HttpResponseMessage()
+        {
+            Content = new ByteArrayContent(Array.Empty<byte>())
+        };
+
+        if (alreadyPresent)
+        {
+            httpContext.Response.Headers.StrictTransportSecurity = "max-age=31536000; includeSubDomains";
+        }
+
+        Assert.True(proxyResponse.Headers.TryAddWithoutValidation(HeaderNames.StrictTransportSecurity, "max-age=31000; preload"));
+
+        await transformer.TransformResponseAsync(httpContext, proxyResponse, CancellationToken.None);
+
+        var result = httpContext.Response.Headers.StrictTransportSecurity;
+        if (alreadyPresent)
+        {
+            Assert.Equal("max-age=31536000; includeSubDomains", result);
+        }
+        else
+        {
+            Assert.Equal("max-age=31000; preload", result);
+        }
+    }
+
     [Fact]
     public async Task TransformResponseAsync_ContentLengthAndTransferEncoding_ContentLengthRemoved()
     {