IdentityStream/HttpMessageSigning

Handler does not handle retry with same HttpRequestMessage instance

Closed this issue · 1 comments

When using the standard resilience handler in Microsoft.Extensions.Http.Resilience, the retry logic reuses the same HttpRequestMessage instance for multiple requests. This causes an issue where the signature header is added twice.

var services = new ServiceCollection();

services.AddHttpClient("client name")
  .ConfigurePrimaryHttpMessageHandler(sp => 
  {
    var signatureAlgorithm = SignatureAlgorithm.Create(rsaOrECDsaAlgorithm);

    var config = new HttpMessageSigningConfiguration("key-id", signatureAlgorithm);

    return new SigningHttpMessageHandler(config);
  })
  .AddStandardResilienceHandler();

var serviceProvider => services.BuildServiceProvider();

var client = serviceProvider.GetRequiredService<IHttpClientFactory>().CreateClient("client name");

// Make requests using client. 
// If a retry is triggered the "Signature" header will have one additional value for each retry attempt.

The cause for this is this line:

public void SetHeader(string name, string value) =>
Request.Headers.TryAddWithoutValidation(name, value);

The SetHeader method uses TryAddWithoutValidation(name, value), which adds the new value to the list of header values already present for that header.

It looks like the WcfHttpRequestMessage implementation is not affected, as the WebHeaderCollection.Set(name, value) method used replaces any existing value.

This change would change the behaviour to replace the existing value if there is one:

-            public void SetHeader(string name, string value) =>
-                Request.Headers.TryAddWithoutValidation(name, value);
+            public void SetHeader(string name, string value)
+            {
+                if (Request.Headers.Contains(name))
+                {
+                    Request.Headers.Remove(name);
+                }
+                
+                Request.Headers.TryAddWithoutValidation(name, value);
+            }

I've also asked a question for whether delegating handlers are expected to handle processing the same HttpRequestMessage instance more than once: dotnet/extensions#4965