Skip to content

Move Cookie handling out of HttpClient so we do not cross pollinate requests #1801

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/RestSharp/KnownHeaders.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public static class KnownHeaders {
public const string ContentLocation = "Content-Location";
public const string ContentRange = "Content-Range";
public const string ContentType = "Content-Type";
public const string Cookie = "Cookie";
public const string LastModified = "Last-Modified";
public const string ContentMD5 = "Content-MD5";
public const string Host = "Host";
Expand Down
14 changes: 13 additions & 1 deletion src/RestSharp/Request/RequestHeaders.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@
//

// ReSharper disable InvertIf
namespace RestSharp;

using System.Net;

namespace RestSharp;

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

return this;
}

// Add Cookie header from the cookie container
public RequestHeaders AddCookieHeaders(CookieContainer cookieContainer, Uri uri) {
var cookies = cookieContainer.GetCookieHeader(uri);
if (cookies.Length > 0) {
Parameters.AddParameter(new HeaderParameter(KnownHeaders.Cookie, cookies));
}
return this;
}
}
17 changes: 17 additions & 0 deletions src/RestSharp/Request/RestRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.

using System.Net;
using RestSharp.Authenticators;
using RestSharp.Extensions;
// ReSharper disable UnusedAutoPropertyAccessor.Global

Expand All @@ -29,6 +31,11 @@ public class RestRequest {
/// </summary>
public RestRequest() => Method = Method.Get;

/// <summary>
/// Constructor for a rest request to a relative resource URL and optional method
/// </summary>
/// <param name="resource">Resource to use</param>
/// <param name="method">Method to use (defaults to Method.Get></param>
public RestRequest(string? resource, Method method = Method.Get) : this() {
Resource = resource ?? "";
Method = method;
Expand Down Expand Up @@ -58,6 +65,11 @@ static IEnumerable<KeyValuePair<string, string>> ParseQuery(string query)
);
}

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

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

/// <summary>
/// Optional cookie container to use for the request. If not set, cookies are not passed.
/// </summary>
public CookieContainer? CookieContainer { get; set; }

/// <summary>
/// Container of all the files to be uploaded with the request.
/// </summary>
Expand Down
16 changes: 16 additions & 0 deletions src/RestSharp/Request/RestRequestExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

using System.Net;
using System.Text.RegularExpressions;
using RestSharp.Extensions;
using RestSharp.Serializers;
Expand Down Expand Up @@ -387,6 +388,21 @@ public static RestRequest AddObject<T>(this RestRequest request, T obj, params s
return request;
}

/// <summary>
/// Adds cookie to the <seealso cref="HttpClient"/> cookie container.
/// </summary>
/// <param name="request">RestRequest to add the cookies to</param>
/// <param name="name">Cookie name</param>
/// <param name="value">Cookie value</param>
/// <param name="path">Cookie path</param>
/// <param name="domain">Cookie domain, must not be an empty string</param>
/// <returns></returns>
public static RestRequest AddCookie(this RestRequest request, string name, string value, string path, string domain) {
request.CookieContainer ??= new CookieContainer();
request.CookieContainer.Add(new Cookie(name, value, path, domain));
return request;
}

