Skip to content

Commit 7980a1a

Browse files
committed
HttpClient should not miss Set-Cookie response headers
Recent changes due to PR dotnet#3199 caused `HttpClient` on Windows to miss `Set-Cookie` response headers when one of the header values was large enough to cause `WinHttpQueryHeaders` to fail with `ERROR_INSUFFICIENT_BUFFER` and advance the index. This commit addresses the regression, by always setting the index back to the index we want to retrieve. Added a new test to validate this change. Fixes #6737.
1 parent 4aefcf4 commit 7980a1a

File tree

2 files changed

+66
-2
lines changed

2 files changed

+66
-2
lines changed

src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseParser.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ public unsafe static bool GetResponseHeader(
132132
Debug.Assert(buffer == null || (buffer != null && buffer.Length > StackLimit));
133133

134134
int bufferLength;
135+
uint originalIndex = index;
135136

136137
if (buffer == null)
137138
{
@@ -166,6 +167,11 @@ public unsafe static bool GetResponseHeader(
166167

167168
if (lastError == Interop.WinHttp.ERROR_INSUFFICIENT_BUFFER)
168169
{
170+
// WinHttpQueryHeaders may advance the index even when it fails due to insufficient buffer,
171+
// so we set the index back to its original value so we can try to retrieve the same
172+
// index again with a larger buffer.
173+
index = originalIndex;
174+
169175
buffer = new char[bufferLength];
170176
return GetResponseHeader(requestHandle, infoLevel, ref buffer, ref index, out headerValue);
171177
}

src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.cs

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,10 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5-
using System;
65
using System.Collections.Generic;
6+
using System.Diagnostics;
77
using System.Linq;
88
using System.IO;
9-
using System.Net.Http.Headers;
109
using System.Net.Test.Common;
1110
using System.Security.Authentication.ExtendedProtection;
1211
using System.Text;
@@ -465,6 +464,65 @@ public async Task GetAsync_RequestHeadersAddCustomHeaders_HeaderAndValueSent(str
465464
}
466465
}
467466

467+
private static KeyValuePair<string, string> GenerateCookie(string name, char repeat, int overallHeaderValueLength)
468+
{
469+
string emptyHeaderValue = $"{name}=; Path=/";
470+
471+
Debug.Assert(overallHeaderValueLength > emptyHeaderValue.Length);
472+
473+
int valueCount = overallHeaderValueLength - emptyHeaderValue.Length;
474+
string value = new string(repeat, valueCount);
475+
476+
return new KeyValuePair<string, string>(name, value);
477+
}
478+
479+
public static object[][] CookieNameValues =
480+
{
481+
new object[] { GenerateCookie(name: "foo", repeat: 'a', overallHeaderValueLength: 254) },
482+
new object[] { GenerateCookie(name: "foo", repeat: 'a', overallHeaderValueLength: 255) },
483+
new object[] { GenerateCookie(name: "foo", repeat: 'a', overallHeaderValueLength: 256) },
484+
new object[] { GenerateCookie(name: "foo", repeat: 'a', overallHeaderValueLength: 257) },
485+
new object[]
486+
{
487+
new KeyValuePair<string, string>(
488+
".AspNetCore.Antiforgery.Xam7_OeLcN4",
489+
"CfDJ8NGNxAt7CbdClq3UJ8_6w_4661wRQZT1aDtUOIUKshbcV4P0NdS8klCL5qGSN-PNBBV7w23G6MYpQ81t0PMmzIN4O04fqhZ0u1YPv66mixtkX3iTi291DgwT3o5kozfQhe08-RAExEmXpoCbueP_QYM")
490+
}
491+
};
492+
493+
[Theory]
494+
[MemberData(nameof(CookieNameValues))]
495+
public async Task GetAsync_ResponseWithSetCookieHeaders_AllCookiesRead(KeyValuePair<string, string> cookie1)
496+
{
497+
var cookie2 = new KeyValuePair<string, string>(".AspNetCore.Session", "RAExEmXpoCbueP_QYM");
498+
var cookie3 = new KeyValuePair<string, string>("name", "value");
499+
500+
string url = string.Format(
501+
"http://httpbin.org/cookies/set?{0}={1}&{2}={3}&{4}={5}",
502+
cookie1.Key,
503+
cookie1.Value,
504+
cookie2.Key,
505+
cookie2.Value,
506+
cookie3.Key,
507+
cookie3.Value);
508+
509+
var handler = new HttpClientHandler()
510+
{
511+
AllowAutoRedirect = false
512+
};
513+
514+
using (var client = new HttpClient(handler))
515+
using (HttpResponseMessage response = await client.GetAsync(url))
516+
{
517+
CookieCollection cookies = handler.CookieContainer.GetCookies(new Uri(url));
518+
519+
Assert.Equal(3, handler.CookieContainer.Count);
520+
Assert.Equal(cookie1.Value, cookies[cookie1.Key].Value);
521+
Assert.Equal(cookie2.Value, cookies[cookie2.Key].Value);
522+
Assert.Equal(cookie3.Value, cookies[cookie3.Key].Value);
523+
}
524+
}
525+
468526
[Fact]
469527
public async Task GetAsync_ResponseHeadersRead_ReadFromEachIterativelyDoesntDeadlock()
470528
{

0 commit comments

Comments
 (0)