Fixes for Interceptor tests.

Author: rassilon

src/RestSharp/Options/RestClientOptions.cs

@@ -50,6 +50,12 @@ public RestClientOptions(string baseUrl) : this(new Uri(Ensure.NotEmptyString(ba
     /// <summary>
     /// Custom configuration for the underlying <seealso cref="HttpMessageHandler"/>
     /// </summary>
+    /// <remarks>
+    /// With the addition of all redirection processing being implemented directly by <see cref="RestClient"/>
+    /// please do not alter the <see cref="System.Net.Http.HttpClientHandler.AllowAutoRedirect""/> from its default supplied by RestClient.
+    /// If you set <see cref="System.Net.Http.HttpClientHandler.AllowAutoRedirect""/> to true, then redirection cookie
+    /// processing improvements in RestClient will be skipped since <see cref="System.Net.Http.HttpClient"/> will hide the details from us.
+    /// </remarks>
     public Func<HttpMessageHandler, HttpMessageHandler>? ConfigureMessageHandler { get; set; }
 
     /// <summary>
@@ -139,6 +145,9 @@ public RestClientOptions(string baseUrl) : this(new Uri(Ensure.NotEmptyString(ba
     /// <summary>
     /// Instruct the client to follow redirects. Default is true.
     /// </summary>
+    /// <remarks>
+    /// Note: This now delegates the property implementation to <see cref="RestClientRedirectionOptions"/>.
+    /// </remarks>
     public bool FollowRedirects {
         get {
             return RedirectOptions.FollowRedirects;

src/RestSharp/Options/RestClientRedirectionOptions.cs

@@ -100,7 +100,7 @@ public class RestClientRedirectionOptions {
 
     /// <summary>
     /// HttpStatusCodes that trigger redirect processing. Defaults to MovedPermanently (301),
-    /// SeeOther (303),
+    /// SeeOther/RedirectMethod (303),
     /// TemporaryRedirect (307),
     /// Redirect (302),
     /// PermanentRedirect (308)

src/RestSharp/RestClient.Async.cs

@@ -96,11 +96,10 @@ async Task<HttpResponse> ExecuteRequestAsync(RestRequest request, CancellationTo
         var ct = cts.Token;
 
         HttpResponseMessage? responseMessage = null;
+        // Make sure we have a cookie container if not provided in the request
         var cookieContainer = request.CookieContainer ??= new CookieContainer();
 
         try {
-            // Make sure we have a cookie container if not provided in the request
-
             var headers = new RequestHeaders()
                 .AddHeaders(request.Parameters)
                 .AddHeaders(DefaultParameters)
@@ -109,6 +108,7 @@ async Task<HttpResponse> ExecuteRequestAsync(RestRequest request, CancellationTo
                 .AddCookieHeaders(url, Options.CookieContainer);
 
             bool foundCookies = false;
+            bool firstAttempt = true;
 
             do {
                 using var requestContent = new RequestContent(this, request);
@@ -121,14 +121,19 @@ async Task<HttpResponse> ExecuteRequestAsync(RestRequest request, CancellationTo
                 }
                 using var message = PrepareRequestMessage(httpMethod, url, content, headers);
 
-                if (request.OnBeforeRequest != null) await request.OnBeforeRequest(message).ConfigureAwait(false);
-                await OnBeforeRequest(message).ConfigureAwait(false);
+                if (firstAttempt) {
+                    firstAttempt = false;
+                    try {
+                        if (request.OnBeforeRequest != null) await request.OnBeforeRequest(message).ConfigureAwait(false);
+                        await OnBeforeRequest(message).ConfigureAwait(false);
+                    }
+                    catch (Exception e) {
+                        throw new RestClientInternalException("RestClient.ExecuteRequestAsync OnBeforeRequest threw an exception: ", e);
+                    }
+                }
 
                 responseMessage = await HttpClient.SendAsync(message, request.CompletionOption, ct).ConfigureAwait(false);
 
-                if (request.OnAfterRequest != null) await request.OnAfterRequest(responseMessage).ConfigureAwait(false);
-                await OnAfterRequest(responseMessage).ConfigureAwait(false);
-
                 if (!IsRedirect(Options.RedirectOptions, responseMessage)) {
                     break;
                 }
@@ -187,6 +192,10 @@ async Task<HttpResponse> ExecuteRequestAsync(RestRequest request, CancellationTo
                     httpMethod = HttpMethod.Get;
                     if (!Options.RedirectOptions.ForceForwardBody) {
                         // HttpClient RedirectHandler sets request.Content to null here:
+                        // TODO: I don't think is quite correct yet.. 
+                        // We don't necessarily want to modify the original request, but..
+                        // is there a way to clone it properly and then clear out what we don't
+                        // care about?
                         message.Content = null;
                         // HttpClient Redirect handler also foribly removes
                         // a Transfer-Encoding of chunked in this case.
@@ -218,6 +227,9 @@ async Task<HttpResponse> ExecuteRequestAsync(RestRequest request, CancellationTo
                 Options.CookieContainer?.AddCookies(url, cookiesHeader2);
             }
         }
+        catch (RestClientInternalException e) {
+            throw e.InnerException!;
+        }
         catch (Exception ex) {
             return new HttpResponse(null, url, null, ex, timeoutCts.Token);
         }
@@ -310,7 +322,7 @@ HttpRequestMessage PrepareRequestMessage(HttpMethod httpMethod, Uri url, HttpCon
     }
 
     static bool IsRedirect(RestClientRedirectionOptions options, HttpResponseMessage responseMessage) {
-        return options.RedirectStatusCodes.Contains(responseMessage.StatusCode);
+        return options.FollowRedirects && options.RedirectStatusCodes.Contains(responseMessage.StatusCode);
     }
 
     record HttpResponse(

src/RestSharp/RestClient.cs

@@ -247,7 +247,9 @@ static void ConfigureHttpMessageHandler(HttpClientHandler handler, ReadOnlyRestC
 #if NET
         }
 #endif
-        handler.AllowAutoRedirect = options.FollowRedirects;
+        // ExecuteAsync and RedirectionOptions now own
+        // redirection processing:
+        handler.AllowAutoRedirect = false;
 
 #if NET
         if (!OperatingSystem.IsBrowser() && !OperatingSystem.IsIOS() && !OperatingSystem.IsTvOS()) {

src/RestSharp/RestClientInternalException.cs

@@ -0,0 +1,22 @@
+using System;
+using System.Collections.Generic;
+using System.Runtime.Serialization;
+using System.Text;
+
+namespace RestSharp;
+
+/// <summary>
+/// This exception SHOULD only be used for catching and throwing internal
+/// exceptions within RestSharp.
+/// </summary>
+[Serializable]
+public class RestClientInternalException : Exception {
+    public RestClientInternalException() {
+    }
+
+    public RestClientInternalException(string? message, Exception? innerException) : base(message, innerException) {
+    }
+
+    protected RestClientInternalException(SerializationInfo info, StreamingContext context) : base(info, context) {
+    }
+}

test/RestSharp.Tests.Integrated/RedirectTests.cs

@@ -25,7 +25,7 @@ public class RedirectTests {
 
     public RedirectTests(TestServerFixture fixture) {
         var options = new RestClientOptions(fixture.Server.Url) {
-            FollowRedirects = false
+            FollowRedirects = true
         };
         _client = new RestClient(options);
         _host   = _client.Options.BaseUrl!.Host;
@@ -99,6 +99,30 @@ public async Task Can_Perform_PUT_Async_With_RedirectionResponse_Cookies() {
         response.StatusCode.Should().Be(HttpStatusCode.MethodNotAllowed);
     }
 
+    // Needed tests:
+    //Test: ForwardHeaders = false
+    //Test: ForwardHeaders = true (default) might not need separate test
+    //Test: ForwardAuthorization = true
+    //Test: ForwardAuthorization = false (default) probably need separate test
+    //Test: ForwardCookies = true (might already have test)
+    //Test: ForwardCookies = false
+    //Test: ForwardBody = true (default, might not need test)
+    //Test: ForwardBody = false
+    //Test: ForceForwardBody = false (default, might not need test)
+    //Test: ForwardQuery = true (default, might not need test)
+    //Test: ForwardQuery = false
+    //Test: MaxRedirects
+    //Test: ForwardFragment = true
+    //Test: ForwardFragment = false
+    //Test: AllowRedirectMethodStatusCodeToAlterVerb = true (default, might not need test)
+    //Test: AllowRedirectMethodStatusCodeToAlterVerb = false
+    //Test: Altered Redirect Status Codes list
+    //Test: FollowRedirects = false
+    // Problem: Need secure test server:
+    //Test: FollowRedirectsToInsecure = true
+    //Test: FollowRedirectsToInsecure = false
+
+
     class Response {
         public string? Message { get; set; }
     }

test/RestSharp.Tests/OptionsTests.cs

@@ -2,11 +2,11 @@ namespace RestSharp.Tests;
 
 public class OptionsTests {
     [Fact]
-    public void Ensure_follow_redirect() {
+    public void Ensure_no_httpclient_follow_redirect() {
         var value   = false;
         var options = new RestClientOptions { FollowRedirects = true, ConfigureMessageHandler = Configure};
         var _  = new RestClient(options);
-        value.Should().BeTrue();
+        value.Should().BeFalse();
 
         HttpMessageHandler Configure(HttpMessageHandler handler) {
             value = (handler as HttpClientHandler)!.AllowAutoRedirect;