From fe5cedaef9009c36ca69133148fd3122d4fc434b Mon Sep 17 00:00:00 2001 From: Mahdi Golestan Date: Fri, 3 Oct 2025 10:49:29 +0330 Subject: [PATCH 1/3] Enhance server creation logic and add unit tests Updated the `MakeServers` method to include a check for `defaultUrl` when creating servers. The method now returns early if all relevant parameters are null, and the host assignment from `defaultUrl` now preserves the port. Added new unit tests in `OpenApiServerTests.cs` to ensure that base URLs with ports are correctly handled. Tests cover scenarios for base URLs with a port, with a port and path, and with a non-standard port. --- .../V2/OpenApiDocumentDeserializer.cs | 6 +- .../V2Tests/OpenApiServerTests.cs | 69 +++++++++++++++++++ 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.OpenApi.Readers/V2/OpenApiDocumentDeserializer.cs b/src/Microsoft.OpenApi.Readers/V2/OpenApiDocumentDeserializer.cs index 07bdcb301..99b316b79 100644 --- a/src/Microsoft.OpenApi.Readers/V2/OpenApiDocumentDeserializer.cs +++ b/src/Microsoft.OpenApi.Readers/V2/OpenApiDocumentDeserializer.cs @@ -145,8 +145,8 @@ private static void MakeServers(IList 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 (host == null && basePath == null && schemes == null && defaultUrl == null) { return; } @@ -161,7 +161,7 @@ private static void MakeServers(IList 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 { defaultUrl.GetComponents(UriComponents.Scheme, UriFormat.SafeUnescaped) }; } diff --git a/test/Microsoft.OpenApi.Readers.Tests/V2Tests/OpenApiServerTests.cs b/test/Microsoft.OpenApi.Readers.Tests/V2Tests/OpenApiServerTests.cs index f254800b9..bf7520c34 100644 --- a/test/Microsoft.OpenApi.Readers.Tests/V2Tests/OpenApiServerTests.cs +++ b/test/Microsoft.OpenApi.Readers.Tests/V2Tests/OpenApiServerTests.cs @@ -323,5 +323,74 @@ 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); + } } } From 664a2f4e1442053cd4bf6b4c818f57a2c865ab2d Mon Sep 17 00:00:00 2001 From: Mahdi Golestan Date: Fri, 3 Oct 2025 17:26:08 +0330 Subject: [PATCH 2/3] Improve server URL handling and add unit tests Refactor `MakeServers` method for better null checks on `host`, `basePath`, `schemes`, and `defaultUrl`. Enhance `BuildUrl` to remove default ports (80 for HTTP, 443 for HTTPS) from generated URLs. Add unit tests in `OpenApiServerTests.cs` to verify correct URL generation when standard ports are used. --- .../V2/OpenApiDocumentDeserializer.cs | 11 ++- .../V2Tests/OpenApiServerTests.cs | 96 +++++++++++++++++++ 2 files changed, 105 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.OpenApi.Readers/V2/OpenApiDocumentDeserializer.cs b/src/Microsoft.OpenApi.Readers/V2/OpenApiDocumentDeserializer.cs index 99b316b79..c23813bbb 100644 --- a/src/Microsoft.OpenApi.Readers/V2/OpenApiDocumentDeserializer.cs +++ b/src/Microsoft.OpenApi.Readers/V2/OpenApiDocumentDeserializer.cs @@ -146,13 +146,13 @@ private static void MakeServers(IList servers, ParsingContext con } // If nothing is provided and there's no defaultUrl, don't create a server - if (host == null && basePath == null && schemes == null && defaultUrl == null) + 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; @@ -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 ((uriBuilder.Scheme == "https" && uriBuilder.Port == 443) || + (uriBuilder.Scheme == "http" && uriBuilder.Port == 80)) + { + uriBuilder.Port = -1; // Setting to -1 removes the port from the URL + } + return uriBuilder.ToString(); } diff --git a/test/Microsoft.OpenApi.Readers.Tests/V2Tests/OpenApiServerTests.cs b/test/Microsoft.OpenApi.Readers.Tests/V2Tests/OpenApiServerTests.cs index bf7520c34..0b8411ac3 100644 --- a/test/Microsoft.OpenApi.Readers.Tests/V2Tests/OpenApiServerTests.cs +++ b/test/Microsoft.OpenApi.Readers.Tests/V2Tests/OpenApiServerTests.cs @@ -392,5 +392,101 @@ public void BaseUrlWithNonStandardPortShouldPreservePort() 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); + } } } From e332b5747b8f39e45c7810b8c1cfdd46e0dda4cf Mon Sep 17 00:00:00 2001 From: Mahdigln <139457032+Mahdigln@users.noreply.github.com> Date: Fri, 3 Oct 2025 23:50:29 +0330 Subject: [PATCH 3/3] Update src/Microsoft.OpenApi.Readers/V2/OpenApiDocumentDeserializer.cs Co-authored-by: Vincent Biret --- .../V2/OpenApiDocumentDeserializer.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.OpenApi.Readers/V2/OpenApiDocumentDeserializer.cs b/src/Microsoft.OpenApi.Readers/V2/OpenApiDocumentDeserializer.cs index c23813bbb..0141d734a 100644 --- a/src/Microsoft.OpenApi.Readers/V2/OpenApiDocumentDeserializer.cs +++ b/src/Microsoft.OpenApi.Readers/V2/OpenApiDocumentDeserializer.cs @@ -237,8 +237,8 @@ private static string BuildUrl(string scheme, string host, string basePath) } // Remove default ports to clean up the URL - if ((uriBuilder.Scheme == "https" && uriBuilder.Port == 443) || - (uriBuilder.Scheme == "http" && uriBuilder.Port == 80)) + 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 }