Skip to content

Commit 1328759

Browse files
Mahdi GolestanMahdi Golestan
authored andcommitted
Improve server creation and URL handling logic
Updated the `MakeServers` method to include checks for `defaultUrl` and refined host validation. Modified the `BuildUrl` method to correctly extract ports and remove default ports from URLs. Added unit tests in `OpenApiServerTests.cs` to ensure proper handling of base URLs and ports across various scenarios.
1 parent 717f154 commit 1328759

File tree

2 files changed

+186
-7
lines changed

2 files changed

+186
-7
lines changed

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

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -124,14 +124,14 @@ private static void MakeServers(IList<OpenApiServer> servers, ParsingContext con
124124
basePath = "/";
125125
}
126126

127-
// If nothing is provided, don't create a server
128-
if (host == null && basePath == null && schemes == null)
127+
// If nothing is provided and there's no defaultUrl, don't create a server
128+
if (string.IsNullOrEmpty(host) && string.IsNullOrEmpty(basePath) && (schemes == null || schemes.Count == 0) && defaultUrl == null)
129129
{
130130
return;
131131
}
132132

133133
//Validate host
134-
if (host != null && !IsHostValid(host))
134+
if (!string.IsNullOrEmpty(host) && !IsHostValid(host!))
135135
{
136136
rootNode.Context.Diagnostic.Errors.Add(new(rootNode.Context.GetLocation(), "Invalid host"));
137137
return;
@@ -140,7 +140,7 @@ private static void MakeServers(IList<OpenApiServer> servers, ParsingContext con
140140
// Fill in missing information based on the defaultUrl
141141
if (defaultUrl != null)
142142
{
143-
host = host ?? defaultUrl.GetComponents(UriComponents.NormalizedHost, UriFormat.SafeUnescaped);
143+
host = host ?? defaultUrl.GetComponents(UriComponents.Host | UriComponents.Port, UriFormat.SafeUnescaped);
144144
basePath = basePath ?? defaultUrl.GetComponents(UriComponents.Path, UriFormat.SafeUnescaped);
145145
schemes = schemes ?? [defaultUrl.GetComponents(UriComponents.Scheme, UriFormat.SafeUnescaped)];
146146
}
@@ -150,7 +150,7 @@ private static void MakeServers(IList<OpenApiServer> servers, ParsingContext con
150150
}
151151

152152
// Create the Server objects
153-
if (schemes is {Count: > 0})
153+
if (schemes is { Count: > 0 })
154154
{
155155
foreach (var scheme in schemes)
156156
{
@@ -202,8 +202,8 @@ private static string BuildUrl(string? scheme, string? host, string? basePath)
202202
if (pieces is not null)
203203
{
204204
host = pieces[0];
205-
port = int.Parse(pieces[pieces.Count() -1], CultureInfo.InvariantCulture);
206-
}
205+
port = int.Parse(pieces[pieces.Count() - 1], CultureInfo.InvariantCulture);
206+
}
207207
}
208208

209209
var uriBuilder = new UriBuilder
@@ -218,6 +218,13 @@ private static string BuildUrl(string? scheme, string? host, string? basePath)
218218
uriBuilder.Port = port.Value;
219219
}
220220

221+
// Remove default ports to clean up the URL
222+
if (("https".Equals(uriBuilder.Scheme, StringComparison.OrdinalIgnoreCase) && uriBuilder.Port == 443) ||
223+
("http".Equals(uriBuilder.Scheme, StringComparison.OrdinalIgnoreCase) && uriBuilder.Port == 80))
224+
{
225+
uriBuilder.Port = -1; // Setting to -1 removes the port from the URL
226+
}
227+
221228
return uriBuilder.ToString();
222229
}
223230

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

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,5 +318,177 @@ public void InvalidHostShouldYieldError()
318318
Format = OpenApiConstants.Yaml
319319
}, result.Diagnostic);
320320
}
321+
322+
[Fact]
323+
public void BaseUrlWithPortShouldPreservePort()
324+
{
325+
var input =
326+
"""
327+
swagger: 2.0
328+
info:
329+
title: test
330+
version: 1.0.0
331+
paths: {}
332+
""";
333+
var settings = new OpenApiReaderSettings
334+
{
335+
BaseUrl = new("http://demo.testfire.net:8080")
336+
};
337+
settings.AddYamlReader();
338+
339+
var result = OpenApiDocument.Parse(input, "yaml", settings);
340+
341+
var server = result.Document.Servers.First();
342+
Assert.Single(result.Document.Servers);
343+
Assert.Equal("http://demo.testfire.net:8080", server.Url);
344+
}
345+
346+
[Fact]
347+
public void BaseUrlWithPortAndPathShouldPreservePort()
348+
{
349+
var input =
350+
"""
351+
swagger: 2.0
352+
info:
353+
title: test
354+
version: 1.0.0
355+
paths: {}
356+
""";
357+
var settings = new OpenApiReaderSettings
358+
{
359+
BaseUrl = new("http://demo.testfire.net:8080/swagger/properties.json")
360+
};
361+
settings.AddYamlReader();
362+
363+
var result = OpenApiDocument.Parse(input, "yaml", settings);
364+
365+
var server = result.Document.Servers.First();
366+
Assert.Single(result.Document.Servers);
367+
Assert.Equal("http://demo.testfire.net:8080/swagger/properties.json", server.Url);
368+
}
369+
370+
[Fact]
371+
public void BaseUrlWithNonStandardPortShouldPreservePort()
372+
{
373+
var input =
374+
"""
375+
swagger: 2.0
376+
info:
377+
title: test
378+
version: 1.0.0
379+
paths: {}
380+
""";
381+
var settings = new OpenApiReaderSettings
382+
{
383+
BaseUrl = new("https://api.example.com:9443/v1/openapi.yaml")
384+
};
385+
settings.AddYamlReader();
386+
387+
var result = OpenApiDocument.Parse(input, "yaml", settings);
388+
389+
var server = result.Document.Servers.First();
390+
Assert.Single(result.Document.Servers);
391+
Assert.Equal("https://api.example.com:9443/v1/openapi.yaml", server.Url);
392+
}
393+
394+
[Fact]
395+
public void BaseUrlWithStandardHttpsPortShouldRemovePort()
396+
{
397+
var input =
398+
"""
399+
swagger: 2.0
400+
info:
401+
title: test
402+
version: 1.0.0
403+
paths: {}
404+
""";
405+
var settings = new OpenApiReaderSettings
406+
{
407+
BaseUrl = new("https://foo.bar:443/api")
408+
};
409+
settings.AddYamlReader();
410+
411+
var result = OpenApiDocument.Parse(input, "yaml", settings);
412+
413+
var server = result.Document.Servers.First();
414+
Assert.Single(result.Document.Servers);
415+
Assert.Equal("https://foo.bar/api", server.Url);
416+
}
417+
418+
[Fact]
419+
public void BaseUrlWithStandardHttpPortShouldRemovePort()
420+
{
421+
var input =
422+
"""
423+
swagger: 2.0
424+
info:
425+
title: test
426+
version: 1.0.0
427+
paths: {}
428+
""";
429+
var settings = new OpenApiReaderSettings
430+
{
431+
BaseUrl = new("http://foo.bar:80/api")
432+
};
433+
settings.AddYamlReader();
434+
435+
var result = OpenApiDocument.Parse(input, "yaml", settings);
436+
437+
var server = result.Document.Servers.First();
438+
Assert.Single(result.Document.Servers);
439+
Assert.Equal("http://foo.bar/api", server.Url);
440+
}
441+
442+
[Fact]
443+
public void HostWithStandardHttpsPortShouldRemovePort()
444+
{
445+
var input =
446+
"""
447+
swagger: 2.0
448+
info:
449+
title: test
450+
version: 1.0.0
451+
host: foo.bar:443
452+
schemes:
453+
- https
454+
paths: {}
455+
""";
456+
var settings = new OpenApiReaderSettings
457+
{
458+
};
459+
settings.AddYamlReader();
460+
461+
var result = OpenApiDocument.Parse(input, "yaml", settings);
462+
463+
var server = result.Document.Servers.First();
464+
Assert.Single(result.Document.Servers);
465+
Assert.Equal("https://foo.bar", server.Url);
466+
}
467+
468+
[Fact]
469+
public void HostWithStandardHttpPortShouldRemovePort()
470+
{
471+
var input =
472+
"""
473+
swagger: 2.0
474+
info:
475+
title: test
476+
version: 1.0.0
477+
host: foo.bar:80
478+
schemes:
479+
- http
480+
paths: {}
481+
""";
482+
var settings = new OpenApiReaderSettings
483+
{
484+
};
485+
settings.AddYamlReader();
486+
487+
var result = OpenApiDocument.Parse(input, "yaml", settings);
488+
489+
var server = result.Document.Servers.First();
490+
Assert.Single(result.Document.Servers);
491+
Assert.Equal("http://foo.bar", server.Url);
492+
}
321493
}
322494
}

0 commit comments

Comments
 (0)