#653 Adding null check for downstreamResponse (#1334)

* Adding a check for null downstreamResponse, plus tests. * Target-typed new expressions * Remove and Sort Usings * Fix test * Remove usings --------- Co-authored-by: raman-m <dotnet044@gmail.com>
ThreeMammals · Sep 30, 2023 · e950cf2 · e950cf2
1 parent 5a81cce
commit e950cf2
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 23 deletions.
diff --git a/src/Ocelot/Responder/Middleware/ResponderMiddleware.cs b/src/Ocelot/Responder/Middleware/ResponderMiddleware.cs
@@ -32,6 +32,7 @@ public async Task Invoke(HttpContext httpContext)
             await _next.Invoke(httpContext);
 
             var errors = httpContext.Items.Errors();
+            var downstreamResponse = httpContext.Items.DownstreamResponse();
 
             // todo check errors is ok
             if (errors.Count > 0)
@@ -40,12 +41,14 @@ public async Task Invoke(HttpContext httpContext)
 
                 SetErrorResponse(httpContext, errors);
             }
+            else if (downstreamResponse == null)
+            {
+                Logger.LogDebug($"Pipeline was terminated early in {MiddlewareName}");
+            }
             else
             {
                 Logger.LogDebug("no pipeline errors, setting and returning completed response");
 
-                var downstreamResponse = httpContext.Items.DownstreamResponse();
-
                 await _responder.SetResponseOnHttpContext(httpContext, downstreamResponse);
             }
         }

diff --git a/test/Ocelot.AcceptanceTests/CustomMiddlewareTests.cs b/test/Ocelot.AcceptanceTests/CustomMiddlewareTests.cs
@@ -28,8 +28,8 @@ public void should_call_pre_query_string_builder_middleware()
                     _counter++;
                     await next.Invoke();
                 },
-            };
-
+            };
+
             var port = RandomPortFinder.GetRandomPort();
 
             var fileConfiguration = new FileConfiguration
@@ -73,8 +73,8 @@ public void should_call_authorization_middleware()
                     _counter++;
                     await next.Invoke();
                 },
-            };
-
+            };
+
             var port = RandomPortFinder.GetRandomPort();
 
             var fileConfiguration = new FileConfiguration
@@ -118,8 +118,8 @@ public void should_call_authentication_middleware()
                     _counter++;
                     await next.Invoke();
                 },
-            };
-
+            };
+
             var port = RandomPortFinder.GetRandomPort();
 
             var fileConfiguration = new FileConfiguration
@@ -163,8 +163,8 @@ public void should_call_pre_error_middleware()
                     _counter++;
                     await next.Invoke();
                 },
-            };
-
+            };
+
             var port = RandomPortFinder.GetRandomPort();
 
             var fileConfiguration = new FileConfiguration
@@ -208,8 +208,8 @@ public void should_call_pre_authorization_middleware()
                     _counter++;
                     await next.Invoke();
                 },
-            };
-
+            };
+
             var port = RandomPortFinder.GetRandomPort();
 
             var fileConfiguration = new FileConfiguration
@@ -253,8 +253,8 @@ public void should_call_pre_http_authentication_middleware()
                     _counter++;
                     await next.Invoke();
                 },
-            };
-
+            };
+
             var port = RandomPortFinder.GetRandomPort();
 
             var fileConfiguration = new FileConfiguration
@@ -286,8 +286,54 @@ public void should_call_pre_http_authentication_middleware()
                 .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.OK))
                 .And(x => x.ThenTheCounterIs(1))
                 .BDDfy();
-        }
+        }
+
+        [Fact]
+        public void should_not_throw_when_pipeline_terminates_early()
+        {
+            var configuration = new OcelotPipelineConfiguration
+            {
+                PreQueryStringBuilderMiddleware = (context, next) =>
+                    Task.Run(() =>
+                    {
+                        _counter++;
+                        return; // do not invoke the rest of the pipeline
+                    }),
+            };
+
+            var port = RandomPortFinder.GetRandomPort();
 
+            var fileConfiguration = new FileConfiguration
+            {
+                Routes = new List<FileRoute>
+                    {
+                        new()
+                        {
+                            DownstreamPathTemplate = "/",
+                            DownstreamHostAndPorts = new List<FileHostAndPort>
+                            {
+                                new()
+                                {
+                                    Host = "localhost",
+                                    Port = port,
+                                },
+                            },
+                            DownstreamScheme = "http",
+                            UpstreamPathTemplate = "/",
+                            UpstreamHttpMethod = new List<string> { "Get" },
+                        },
+                    },
+            };
+
+            this.Given(x => x.GivenThereIsAServiceRunningOn($"http://localhost:{port}", 200, ""))
+                .And(x => _steps.GivenThereIsAConfiguration(fileConfiguration))
+                .And(x => _steps.GivenOcelotIsRunning(configuration))
+                .When(x => _steps.WhenIGetUrlOnTheApiGateway("/"))
+                .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.OK))
+                .And(x => x.ThenTheCounterIs(1))
+                .BDDfy();
+        }
+
         [Fact(Skip = "This is just an example to show how you could hook into Ocelot pipeline with your own middleware. At the moment you must use Response.OnCompleted callback and cannot change the response :( I will see if this can be changed one day!")]
         public void should_fix_issue_237()
         {
@@ -296,14 +342,14 @@ public void should_fix_issue_237()
                 var httpContext = (HttpContext)state;
 
                 if (httpContext.Response.StatusCode > 400)
-                {
+                {
                     Debug.WriteLine("COUNT CALLED");
                     Console.WriteLine("COUNT CALLED");
                 }
 
                 return Task.CompletedTask;
-            };
-
+            };
+
             var port = RandomPortFinder.GetRandomPort();
 
             var fileConfiguration = new FileConfiguration
@@ -367,8 +413,8 @@ public void Dispose()
         public class FakeMiddleware
         {
             private readonly RequestDelegate _next;
-            private readonly Func<object, Task> _callback;
-
+            private readonly Func<object, Task> _callback;
+
             public FakeMiddleware(RequestDelegate next, Func<object, Task> callback)
             {
                 _next = next;
@@ -377,10 +423,10 @@ public FakeMiddleware(RequestDelegate next, Func<object, Task> callback)
 
             public async Task Invoke(HttpContext context)
             {
-                await _next(context);
-
+                await _next(context);
+
                 context.Response.OnCompleted(_callback, context);
             }
         }
     }
-}
+}
diff --git a/test/Ocelot.UnitTests/Responder/ResponderMiddlewareTests.cs b/test/Ocelot.UnitTests/Responder/ResponderMiddlewareTests.cs
@@ -49,6 +49,17 @@ public void should_return_any_errors()
                 .BDDfy();
         }
 
+        [Fact]
+        public void should_not_call_responder_when_null_downstream_response()
+        {
+            this._responder.Reset();
+            this.Given(x => x.GivenTheHttpResponseMessageIs(null))
+                .When(x => x.WhenICallTheMiddleware())
+                .Then(x => x.ThenThereAreNoErrors())
+                .Then(x => x._responder.VerifyNoOtherCalls())
+                .BDDfy();
+        }
+
         private void WhenICallTheMiddleware()
         {
             _middleware.Invoke(_httpContext).GetAwaiter().GetResult();