Merge pull request microsoft#2525 from Mahdigln/fix/issue-1294-preserve-baseurl-port

fix: Improve server creation and URL handling logic to maintain port

Author: baywet

src/Microsoft.OpenApi.Readers/V2/OpenApiDocumentDeserializer.cs

@@ -145,14 +145,14 @@ private static void MakeServers(IList<OpenApiServer> servers, ParsingContext con
                 basePath = "/";
             }
 
-            // If nothing is provided, don't create a server
-            if (host == null && basePath == null && schemes == null)
+            // If nothing is provided and there's no defaultUrl, don't create a server
+            if (string.IsNullOrEmpty(host) && string.IsNullOrEmpty(basePath) && (schemes == null || schemes.Count == 0) && defaultUrl == null)
             {
                 return;
             }
 
             //Validate host
-            if (host != null && !IsHostValid(host))
+            if (!string.IsNullOrEmpty(host) && !IsHostValid(host))
             {
                 rootNode.Context.Diagnostic.Errors.Add(new(rootNode.Context.GetLocation(), "Invalid host"));
                 return;
@@ -161,7 +161,7 @@ private static void MakeServers(IList<OpenApiServer> servers, ParsingContext con
             // Fill in missing information based on the defaultUrl
             if (defaultUrl != null)
             {
-                host = host ?? defaultUrl.GetComponents(UriComponents.NormalizedHost, UriFormat.SafeUnescaped);
+                host = host ?? defaultUrl.GetComponents(UriComponents.Host | UriComponents.Port, UriFormat.SafeUnescaped);
                 basePath = basePath ?? defaultUrl.GetComponents(UriComponents.Path, UriFormat.SafeUnescaped);
                 schemes = schemes ?? new List<string> { defaultUrl.GetComponents(UriComponents.Scheme, UriFormat.SafeUnescaped) };
             }
@@ -236,6 +236,13 @@ private static string BuildUrl(string scheme, string host, string basePath)
                 uriBuilder.Port = port.Value;
             }
 
+            // Remove default ports to clean up the URL
+            if (("https".Equals(uriBuilder.Scheme, StringComparison.OrdinalIgnoreCase)  && uriBuilder.Port == 443) ||
+                ("http".Equals(uriBuilder.Scheme, StringComparison.OrdinalIgnoreCase) && uriBuilder.Port == 80))
+            {
+                uriBuilder.Port = -1; // Setting to -1 removes the port from the URL
+            }
+
             return uriBuilder.ToString();
         }
 

test/Microsoft.OpenApi.Readers.Tests/V2Tests/OpenApiServerTests.cs

@@ -323,5 +323,170 @@ public void InvalidHostShouldYieldError()
                     SpecificationVersion = OpenApiSpecVersion.OpenApi2_0
                 });
         }
+
+        [Fact]
+        public void BaseUrlWithPortShouldPreservePort()
+        {
+            var input =
+                """
+                swagger: 2.0
+                info:
+                  title: test
+                  version: 1.0.0
+                paths: {}
+                """;
+            var reader = new OpenApiStringReader(new()
+            {
+                BaseUrl = new("http://demo.testfire.net:8080")
+            });
+
+            var doc = reader.Read(input, out var diagnostic);
+
+            var server = doc.Servers.First();
+            Assert.Single(doc.Servers);
+            Assert.Equal("http://demo.testfire.net:8080", server.Url);
+        }
+
+        [Fact]
+        public void BaseUrlWithPortAndPathShouldPreservePort()
+        {
+            var input =
+                """
+                swagger: 2.0
+                info:
+                  title: test
+                  version: 1.0.0
+                paths: {}
+                """;
+            var reader = new OpenApiStringReader(new()
+            {
+                BaseUrl = new("http://demo.testfire.net:8080/swagger/properties.json")
+            });
+
+            var doc = reader.Read(input, out var diagnostic);
+
+            var server = doc.Servers.First();
+            Assert.Single(doc.Servers);
+            Assert.Equal("http://demo.testfire.net:8080/swagger/properties.json", server.Url);
+        }
+
+        [Fact]
+        public void BaseUrlWithNonStandardPortShouldPreservePort()
+        {
+            var input =
+                """
+                swagger: 2.0
+                info:
+                  title: test
+                  version: 1.0.0
+                paths: {}
+                """;
+            var reader = new OpenApiStringReader(new()
+            {
+                BaseUrl = new("https://api.example.com:9443/v1/openapi.yaml")
+            });
+
+            var doc = reader.Read(input, out var diagnostic);
+
+            var server = doc.Servers.First();
+            Assert.Single(doc.Servers);
+            Assert.Equal("https://api.example.com:9443/v1/openapi.yaml", server.Url);
+        }
+
+        [Fact]
+        public void BaseUrlWithStandardHttpsPortShouldRemovePort()
+        {
+            var input =
+                """
+                swagger: 2.0
+                info:
+                  title: test
+                  version: 1.0.0
+                paths: {}
+                """;
+            var reader = new OpenApiStringReader(new()
+            {
+                BaseUrl = new("https://foo.bar:443/api")
+            });
+
+            var doc = reader.Read(input, out var diagnostic);
+
+            var server = doc.Servers.First();
+            Assert.Single(doc.Servers);
+            Assert.Equal("https://foo.bar/api", server.Url);
+        }
+
+        [Fact]
+        public void BaseUrlWithStandardHttpPortShouldRemovePort()
+        {
+            var input =
+                """
+                swagger: 2.0
+                info:
+                  title: test
+                  version: 1.0.0
+                paths: {}
+                """;
+            var reader = new OpenApiStringReader(new()
+            {
+                BaseUrl = new("http://foo.bar:80/api")
+            });
+
+            var doc = reader.Read(input, out var diagnostic);
+
+            var server = doc.Servers.First();
+            Assert.Single(doc.Servers);
+            Assert.Equal("http://foo.bar/api", server.Url);
+        }
+
+        [Fact]
+        public void HostWithStandardHttpsPortShouldRemovePort()
+        {
+            var input =
+                """
+                swagger: 2.0
+                info:
+                  title: test
+                  version: 1.0.0
+                host: foo.bar:443
+                schemes:
+                  - https
+                paths: {}
+                """;
+            var reader = new OpenApiStringReader(new()
+            {
+            });
+
+            var doc = reader.Read(input, out var diagnostic);
+
+            var server = doc.Servers.First();
+            Assert.Single(doc.Servers);
+            Assert.Equal("https://foo.bar", server.Url);
+        }
+
+        [Fact]
+        public void HostWithStandardHttpPortShouldRemovePort()
+        {
+            var input =
+                """
+                swagger: 2.0
+                info:
+                  title: test
+                  version: 1.0.0
+                host: foo.bar:80
+                schemes:
+                  - http
+                paths: {}
+                """;
+            var reader = new OpenApiStringReader(new()
+            {
+            });
+
+            var doc = reader.Read(input, out var diagnostic);
+
+            var server = doc.Servers.First();
+            Assert.Single(doc.Servers);
+            Assert.Equal("http://foo.bar", server.Url);
+        }
     }
 }