static void CheckAndThrowsForInvalidHost(string name, string value) {
static bool InvalidHost(string host) => Uri.CheckHostName(PortSplitRegex.Split(host)[0]) == UriHostNameType.Unknown;

Expand Down
2 changes: 1 addition & 1 deletion src/RestSharp/Response/RestResponse.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ internal static async Task<RestResponse> FromHttpResponse(
HttpResponseMessage httpResponse,
RestRequest request,
Encoding encoding,
CookieCollection cookieCollection,
CookieCollection? cookieCollection,
CalculateResponseStatus calculateResponseStatus,
CancellationToken cancellationToken
) {
Expand Down
15 changes: 13 additions & 2 deletions src/RestSharp/RestClient.Async.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

using System.Net;
using RestSharp.Extensions;

namespace RestSharp;
Expand All @@ -32,7 +33,7 @@ public async Task<RestResponse> ExecuteAsync(RestRequest request, CancellationTo
internalResponse.ResponseMessage!,
request,
Options.Encoding,
CookieContainer.GetCookies(internalResponse.Url),
request.CookieContainer!.GetCookies(internalResponse.Url),
CalculateResponseStatus,
cancellationToken
)
Expand Down Expand Up @@ -64,17 +65,27 @@ async Task<InternalResponse> ExecuteInternal(RestRequest request, CancellationTo
var ct = cts.Token;

try {
// Make sure we have a cookie container if not provided in the request
var cookieContainer = request.CookieContainer ??= new CookieContainer();
var headers = new RequestHeaders()
.AddHeaders(request.Parameters)
.AddHeaders(DefaultParameters)
.AddAcceptHeader(AcceptedContentTypes);
.AddAcceptHeader(AcceptedContentTypes)
.AddCookieHeaders(cookieContainer, url);
message.AddHeaders(headers);

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

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

// Parse all the cookies from the response and update the cookie jar with cookies
if (responseMessage.Headers.TryGetValues("Set-Cookie", out var cookiesHeader)) {
foreach (var header in cookiesHeader) {
cookieContainer.SetCookies(url, header);
}
}

if (request.OnAfterRequest != null)
await request.OnAfterRequest(responseMessage).ConfigureAwait(false);

Expand Down
44 changes: 27 additions & 17 deletions src/RestSharp/RestClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ namespace RestSharp;
/// Client to translate RestRequests into Http requests and process response result
/// </summary>
public partial class RestClient : IDisposable {
public CookieContainer CookieContainer { get; }

/// <summary>
/// Content types that will be sent in the Accept header. The list is populated from the known serializers.
/// If you need to send something else by default, set this property to a different value.
Expand All @@ -45,13 +43,12 @@ public partial class RestClient : IDisposable {

HttpClient HttpClient { get; }

public RestClientOptions Options { get; }
internal RestClientOptions Options { get; }

public RestClient(RestClientOptions options, Action<HttpRequestHeaders>? configureDefaultHeaders = null) {
UseDefaultSerializers();

Options = options;
CookieContainer = Options.CookieContainer ?? new CookieContainer();
_disposeHttpClient = true;

var handler = new HttpClientHandler();
Expand All @@ -69,25 +66,27 @@ public RestClient(RestClientOptions options, Action<HttpRequestHeaders>? configu
/// </summary>
public RestClient() : this(new RestClientOptions()) { }

/// <inheritdoc />
/// <summary>
/// Sets the BaseUrl property for requests made by this client instance
/// Creates an instance of RestClient using a specific BaseUrl for requests made by this client instance
/// </summary>
/// <param name="baseUrl"></param>
/// <param name="baseUrl">Base URI for the new client</param>
public RestClient(Uri baseUrl) : this(new RestClientOptions { BaseUrl = baseUrl }) { }

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

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

HttpClient = httpClient;
CookieContainer = new CookieContainer();
Options = new RestClientOptions();
_disposeHttpClient = disposeHttpClient;

Expand All @@ -96,15 +95,16 @@ public RestClient(HttpClient httpClient, bool disposeHttpClient = false) {
}
}

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

UseDefaultSerializers();

HttpClient = httpClient;
CookieContainer = new CookieContainer();
Options = options;
_disposeHttpClient = disposeHttpClient;

Expand All @@ -123,6 +123,16 @@ public RestClient(HttpClient httpClient, RestClientOptions options, bool dispose
/// <param name="disposeHandler">Dispose the handler when disposing RestClient, true by default</param>
public RestClient(HttpMessageHandler handler, bool disposeHandler = true) : this(new HttpClient(handler, disposeHandler), true) { }

/// <summary>
/// Returns the currently configured BaseUrl for this RestClient instance
/// </summary>
public Uri? BaseUrl => Options.BaseUrl;

/// <summary>
/// Returns the currently configured BaseHost for this RestClient instance
/// </summary>
public string? BaseHost => Options.BaseHost;

void ConfigureHttpClient(HttpClient httpClient) {
if (Options.MaxTimeout > 0) httpClient.Timeout = TimeSpan.FromMilliseconds(Options.MaxTimeout);
if (httpClient.DefaultRequestHeaders.UserAgent.All(x => x.Product.Name != "RestSharp")) {
Expand All @@ -133,9 +143,9 @@ void ConfigureHttpClient(HttpClient httpClient) {
}

void ConfigureHttpMessageHandler(HttpClientHandler handler) {
handler.UseCookies = false;
handler.Credentials = Options.Credentials;
handler.UseDefaultCredentials = Options.UseDefaultCredentials;
handler.CookieContainer = CookieContainer;
handler.AutomaticDecompression = Options.AutomaticDecompression;
handler.PreAuthenticate = Options.PreAuthenticate;
handler.AllowAutoRedirect = Options.FollowRedirects;
Expand Down
18 changes: 0 additions & 18 deletions src/RestSharp/RestClientExtensions.Config.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
// limitations under the License.
//

using System.Net;
using System.Text;
using RestSharp.Authenticators;
using RestSharp.Extensions;
Expand Down Expand Up @@ -43,23 +42,6 @@ public static partial class RestClientExtensions {
public static RestClient UseQueryEncoder(this RestClient client, Func<string, Encoding, string> queryEncoder)
=> client.With(x => x.EncodeQuery = queryEncoder);

/// <summary>
/// Adds cookie to the <seealso cref="HttpClient"/> cookie container.
/// </summary>
/// <param name="client"></param>
/// <param name="name">Cookie name</param>
/// <param name="value">Cookie value</param>
/// <param name="path">Cookie path</param>
/// <param name="domain">Cookie domain, must not be an empty string</param>
/// <returns></returns>
public static RestClient AddCookie(this RestClient client, string name, string value, string path, string domain) {
lock (client.CookieContainer) {
client.CookieContainer.Add(new Cookie(name, value, path, domain));
}

return client;
}

public static RestClient UseAuthenticator(this RestClient client, IAuthenticator authenticator)
=> client.With(x => x.Authenticator = authenticator);
}
1 change: 0 additions & 1 deletion src/RestSharp/RestClientOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ public RestClientOptions(string baseUrl) : this(new Uri(Ensure.NotEmptyString(ba
public CacheControlHeaderValue? CachePolicy { get; set; }
public bool FollowRedirects { get; set; } = true;
public bool? Expect100Continue { get; set; } = null;
public CookieContainer? CookieContainer { get; set; }
public string UserAgent { get; set; } = DefaultUserAgent;

/// <summary>
Expand Down
57 changes: 57 additions & 0 deletions test/RestSharp.Tests.Integrated/RequestTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,63 @@ public async Task Can_Perform_GET_Async() {
response.Content.Should().Be(val);
}

[Fact]
public async Task Can_Perform_GET_Async_With_Request_Cookies() {
var request = new RestRequest("get-cookies") {
CookieContainer = new CookieContainer()
};
request.CookieContainer.Add(new Cookie("cookie", "value", null, _client.BaseUrl.Host));
request.CookieContainer.Add(new Cookie("cookie2", "value2", null, _client.BaseUrl.Host));
var response = await _client.ExecuteAsync(request);
response.Content.Should().Be("[\"cookie=value\",\"cookie2=value2\"]");
}

[Fact]
public async Task Can_Perform_GET_Async_With_Response_Cookies() {
var request = new RestRequest("set-cookies");
var response = await _client.ExecuteAsync(request);
response.Content.Should().Be("success");

// Check we got all our cookies
var domain = _client.BaseUrl.Host;
var cookie = response.Cookies!.First(p => p.Name == "cookie1");
Assert.Equal("value1", cookie.Value);
Assert.Equal("/", cookie.Path);
Assert.Equal(domain, cookie.Domain);
Assert.Equal(DateTime.MinValue, cookie.Expires);
Assert.False(cookie.HttpOnly);

// Cookie 2 should vanish as the path will not match
cookie = response.Cookies!.FirstOrDefault(p => p.Name == "cookie2");
Assert.Null(cookie);

// Check cookie3 has a valid expiration
cookie = response.Cookies!.First(p => p.Name == "cookie3");
Assert.Equal("value3", cookie.Value);
Assert.Equal("/", cookie.Path);
Assert.Equal(domain, cookie.Domain);
Assert.True(cookie.Expires > DateTime.Now);

// Check cookie4 has a valid expiration
cookie = response.Cookies!.First(p => p.Name == "cookie4");
Assert.Equal("value4", cookie.Value);
Assert.Equal("/", cookie.Path);
Assert.Equal(domain, cookie.Domain);
Assert.True(cookie.Expires > DateTime.Now);

// Cookie 5 should vanish as the request is not SSL
cookie = response.Cookies!.FirstOrDefault(p => p.Name == "cookie5");
Assert.Null(cookie);

// Check cookie6 should be http only
cookie = response.Cookies!.First(p => p.Name == "cookie6");
Assert.Equal("value6", cookie.Value);
Assert.Equal("/", cookie.Path);
Assert.Equal(domain, cookie.Domain);
Assert.Equal(DateTime.MinValue, cookie.Expires);
Assert.True(cookie.HttpOnly);
}

[Fact]
public async Task Can_Timeout_GET_Async() {
var request = new RestRequest("timeout").AddBody("Body_Content");
Expand Down
Loading