Skip to content

Commit b2629ef

Browse files
committed
Ensure multi-cookie headers are joined with ; instead of ,
Fixes #228
1 parent 2ec1de1 commit b2629ef

File tree

2 files changed

+75
-1
lines changed

2 files changed

+75
-1
lines changed

src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/Authentication/RemoteAppAuthenticationService.cs

+10-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
using Microsoft.AspNetCore.Http.Extensions;
1414
using Microsoft.Extensions.Logging;
1515
using Microsoft.Extensions.Options;
16+
using Microsoft.Net.Http.Headers;
1617

1718
namespace Microsoft.AspNetCore.SystemWebAdapters.Authentication;
1819

@@ -97,7 +98,7 @@ public async Task<RemoteAppAuthenticationResult> AuthenticateAsync(HttpRequest o
9798
}
9899

99100
// Add configured headers to the request, or all headers if none in particular are specified
100-
private static void AddHeaders(IEnumerable<string> headersToForward, HttpRequest originalRequest, HttpRequestMessage authRequest)
101+
internal static void AddHeaders(IEnumerable<string> headersToForward, HttpRequest originalRequest, HttpRequestMessage authRequest)
101102
{
102103
// Add x-forwarded headers so that the authenticate API will know which host the HTTP request was addressed to originally.
103104
// These headers are also used by result processors - to fix-up redirect responses, for example, to redirect back to the
@@ -118,6 +119,14 @@ private static void AddHeaders(IEnumerable<string> headersToForward, HttpRequest
118119

119120
foreach (var headerName in headerNames)
120121
{
122+
// HttpClient wrongly uses comma (",") instead of semi-colon (";") as a separator for Cookie headers.
123+
// To mitigate this, we concatenate them manually and put them back as a single header value.
124+
if (string.Equals(headerName, HeaderNames.Cookie, StringComparison.OrdinalIgnoreCase))
125+
{
126+
authRequest.Headers.Add(headerName, string.Join("; ", originalRequest.Headers[headerName].ToArray()));
127+
continue;
128+
}
129+
121130
authRequest.Headers.Add(headerName, originalRequest.Headers[headerName].ToArray());
122131
}
123132
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Linq;
4+
using System.Net.Http;
5+
using Microsoft.AspNetCore.Http;
6+
using Microsoft.Extensions.Primitives;
7+
using Moq;
8+
using Xunit;
9+
10+
namespace Microsoft.AspNetCore.SystemWebAdapters.Authentication.Tests;
11+
12+
public class RemoteAppAuthenticationServiceTests
13+
{
14+
[Theory]
15+
[MemberData(nameof(GetHeadersForConcatenation))]
16+
public void AddHeadersConcatenation(Dictionary<string, StringValues> originalHeaders, Dictionary<string, StringValues> resultHeaders)
17+
{
18+
var originalRequest = new Mock<HttpRequestCore>();
19+
originalRequest.Setup(r => r.Headers).Returns(new HeaderDictionary(originalHeaders));
20+
originalRequest.Setup(r => r.Scheme).Returns("scheme");
21+
originalRequest.Setup(r => r.Host).Returns(new HostString("host"));
22+
23+
// Add additional headers that are added to the original request's headers
24+
resultHeaders.Add(AuthenticationConstants.ForwardedHostHeaderName, "host");
25+
resultHeaders.Add(AuthenticationConstants.ForwardedProtoHeaderName, "scheme");
26+
resultHeaders.Add(AuthenticationConstants.MigrationAuthenticateRequestHeaderName, "true");
27+
28+
var authRequest = new HttpRequestMessage();
29+
30+
RemoteAppAuthenticationService.AddHeaders(originalHeaders.Keys, originalRequest.Object, authRequest);
31+
32+
Assert.Collection(
33+
authRequest.Headers.OrderBy(h => h.Key).Select(h => KeyValuePair.Create(h.Key, new StringValues(h.Value.ToArray()))),
34+
resultHeaders.OrderBy(h => h.Key).Select<KeyValuePair<string, StringValues>, Action<KeyValuePair<string, StringValues>>>(
35+
expected => (actual =>
36+
{
37+
Assert.Equal(expected.Key, actual.Key);
38+
Assert.Equal(expected.Value, actual.Value);
39+
})).ToArray());
40+
}
41+
42+
public static IEnumerable<object[]> GetHeadersForConcatenation()
43+
{
44+
// Trivial positive case
45+
yield return new object[]
46+
{
47+
new Dictionary<string, StringValues>(){ { "A", new StringValues("1") } },
48+
new Dictionary<string, StringValues>(){ { "A", new StringValues("1") } }
49+
};
50+
51+
// Multi-header case
52+
yield return new object[]
53+
{
54+
new Dictionary<string, StringValues>(){ { "A", new StringValues(new[] { "1", "2" }) } },
55+
new Dictionary<string, StringValues>(){ { "A", new StringValues(new[] { "1", "2" }) } }
56+
};
57+
58+
// Multi-cookie case
59+
yield return new object[]
60+
{
61+
new Dictionary<string, StringValues>(){ { "Cookie", new StringValues(new[] { "1", "2" }) } },
62+
new Dictionary<string, StringValues>(){ { "Cookie", new StringValues(new[] { "1; 2" }) } }
63+
};
64+
}
65+
}

0 commit comments

Comments
 (0)