fix: use space character as the separator for user-agent header inste…

…ad of comma (#1156) * Use space character as the separator for user-agent header instead of comma * Update changelog * chore: revert changes to settings file More because I don't know what they will affect, due to not using Rider myself. * docs(changelog): attribute contributor * refactor: add remark on why user-agent header is treated differently * refactor: minor changes --------- Co-authored-by: FantasticFiasco <mattias@kindb.org>
FantasticFiasco · Sep 17, 2024 · 85b6d17 · 85b6d17
1 parent 8596e6b
commit 85b6d17
Show file tree

Hide file tree

Showing 17 changed files with 56 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,10 @@ This project adheres to [Semantic Versioning](http://semver.org/) and is followi
 
 ## Unreleased
 
+### :syringe: Fixed
+
+- [#1155](https://github.com/FantasticFiasco/aws-signature-version-4/issues/1155) Multi-part user-agent header is joined incorrectly leading to AWS signature failures (contribution by [@mungojam](https://github.com/mungojam))
+
 ## [4.0.5] - 2024-05-01
 
 ### :syringe: Fixed

diff --git a/src/Private/CanonicalRequest.cs b/src/Private/CanonicalRequest.cs
@@ -112,7 +112,10 @@ public static (string, string) Build(
 
             foreach (var header in sortedHeaders)
             {
-                builder.Append($"{header.Key}:{string.Join(HeaderValueSeparator, header.Value)}\n");
+                // The 'user-agent' header should be treated differently, as discovered in
+                // https://github.com/FantasticFiasco/aws-signature-version-4/issues/1155
+                var separator = header.Key.ToLowerInvariant() == "user-agent" ? " " : HeaderValueSeparator;
+                builder.Append($"{header.Key}:{string.Join(separator, header.Value)}\n");
             }
 
             builder.Append('\n');

diff --git a/test/Integration/ApiGateway/AwsSignatureHandlerShould.cs b/test/Integration/ApiGateway/AwsSignatureHandlerShould.cs
@@ -41,6 +41,7 @@ public AwsSignatureHandlerShould(IntegrationTestContext context, TestSuiteContex
         [Theory]
         [InlineData("get-header-key-duplicate")]
         [InlineData("get-header-value-multiline")]
+        [InlineData("get-header-value-multiple-user-agent")]
         [InlineData("get-header-value-order")]
         [InlineData("get-header-value-trim")]
         [InlineData("get-unreserved")]
@@ -86,6 +87,7 @@ public async Task PassTestSuiteGivenUserWithPermissions(params string[] scenario
         [Theory]
         [InlineData("get-header-key-duplicate")]
         [InlineData("get-header-value-multiline")]
+        [InlineData("get-header-value-multiple-user-agent")]
         [InlineData("get-header-value-order")]
         [InlineData("get-header-value-trim")]
         [InlineData("get-unreserved")]

diff --git a/test/Integration/ApiGateway/SendAsyncShould.cs b/test/Integration/ApiGateway/SendAsyncShould.cs
@@ -298,6 +298,7 @@ public async Task SucceedGivenHttpCompletionOptionAndCancellationTokenAndImmutab
         [Theory]
         [InlineData("get-header-key-duplicate")]
         [InlineData("get-header-value-multiline")]
+        [InlineData("get-header-value-multiple-user-agent")]
         [InlineData("get-header-value-order")]
         [InlineData("get-header-value-trim")]
         [InlineData("get-unreserved")]
@@ -347,6 +348,7 @@ public async Task PassTestSuiteGivenUserWithPermissions(params string[] scenario
         [Theory]
         [InlineData("get-header-key-duplicate")]
         [InlineData("get-header-value-multiline")]
+        [InlineData("get-header-value-multiple-user-agent")]
         [InlineData("get-header-value-order")]
         [InlineData("get-header-value-trim")]
         [InlineData("get-unreserved")]

diff --git a/test/Integration/ApiGateway/SendShould.cs b/test/Integration/ApiGateway/SendShould.cs
@@ -298,6 +298,7 @@ public async Task SucceedGivenHttpCompletionOptionAndCancellationTokenAndImmutab
         [Theory]
         [InlineData("get-header-key-duplicate")]
         [InlineData("get-header-value-multiline")]
+        [InlineData("get-header-value-multiple-user-agent")]
         [InlineData("get-header-value-order")]
         [InlineData("get-header-value-trim")]
         [InlineData("get-unreserved")]
@@ -347,6 +348,7 @@ public void PassTestSuiteGivenUserWithPermissions(params string[] scenarioName)
         [Theory]
         [InlineData("get-header-key-duplicate")]
         [InlineData("get-header-value-multiline")]
+        [InlineData("get-header-value-multiple-user-agent")]
         [InlineData("get-header-value-order")]
         [InlineData("get-header-value-trim")]
         [InlineData("get-unreserved")]

diff --git a/test/Integration/S3/AwsSignatureHandlerShould.cs b/test/Integration/S3/AwsSignatureHandlerShould.cs
@@ -23,6 +23,7 @@ public AwsSignatureHandlerShould(IntegrationTestContext context, TestSuiteContex
         [Theory]
         [InlineData("get-header-key-duplicate")]
         [InlineData("get-header-value-multiline")]
+        [InlineData("get-header-value-multiple-user-agent")]
         [InlineData("get-header-value-order")]
         [InlineData("get-header-value-trim")]
         [InlineData("get-unreserved")]
@@ -70,6 +71,7 @@ public async Task PassTestSuiteGivenUserWithPermissions(params string[] scenario
         [Theory]
         [InlineData("get-header-key-duplicate")]
         [InlineData("get-header-value-multiline")]
+        [InlineData("get-header-value-multiple-user-agent")]
         [InlineData("get-header-value-order")]
         [InlineData("get-header-value-trim")]
         [InlineData("get-unreserved")]

diff --git a/test/Integration/S3/SendAsyncShould.cs b/test/Integration/S3/SendAsyncShould.cs
@@ -23,6 +23,7 @@ public SendAsyncShould(IntegrationTestContext context, TestSuiteContext testSuit
         [Theory]
         [InlineData("get-header-key-duplicate")]
         [InlineData("get-header-value-multiline")]
+        [InlineData("get-header-value-multiple-user-agent")]
         [InlineData("get-header-value-order")]
         [InlineData("get-header-value-trim")]
         [InlineData("get-unreserved")]
@@ -74,6 +75,7 @@ public async Task PassTestSuiteGivenUserWithPermissions(params string[] scenario
         [Theory]
         [InlineData("get-header-key-duplicate")]
         [InlineData("get-header-value-multiline")]
+        [InlineData("get-header-value-multiple-user-agent")]
         [InlineData("get-header-value-order")]
         [InlineData("get-header-value-trim")]
         [InlineData("get-unreserved")]

diff --git a/test/Integration/S3/SendShould.cs b/test/Integration/S3/SendShould.cs
@@ -22,6 +22,7 @@ public SendShould(IntegrationTestContext context, TestSuiteContext testSuiteCont
         [Theory]
         [InlineData("get-header-key-duplicate")]
         [InlineData("get-header-value-multiline")]
+        [InlineData("get-header-value-multiple-user-agent")]
         [InlineData("get-header-value-order")]
         [InlineData("get-header-value-trim")]
         [InlineData("get-unreserved")]
@@ -73,6 +74,7 @@ public async Task PassTestSuiteGivenUserWithPermissions(params string[] scenario
         [Theory]
         [InlineData("get-header-key-duplicate")]
         [InlineData("get-header-value-multiline")]
+        [InlineData("get-header-value-multiple-user-agent")]
         [InlineData("get-header-value-order")]
         [InlineData("get-header-value-trim")]
         [InlineData("get-unreserved")]

diff --git a/...est-suite/get-header-value-multiple-user-agent/get-header-value-multiple-user-agent.authz b/...est-suite/get-header-value-multiple-user-agent/get-header-value-multiple-user-agent.authz
@@ -0,0 +1 @@
+AWS4-HMAC-SHA256 Credential=AKIDEXAMPLE/20150830/us-east-1/service/aws4_request, SignedHeaders=host;my-header1;user-agent;x-amz-date, Signature=01fe72f4f4f40689e7868c6d448c29133334c2ad2e8f5f08ae5537155989cee0
diff --git a/...test-suite/get-header-value-multiple-user-agent/get-header-value-multiple-user-agent.creq b/...test-suite/get-header-value-multiple-user-agent/get-header-value-multiple-user-agent.creq
@@ -0,0 +1,10 @@
+GET
+/
+
+host:example.amazonaws.com
+my-header1:value3,value2
+user-agent:value4 value1
+x-amz-date:20150830T123600Z
+
+host;my-header1;user-agent;x-amz-date
+e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
diff --git a/...-test-suite/get-header-value-multiple-user-agent/get-header-value-multiple-user-agent.req b/...-test-suite/get-header-value-multiple-user-agent/get-header-value-multiple-user-agent.req
@@ -0,0 +1,7 @@
+GET / HTTP/1.1
+Host:example.amazonaws.com
+User-Agent:value4
+User-Agent:value1
+My-Header1:value3
+My-Header1:value2
+X-Amz-Date:20150830T123600Z
diff --git a/...test-suite/get-header-value-multiple-user-agent/get-header-value-multiple-user-agent.sreq b/...test-suite/get-header-value-multiple-user-agent/get-header-value-multiple-user-agent.sreq
@@ -0,0 +1,8 @@
+GET / HTTP/1.1
+Host:example.amazonaws.com
+My-Header1:value3
+My-Header1:value2
+User-Agent:value4
+User-Agent:value1
+X-Amz-Date:20150830T123600Z
+Authorization: AWS4-HMAC-SHA256 Credential=AKIDEXAMPLE/20150830/us-east-1/service/aws4_request, SignedHeaders=host;my-header1;x-amz-date, Signature=08c7e5a9acfcfeb3ab6b2185e75ce8b1deb5e634ec47601a50643f830c755c01
diff --git a/...-test-suite/get-header-value-multiple-user-agent/get-header-value-multiple-user-agent.sts b/...-test-suite/get-header-value-multiple-user-agent/get-header-value-multiple-user-agent.sts
@@ -0,0 +1,4 @@
+AWS4-HMAC-SHA256
+20150830T123600Z
+20150830/us-east-1/service/aws4_request
+cdfe07e1b7f30c9fcc39be612b73a3a097454d4adea5b719884be5cb0f46f28e
diff --git a/test/Unit/Private/AuthorizationHeaderShould.cs b/test/Unit/Private/AuthorizationHeaderShould.cs
@@ -18,6 +18,7 @@ public AuthorizationHeaderShould(TestSuiteContext context)
         [Theory]
         [InlineData("get-header-key-duplicate")]
         [InlineData("get-header-value-multiline")]
+        [InlineData("get-header-value-multiple-user-agent")]
         [InlineData("get-header-value-order")]
         [InlineData("get-header-value-trim")]
         [InlineData("get-unreserved")]

diff --git a/test/Unit/Private/CanonicalRequestShould.cs b/test/Unit/Private/CanonicalRequestShould.cs
@@ -23,6 +23,7 @@ public CanonicalRequestShould(TestSuiteContext context)
         [Theory]
         [InlineData("get-header-key-duplicate")]
         [InlineData("get-header-value-multiline")]
+        [InlineData("get-header-value-multiple-user-agent")]
         [InlineData("get-header-value-order")]
         [InlineData("get-header-value-trim")]
         [InlineData("get-unreserved")]

diff --git a/test/Unit/Private/SignerShould.cs b/test/Unit/Private/SignerShould.cs
@@ -30,6 +30,7 @@ public SignerShould(TestSuiteContext context)
         [Theory]
         [InlineData("get-header-key-duplicate")]
         [InlineData("get-header-value-multiline")]
+        [InlineData("get-header-value-multiple-user-agent")]
         [InlineData("get-header-value-order")]
         [InlineData("get-header-value-trim")]
         [InlineData("get-unreserved")]
@@ -85,6 +86,7 @@ public async Task PassTestSuiteAsync(params string[] scenarioName)
         [Theory]
         [InlineData("get-header-key-duplicate")]
         [InlineData("get-header-value-multiline")]
+        [InlineData("get-header-value-multiple-user-agent")]
         [InlineData("get-header-value-order")]
         [InlineData("get-header-value-trim")]
         [InlineData("get-unreserved")]
@@ -204,7 +206,7 @@ public void RespectBaseAddress(string baseAddress, string requestUri, string exp
         #region Not add X-Amz-Content-SHA256 content header
 
         // Only requests to S3 should add the "X-Amz-Content-SHA256" header
-        
+
         [Fact]
         public async Task NotAddXAmzContentHeaderAsync()
         {

diff --git a/test/Unit/Private/StringToSignShould.cs b/test/Unit/Private/StringToSignShould.cs
@@ -18,6 +18,7 @@ public StringToSignShould(TestSuiteContext context)
         [Theory]
         [InlineData("get-header-key-duplicate")]
         [InlineData("get-header-value-multiline")]
+        [InlineData("get-header-value-multiple-user-agent")]
         [InlineData("get-header-value-order")]
         [InlineData("get-header-value-trim")]
         [InlineData("get-unreserved")]