Skip to content

Conversation

@Mahdigln
Copy link

@Mahdigln Mahdigln commented Oct 3, 2025

Fix port omission from BaseUrl in V2 server deserialization

Problem
When setting a BaseUrl with a non-standard port and the OpenAPI V2 document doesn't contain any server definitions, the port number was being omitted from the default server URL.

Example:

  • Input: BaseUrl = "http://demo.testfire.net:8080"
  • Current behavior: Server URL = "http://demo.testfire.net"
  • Expected behavior: Server URL = "http://demo.testfire.net:8080"

Root Cause
The MakeServers method in OpenApiDocumentDeserializer.cs was using UriComponents.NormalizedHost which only extracts the hostname without the port number.

Solution
Changed UriComponents.NormalizedHost to UriComponents.Host | UriComponents.Port to preserve port information when extracting host details from the BaseUrl. This approach is consistent with the existing pattern used in OpenApiDocument.WriteHostInfoV2.

Additionally, fixed the early return logic to allow server creation from BaseUrl even when no host, basePath, or schemes are defined in the document.

Changes

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

    • Line 154: Changed from UriComponents.NormalizedHost to UriComponents.Host | UriComponents.Port
    • Line 144: Modified early return condition to check for defaultUrl != null
  2. test/Microsoft.OpenApi.Readers.Tests/V2Tests/OpenApiServerTests.cs:

    • Added 3 new test cases to verify port preservation in various scenarios

Testing

  • All existing tests pass ✅
  • Added comprehensive test coverage for port preservation scenarios
  • 16/16 OpenApiServerTests passing
  • 78/78 V2Tests passing

Fixes #1294

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.
@Mahdigln Mahdigln requested a review from a team as a code owner October 3, 2025 07:22
@Mahdigln Mahdigln closed this Oct 3, 2025
@baywet
Copy link
Member

baywet commented Oct 3, 2025

@Mahdigln why did you close this one? While we're happy to still accept fixes for v1, the main active branch for v2 will need to have the fix as well.

For others reading #2525

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

When BaseUrl has a port, the default server url doesn't contain the port

2 participants