From 379b399e6b284e4ef264ef55f1221d232a253082 Mon Sep 17 00:00:00 2001 From: Kendall Bennett Date: Mon, 4 Apr 2022 12:32:53 -0400 Subject: [PATCH] Make RestClient.Options internal again and expose the BaseUrl and BaseHost as read only getters on the client. 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. --- src/RestSharp/KnownHeaders.cs | 1 + src/RestSharp/Request/RequestHeaders.cs | 14 ++++- src/RestSharp/Request/RestRequest.cs | 17 ++++++ .../Request/RestRequestExtensions.cs | 16 ++++++ src/RestSharp/Response/RestResponse.cs | 2 +- src/RestSharp/RestClient.Async.cs | 15 ++++- src/RestSharp/RestClient.cs | 44 ++++++++------ src/RestSharp/RestClientExtensions.Config.cs | 18 ------ src/RestSharp/RestClientOptions.cs | 1 - .../RequestTests.cs | 57 +++++++++++++++++++ .../Server/TestServer.cs | 32 +++++++++++ 11 files changed, 177 insertions(+), 40 deletions(-) diff --git a/src/RestSharp/KnownHeaders.cs b/src/RestSharp/KnownHeaders.cs index a7409c424..60692dc03 100644 --- a/src/RestSharp/KnownHeaders.cs +++ b/src/RestSharp/KnownHeaders.cs @@ -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"; diff --git a/src/RestSharp/Request/RequestHeaders.cs b/src/RestSharp/Request/RequestHeaders.cs index 0c3a62471..b8fc04cd4 100644 --- a/src/RestSharp/Request/RequestHeaders.cs +++ b/src/RestSharp/Request/RequestHeaders.cs @@ -14,7 +14,10 @@ // // ReSharper disable InvertIf -namespace RestSharp; + +using System.Net; + +namespace RestSharp; class RequestHeaders { public ParametersCollection Parameters { get; } = new(); @@ -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; + } } \ No newline at end of file diff --git a/src/RestSharp/Request/RestRequest.cs b/src/RestSharp/Request/RestRequest.cs index bf6fbb5c5..101e9c34f 100644 --- a/src/RestSharp/Request/RestRequest.cs +++ b/src/RestSharp/Request/RestRequest.cs @@ -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 @@ -29,6 +31,11 @@ public class RestRequest { /// public RestRequest() => Method = Method.Get; + /// + /// Constructor for a rest request to a relative resource URL and optional method + /// + /// Resource to use + /// Method to use (defaults to Method.Get> public RestRequest(string? resource, Method method = Method.Get) : this() { Resource = resource ?? ""; Method = method; @@ -58,6 +65,11 @@ static IEnumerable> ParseQuery(string query) ); } + /// + /// Constructor for a rest request to a specific resource Uri and optional method + /// + /// Resource Uri to use + /// Method to use (defaults to Method.Get> public RestRequest(Uri resource, Method method = Method.Get) : this(resource.IsAbsoluteUri ? resource.AbsoluteUri : resource.OriginalString, method) { } @@ -83,6 +95,11 @@ public RestRequest(Uri resource, Method method = Method.Get) /// public ParametersCollection Parameters { get; } = new(); + /// + /// Optional cookie container to use for the request. If not set, cookies are not passed. + /// + public CookieContainer? CookieContainer { get; set; } + /// /// Container of all the files to be uploaded with the request. /// diff --git a/src/RestSharp/Request/RestRequestExtensions.cs b/src/RestSharp/Request/RestRequestExtensions.cs index d38816262..d54327722 100644 --- a/src/RestSharp/Request/RestRequestExtensions.cs +++ b/src/RestSharp/Request/RestRequestExtensions.cs @@ -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; @@ -387,6 +388,21 @@ public static RestRequest AddObject(this RestRequest request, T obj, params s return request; } + /// + /// Adds cookie to the cookie container. + /// + /// RestRequest to add the cookies to + /// Cookie name + /// Cookie value + /// Cookie path + /// Cookie domain, must not be an empty string + /// + 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; diff --git a/src/RestSharp/Response/RestResponse.cs b/src/RestSharp/Response/RestResponse.cs index 859db23a7..6a03664c8 100644 --- a/src/RestSharp/Response/RestResponse.cs +++ b/src/RestSharp/Response/RestResponse.cs @@ -64,7 +64,7 @@ internal static async Task FromHttpResponse( HttpResponseMessage httpResponse, RestRequest request, Encoding encoding, - CookieCollection cookieCollection, + CookieCollection? cookieCollection, CalculateResponseStatus calculateResponseStatus, CancellationToken cancellationToken ) { diff --git a/src/RestSharp/RestClient.Async.cs b/src/RestSharp/RestClient.Async.cs index ed7b502fa..da8ae51dc 100644 --- a/src/RestSharp/RestClient.Async.cs +++ b/src/RestSharp/RestClient.Async.cs @@ -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; @@ -32,7 +33,7 @@ public async Task ExecuteAsync(RestRequest request, CancellationTo internalResponse.ResponseMessage!, request, Options.Encoding, - CookieContainer.GetCookies(internalResponse.Url), + request.CookieContainer!.GetCookies(internalResponse.Url), CalculateResponseStatus, cancellationToken ) @@ -64,10 +65,13 @@ async Task 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) @@ -75,6 +79,13 @@ async Task ExecuteInternal(RestRequest request, CancellationTo 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); diff --git a/src/RestSharp/RestClient.cs b/src/RestSharp/RestClient.cs index 4df35a896..842f2299f 100644 --- a/src/RestSharp/RestClient.cs +++ b/src/RestSharp/RestClient.cs @@ -27,8 +27,6 @@ namespace RestSharp; /// Client to translate RestRequests into Http requests and process response result /// public partial class RestClient : IDisposable { - public CookieContainer CookieContainer { get; } - /// /// 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. @@ -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? configureDefaultHeaders = null) { UseDefaultSerializers(); Options = options; - CookieContainer = Options.CookieContainer ?? new CookieContainer(); _disposeHttpClient = true; var handler = new HttpClientHandler(); @@ -69,25 +66,27 @@ public RestClient(RestClientOptions options, Action? configu /// public RestClient() : this(new RestClientOptions()) { } - /// /// - /// 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 /// - /// + /// Base URI for the new client public RestClient(Uri baseUrl) : this(new RestClientOptions { BaseUrl = baseUrl }) { } - /// /// - /// 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 /// - /// + /// Base URI for this new client as a string public RestClient(string baseUrl) : this(new Uri(Ensure.NotEmptyString(baseUrl, nameof(baseUrl)))) { } + /// + /// Creates an instance of RestClient using a shared HttpClient and does not allocate one internally. + /// + /// HttpClient to use + /// True to dispose of the client, false to assume the caller does (defaults to false) public RestClient(HttpClient httpClient, bool disposeHttpClient = false) { UseDefaultSerializers(); HttpClient = httpClient; - CookieContainer = new CookieContainer(); Options = new RestClientOptions(); _disposeHttpClient = disposeHttpClient; @@ -96,15 +95,16 @@ public RestClient(HttpClient httpClient, bool disposeHttpClient = false) { } } + /// + /// Creates an instance of RestClient using a shared HttpClient and specific RestClientOptions and does not allocate one internally. + /// + /// HttpClient to use + /// RestClient options to use + /// True to dispose of the client, false to assume the caller does (defaults to false) 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; @@ -123,6 +123,16 @@ public RestClient(HttpClient httpClient, RestClientOptions options, bool dispose /// Dispose the handler when disposing RestClient, true by default public RestClient(HttpMessageHandler handler, bool disposeHandler = true) : this(new HttpClient(handler, disposeHandler), true) { } + /// + /// Returns the currently configured BaseUrl for this RestClient instance + /// + public Uri? BaseUrl => Options.BaseUrl; + + /// + /// Returns the currently configured BaseHost for this RestClient instance + /// + 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")) { @@ -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; diff --git a/src/RestSharp/RestClientExtensions.Config.cs b/src/RestSharp/RestClientExtensions.Config.cs index 39ee8c05e..524e5151b 100644 --- a/src/RestSharp/RestClientExtensions.Config.cs +++ b/src/RestSharp/RestClientExtensions.Config.cs @@ -13,7 +13,6 @@ // limitations under the License. // -using System.Net; using System.Text; using RestSharp.Authenticators; using RestSharp.Extensions; @@ -43,23 +42,6 @@ public static partial class RestClientExtensions { public static RestClient UseQueryEncoder(this RestClient client, Func queryEncoder) => client.With(x => x.EncodeQuery = queryEncoder); - /// - /// Adds cookie to the cookie container. - /// - /// - /// Cookie name - /// Cookie value - /// Cookie path - /// Cookie domain, must not be an empty string - /// - 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); } \ No newline at end of file diff --git a/src/RestSharp/RestClientOptions.cs b/src/RestSharp/RestClientOptions.cs index 38e4972ab..6837c44e4 100644 --- a/src/RestSharp/RestClientOptions.cs +++ b/src/RestSharp/RestClientOptions.cs @@ -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; /// diff --git a/test/RestSharp.Tests.Integrated/RequestTests.cs b/test/RestSharp.Tests.Integrated/RequestTests.cs index 9eac238a0..e407bd4f5 100644 --- a/test/RestSharp.Tests.Integrated/RequestTests.cs +++ b/test/RestSharp.Tests.Integrated/RequestTests.cs @@ -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"); diff --git a/test/RestSharp.Tests.Integrated/Server/TestServer.cs b/test/RestSharp.Tests.Integrated/Server/TestServer.cs index 12681fa1e..9a5269df8 100644 --- a/test/RestSharp.Tests.Integrated/Server/TestServer.cs +++ b/test/RestSharp.Tests.Integrated/Server/TestServer.cs @@ -37,6 +37,10 @@ public HttpServer(ITestOutputHelper output = null) { _app.MapGet("request-echo", async context => await context.Request.BodyReader.AsStream().CopyToAsync(context.Response.BodyWriter.AsStream())); _app.MapDelete("delete", () => new TestResponse { Message = "Works!" }); + // Cookies + _app.MapGet("get-cookies", HandleCookies); + _app.MapGet("set-cookies", HandleSetCookies); + // PUT _app.MapPut( ContentResource, @@ -59,6 +63,34 @@ IResult HandleHeaders(HttpContext ctx) { return Results.Ok(response); } + IResult HandleCookies(HttpContext ctx) { + var results = new List(); + foreach (var (key, value) in ctx.Request.Cookies) { + results.Add($"{key}={value}"); + } + return Results.Ok(results); + } + + IResult HandleSetCookies(HttpContext ctx) { + ctx.Response.Cookies.Append("cookie1", "value1"); + ctx.Response.Cookies.Append("cookie2", "value2", new CookieOptions { + Path = "/path_extra" + }); + ctx.Response.Cookies.Append("cookie3", "value3", new CookieOptions { + Expires = DateTimeOffset.Now.AddDays(2) + }); + ctx.Response.Cookies.Append("cookie4", "value4", new CookieOptions { + MaxAge = TimeSpan.FromSeconds(100) + }); + ctx.Response.Cookies.Append("cookie5", "value5", new CookieOptions { + Secure = true + }); + ctx.Response.Cookies.Append("cookie6", "value6", new CookieOptions { + HttpOnly = true + }); + return Results.Content("success"); + } + async Task HandleUpload(HttpRequest req) { if (!req.HasFormContentType) { return Results.BadRequest("It's not a form");