Second parameter of Substring is wrong in HeaderFindAndReplaceCreator (…

…#1424) * Update HeaderFindAndReplaceCreator.cs Second parameter of Substring was wrong. Existing implementation works only for replacements at the beginning of a string (startOfPlaceholder = 0) * Update HeaderFindAndReplaceCreator.cs Remove and Sort Usings * Update HeaderFindAndReplaceCreator.cs: Replace by char Co-authored-by: Raynald Messié <[email protected]> * Update HeaderFindAndReplaceCreator.cs: Split by char * Review * Fix 'length' value for 'Substring' 2nd param * Add unit test to check replacements in the middle --------- Co-authored-by: Raman Maksimchuk <[email protected]> Co-authored-by: Raynald Messié <[email protected]>
ThreeMammals · Sep 30, 2023 · 5a81cce · 5a81cce
1 parent 5dbbbef
commit 5a81cce
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 10 deletions.
diff --git a/src/Ocelot/Configuration/Creator/HeaderFindAndReplaceCreator.cs b/src/Ocelot/Configuration/Creator/HeaderFindAndReplaceCreator.cs
@@ -1,7 +1,7 @@
 using Ocelot.Configuration.File;
 using Ocelot.Infrastructure;
 using Ocelot.Logging;
-using Ocelot.Responses;
+using Ocelot.Responses;
 
 namespace Ocelot.Configuration.Creator
 {
@@ -69,16 +69,16 @@ public HeaderTransformations Create(FileRoute fileRoute)
 
         private Response<HeaderFindAndReplace> Map(KeyValuePair<string, string> input)
         {
-            var findAndReplace = input.Value.Split(",");
+            var findAndReplace = input.Value.Split(',');
 
             var replace = findAndReplace[1].TrimStart();
 
             var startOfPlaceholder = replace.IndexOf('{', StringComparison.Ordinal);
             if (startOfPlaceholder > -1)
             {
-                var endOfPlaceholder = replace.IndexOf("}", startOfPlaceholder, StringComparison.Ordinal);
+                var endOfPlaceholder = replace.IndexOf('}', startOfPlaceholder);
 
-                var placeholder = replace.Substring(startOfPlaceholder, startOfPlaceholder + (endOfPlaceholder + 1));
+                var placeholder = replace.Substring(startOfPlaceholder, endOfPlaceholder - startOfPlaceholder + 1);
 
                 var value = _placeholders.Get(placeholder);
 

diff --git a/test/Ocelot.UnitTests/Configuration/HeaderFindAndReplaceCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/HeaderFindAndReplaceCreatorTests.cs
@@ -101,7 +101,7 @@ public void should_use_base_url_placeholder()
             };
 
             this.Given(x => GivenTheRoute(route))
-                .And(x => GivenTheBaseUrlIs("http://ocelot.com/"))
+                .And(x => GivenThePlaceholderIs("http://ocelot.com/"))
                 .When(x => WhenICreate())
                 .Then(x => ThenTheFollowingDownstreamIsReturned(downstream))
                 .BDDfy();
@@ -139,7 +139,7 @@ private void ThenTheLoggerIsCalledCorrectly(string message)
             _logger.Verify(x => x.LogWarning(message), Times.Once);
         }
 
-        [Fact]
+        [Fact]
         public void should_use_base_url_partial_placeholder()
         {
             var route = new FileRoute
@@ -156,12 +156,35 @@ public void should_use_base_url_partial_placeholder()
             };
 
             this.Given(x => GivenTheRoute(route))
-                .And(x => GivenTheBaseUrlIs("http://ocelot.com/"))
+                .And(x => GivenThePlaceholderIs("http://ocelot.com/"))
                 .When(x => WhenICreate())
                 .Then(x => ThenTheFollowingDownstreamIsReturned(downstream))
                 .BDDfy();
         }
 
+        [Fact]
+        public void should_map_with_partial_placeholder_in_the_middle()
+        {
+            var route = new FileRoute
+            {
+                DownstreamHeaderTransform = new Dictionary<string, string>
+                {
+                    {"Host-Next", "www.bbc.co.uk, subdomain.{Host}/path"},
+                },
+            };
+
+            var expected = new List<HeaderFindAndReplace>
+            {
+                new("Host-Next", "www.bbc.co.uk", "subdomain.ocelot.next/path", 0),
+            };
+
+            this.Given(x => GivenTheRoute(route))
+                .And(x => GivenThePlaceholderIs("ocelot.next"))
+                .When(x => WhenICreate())
+                .Then(x => ThenTheFollowingDownstreamIsReturned(expected))
+                .BDDfy();
+        }
+
         [Fact]
         public void should_add_trace_id_header()
         {
@@ -176,7 +199,7 @@ public void should_add_trace_id_header()
             var expected = new AddHeader("Trace-Id", "{TraceId}");
 
             this.Given(x => GivenTheRoute(route))
-                .And(x => GivenTheBaseUrlIs("http://ocelot.com/"))
+                .And(x => GivenThePlaceholderIs("http://ocelot.com/"))
                 .When(x => WhenICreate())
                 .Then(x => ThenTheFollowingAddHeaderToDownstreamIsReturned(expected))
                 .BDDfy();
@@ -220,9 +243,9 @@ public void should_add_upstream_header_as_is_when_no_replacement_is_given()
                 .BDDfy();
         }
 
-        private void GivenTheBaseUrlIs(string baseUrl)
+        private void GivenThePlaceholderIs(string placeholderValue)
         {
-            _placeholders.Setup(x => x.Get(It.IsAny<string>())).Returns(new OkResponse<string>(baseUrl));
+            _placeholders.Setup(x => x.Get(It.IsAny<string>())).Returns(new OkResponse<string>(placeholderValue));
         }
 
         private void GivenTheBaseUrlErrors()