Skip to content

Commit 37e7d75

Browse files
committed
Move handling of Cookies out of HttpClient and into RestSharp, so they will not cross pollinate requests.
Make the CookieContainer a property on the request, not the client. Add tests for cookie handling.
1 parent 052357b commit 37e7d75

File tree

11 files changed

+165
-37
lines changed

11 files changed

+165
-37
lines changed

src/RestSharp/KnownHeaders.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ public static class KnownHeaders {
3030
public const string ContentLocation = "Content-Location";
3131
public const string ContentRange = "Content-Range";
3232
public const string ContentType = "Content-Type";
33+
public const string Cookie = "Cookie";
3334
public const string LastModified = "Last-Modified";
3435
public const string ContentMD5 = "Content-MD5";
3536
public const string Host = "Host";

src/RestSharp/Request/RequestHeaders.cs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@
1414
//
1515

1616
// ReSharper disable InvertIf
17-
namespace RestSharp;
17+
18+
using System.Net;
19+
20+
namespace RestSharp;
1821

1922
class RequestHeaders {
2023
public ParametersCollection Parameters { get; } = new();
@@ -33,4 +36,13 @@ public RequestHeaders AddAcceptHeader(string[] acceptedContentTypes) {
3336

3437
return this;
3538
}
39+
40+
// Add Cookie header from the cookie container
41+
public RequestHeaders AddCookieHeaders(CookieContainer cookieContainer, Uri uri) {
42+
var cookies = cookieContainer.GetCookieHeader(uri);
43+
if (cookies.Length > 0) {
44+
Parameters.AddParameter(new HeaderParameter(KnownHeaders.Cookie, cookies));
45+
}
46+
return this;
47+
}
3648
}

src/RestSharp/Request/RestRequest.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15+
using System.Net;
1516
using RestSharp.Extensions;
1617
// ReSharper disable UnusedAutoPropertyAccessor.Global
1718

@@ -29,6 +30,11 @@ public class RestRequest {
2930
/// </summary>
3031
public RestRequest() => Method = Method.Get;
3132

33+
/// <summary>
34+
/// Constructor for a rest request to a relative resource URL and optional method
35+
/// </summary>
36+
/// <param name="resource">Resource to use</param>
37+
/// <param name="method">Method to use (defaults to Method.Get></param>
3238
public RestRequest(string? resource, Method method = Method.Get) : this() {
3339
Resource = resource ?? "";
3440
Method = method;
@@ -58,6 +64,11 @@ static IEnumerable<KeyValuePair<string, string>> ParseQuery(string query)
5864
);
5965
}
6066

67+
/// <summary>
68+
/// Constructor for a rest request to a specific resource Uri and optional method
69+
/// </summary>
70+
/// <param name="resource">Resource Uri to use</param>
71+
/// <param name="method">Method to use (defaults to Method.Get></param>
6172
public RestRequest(Uri resource, Method method = Method.Get)
6273
: this(resource.IsAbsoluteUri ? resource.AbsoluteUri : resource.OriginalString, method) { }
6374

@@ -83,6 +94,11 @@ public RestRequest(Uri resource, Method method = Method.Get)
8394
/// </summary>
8495
public ParametersCollection Parameters { get; } = new();
8596

97+
/// <summary>
98+
/// Optional cookie container to use for the request. If not set, cookies are not passed.
99+
/// </summary>
100+
public CookieContainer? CookieContainer { get; set; }
101+
86102
/// <summary>
87103
/// Container of all the files to be uploaded with the request.
88104
/// </summary>

src/RestSharp/Request/RestRequestExtensions.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15+
using System.Net;
1516
using System.Text.RegularExpressions;
1617
using RestSharp.Extensions;
1718
using RestSharp.Serializers;
@@ -445,6 +446,21 @@ public static RestRequest AddObject<T>(this RestRequest request, T obj, params s
445446
return request;
446447
}
447448

449+
/// <summary>
450+
/// Adds cookie to the <seealso cref="HttpClient"/> cookie container.
451+
/// </summary>
452+
/// <param name="request">RestRequest to add the cookies to</param>
453+
/// <param name="name">Cookie name</param>
454+
/// <param name="value">Cookie value</param>
455+
/// <param name="path">Cookie path</param>
456+
/// <param name="domain">Cookie domain, must not be an empty string</param>
457+
/// <returns></returns>
458+
public static RestRequest AddCookie(this RestRequest request, string name, string value, string path, string domain) {
459+
request.CookieContainer ??= new CookieContainer();
460+
request.CookieContainer.Add(new Cookie(name, value, path, domain));
461+
return request;
462+
}
463+
448464
static void CheckAndThrowsForInvalidHost(string name, string value) {
449465
static bool InvalidHost(string host) => Uri.CheckHostName(PortSplitRegex.Split(host)[0]) == UriHostNameType.Unknown;
450466

src/RestSharp/Response/RestResponse.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ internal static async Task<RestResponse> FromHttpResponse(
6464
HttpResponseMessage httpResponse,
6565
RestRequest request,
6666
Encoding encoding,
67-
CookieCollection cookieCollection,
67+
CookieCollection? cookieCollection,
6868
CalculateResponseStatus calculateResponseStatus,
6969
CancellationToken cancellationToken
7070
) {

src/RestSharp/RestClient.Async.cs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15+
using System.Net;
1516
using RestSharp.Extensions;
1617

1718
namespace RestSharp;
@@ -32,7 +33,7 @@ public async Task<RestResponse> ExecuteAsync(RestRequest request, CancellationTo
3233
internalResponse.ResponseMessage!,
3334
request,
3435
Options.Encoding,
35-
CookieContainer.GetCookies(internalResponse.Url),
36+
request.CookieContainer!.GetCookies(internalResponse.Url),
3637
CalculateResponseStatus,
3738
cancellationToken
3839
)
@@ -64,16 +65,26 @@ async Task<InternalResponse> ExecuteInternal(RestRequest request, CancellationTo
6465
var ct = cts.Token;
6566

6667
try {
68+
// Make sure we have a cookie container if not provided in the request
69+
var cookieContainer = request.CookieContainer ??= new CookieContainer();
6770
var headers = new RequestHeaders()
6871
.AddHeaders(request.Parameters)
6972
.AddHeaders(DefaultParameters)
70-
.AddAcceptHeader(AcceptedContentTypes);
73+
.AddAcceptHeader(AcceptedContentTypes)
74+
.AddCookieHeaders(cookieContainer, url);
7175
message.AddHeaders(headers);
7276

7377
if (request.OnBeforeRequest != null) await request.OnBeforeRequest(message).ConfigureAwait(false);
7478

7579
var responseMessage = await HttpClient.SendAsync(message, request.CompletionOption, ct).ConfigureAwait(false);
7680

81+
// Parse all the cookies from the response and update the cookie jar with cookies
82+
if (responseMessage.Headers.TryGetValues("Set-Cookie", out var cookiesHeader)) {
83+
foreach (var header in cookiesHeader) {
84+
cookieContainer.SetCookies(url, header);
85+
}
86+
}
87+
7788
if (request.OnAfterRequest != null) await request.OnAfterRequest(responseMessage).ConfigureAwait(false);
7889

7990
return new InternalResponse(responseMessage, url, null, timeoutCts.Token);

src/RestSharp/RestClient.cs

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@ namespace RestSharp;
2727
/// Client to translate RestRequests into Http requests and process response result
2828
/// </summary>
2929
public partial class RestClient : IDisposable {
30-
public CookieContainer CookieContainer { get; }
31-
3230
/// <summary>
3331
/// Content types that will be sent in the Accept header. The list is populated from the known serializers.
3432
/// If you need to send something else by default, set this property to a different value.
@@ -51,7 +49,6 @@ public RestClient(RestClientOptions options, Action<HttpRequestHeaders>? configu
5149
UseDefaultSerializers();
5250

5351
Options = options;
54-
CookieContainer = Options.CookieContainer ?? new CookieContainer();
5552
_disposeHttpClient = true;
5653

5754
var handler = new HttpClientHandler();
@@ -71,23 +68,27 @@ public RestClient() : this(new RestClientOptions()) { }
7168

7269
/// <inheritdoc />
7370
/// <summary>
74-
/// Sets the BaseUrl property for requests made by this client instance
71+
/// Creates an instance of RestClient using a specific BaseUrl for requests made by this client instance
7572
/// </summary>
76-
/// <param name="baseUrl"></param>
73+
/// <param name="baseUrl">Base URI for the new client</param>
7774
public RestClient(Uri baseUrl) : this(new RestClientOptions { BaseUrl = baseUrl }) { }
7875

7976
/// <inheritdoc />
8077
/// <summary>
81-
/// Sets the BaseUrl property for requests made by this client instance
78+
/// Creates an instance of RestClient using a specific BaseUrl for requests made by this client instance
8279
/// </summary>
83-
/// <param name="baseUrl"></param>
80+
/// <param name="baseUrl">Base URI for this new client as a string</param>
8481
public RestClient(string baseUrl) : this(new Uri(Ensure.NotEmptyString(baseUrl, nameof(baseUrl)))) { }
8582

83+
/// <summary>
84+
/// Creates an instance of RestClient using a shared HttpClient and does not allocate one internally.
85+
/// </summary>
86+
/// <param name="httpClient">HttpClient to use</param>
87+
/// <param name="disposeHttpClient">True to dispose of the client, false to assume the caller does (defaults to false)</param>
8688
public RestClient(HttpClient httpClient, bool disposeHttpClient = false) {
8789
UseDefaultSerializers();
8890

8991
HttpClient = httpClient;
90-
CookieContainer = new CookieContainer();
9192
Options = new RestClientOptions();
9293
_disposeHttpClient = disposeHttpClient;
9394

@@ -96,15 +97,16 @@ public RestClient(HttpClient httpClient, bool disposeHttpClient = false) {
9697
}
9798
}
9899

100+
/// <summary>
101+
/// Creates an instance of RestClient using a shared HttpClient and specific RestClientOptions and does not allocate one internally.
102+
/// </summary>
103+
/// <param name="httpClient">HttpClient to use</param>
104+
/// <param name="options">RestClient options to use</param>
105+
/// <param name="disposeHttpClient">True to dispose of the client, false to assume the caller does (defaults to false)</param>
99106
public RestClient(HttpClient httpClient, RestClientOptions options, bool disposeHttpClient = false) {
100-
if (options.CookieContainer != null) {
101-
throw new ArgumentException("Custom cookie container cannot be added to the HttpClient instance", nameof(options.CookieContainer));
102-
}
103-
104107
UseDefaultSerializers();
105108

106109
HttpClient = httpClient;
107-
CookieContainer = new CookieContainer();
108110
Options = options;
109111
_disposeHttpClient = disposeHttpClient;
110112

@@ -134,9 +136,9 @@ void ConfigureHttpClient(HttpClient httpClient) {
134136
}
135137

136138
void ConfigureHttpMessageHandler(HttpClientHandler handler) {
139+
handler.UseCookies = false;
137140
handler.Credentials = Options.Credentials;
138141
handler.UseDefaultCredentials = Options.UseDefaultCredentials;
139-
handler.CookieContainer = CookieContainer;
140142
handler.AutomaticDecompression = Options.AutomaticDecompression;
141143
handler.PreAuthenticate = Options.PreAuthenticate;
142144
handler.AllowAutoRedirect = Options.FollowRedirects;

src/RestSharp/RestClientExtensions.Config.cs

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
// limitations under the License.
1414
//
1515

16-
using System.Net;
1716
using System.Text;
1817
using RestSharp.Authenticators;
1918
using RestSharp.Extensions;
@@ -43,23 +42,6 @@ public static partial class RestClientExtensions {
4342
public static RestClient UseQueryEncoder(this RestClient client, Func<string, Encoding, string> queryEncoder)
4443
=> client.With(x => x.EncodeQuery = queryEncoder);
4544

46-
/// <summary>
47-
/// Adds cookie to the <seealso cref="HttpClient"/> cookie container.
48-
/// </summary>
49-
/// <param name="client"></param>
50-
/// <param name="name">Cookie name</param>
51-
/// <param name="value">Cookie value</param>
52-
/// <param name="path">Cookie path</param>
53-
/// <param name="domain">Cookie domain, must not be an empty string</param>
54-
/// <returns></returns>
55-
public static RestClient AddCookie(this RestClient client, string name, string value, string path, string domain) {
56-
lock (client.CookieContainer) {
57-
client.CookieContainer.Add(new Cookie(name, value, path, domain));
58-
}
59-
60-
return client;
61-
}
62-
6345
public static RestClient UseAuthenticator(this RestClient client, IAuthenticator authenticator)
6446
=> client.With(x => x.Authenticator = authenticator);
6547
}

src/RestSharp/RestClientOptions.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@ public RestClientOptions(string baseUrl) : this(new Uri(Ensure.NotEmptyString(ba
7474
public CacheControlHeaderValue? CachePolicy { get; set; }
7575
public bool FollowRedirects { get; set; } = true;
7676
public bool? Expect100Continue { get; set; } = null;
77-
public CookieContainer? CookieContainer { get; set; }
7877
public string? UserAgent { get; set; } = DefaultUserAgent;
7978

8079
/// <summary>

test/RestSharp.Tests.Integrated/RequestTests.cs

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,63 @@ public async Task Can_Perform_GET_Async() {
5151
response.Content.Should().Be(val);
5252
}
5353

54+
[Fact]
55+
public async Task Can_Perform_GET_Async_With_Request_Cookies() {
56+
var request = new RestRequest("get-cookies") {
57+
CookieContainer = new CookieContainer()
58+
};
59+
request.CookieContainer.Add(new Cookie("cookie", "value", null, _client.Options.BaseUrl.Host));
60+
request.CookieContainer.Add(new Cookie("cookie2", "value2", null, _client.Options.BaseUrl.Host));
61+
var response = await _client.ExecuteAsync(request);
62+
response.Content.Should().Be("[\"cookie=value\",\"cookie2=value2\"]");
63+
}
64+
65+
[Fact]
66+
public async Task Can_Perform_GET_Async_With_Response_Cookies() {
67+
var request = new RestRequest("set-cookies");
68+
var response = await _client.ExecuteAsync(request);
69+
response.Content.Should().Be("success");
70+
71+
// Check we got all our cookies
72+
var domain = _client.Options.BaseUrl.Host;
73+
var cookie = response.Cookies!.First(p => p.Name == "cookie1");
74+
Assert.Equal("value1", cookie.Value);
75+
Assert.Equal("/", cookie.Path);
76+
Assert.Equal(domain, cookie.Domain);
77+
Assert.Equal(DateTime.MinValue, cookie.Expires);
78+
Assert.False(cookie.HttpOnly);
79+
80+
// Cookie 2 should vanish as the path will not match
81+
cookie = response.Cookies!.FirstOrDefault(p => p.Name == "cookie2");
82+
Assert.Null(cookie);
83+
84+
// Check cookie3 has a valid expiration
85+
cookie = response.Cookies!.First(p => p.Name == "cookie3");
86+
Assert.Equal("value3", cookie.Value);
87+
Assert.Equal("/", cookie.Path);
88+
Assert.Equal(domain, cookie.Domain);
89+
Assert.True(cookie.Expires > DateTime.Now);
90+
91+
// Check cookie4 has a valid expiration
92+
cookie = response.Cookies!.First(p => p.Name == "cookie4");
93+
Assert.Equal("value4", cookie.Value);
94+
Assert.Equal("/", cookie.Path);
95+
Assert.Equal(domain, cookie.Domain);
96+
Assert.True(cookie.Expires > DateTime.Now);
97+
98+
// Cookie 5 should vanish as the request is not SSL
99+
cookie = response.Cookies!.FirstOrDefault(p => p.Name == "cookie5");
100+
Assert.Null(cookie);
101+
102+
// Check cookie6 should be http only
103+
cookie = response.Cookies!.First(p => p.Name == "cookie6");
104+
Assert.Equal("value6", cookie.Value);
105+
Assert.Equal("/", cookie.Path);
106+
Assert.Equal(domain, cookie.Domain);
107+
Assert.Equal(DateTime.MinValue, cookie.Expires);
108+
Assert.True(cookie.HttpOnly);
109+
}
110+
54111
[Fact]
55112
public async Task Can_Timeout_GET_Async() {
56113
var request = new RestRequest("timeout").AddBody("Body_Content");

0 commit comments

Comments
 (0)