Richfr/portmapping by 1wizkid · Pull Request #40037 · microsoft/WSL

1wizkid · 2026-03-30T16:38:16Z

Summary of the Pull Request

PR Checklist

Closes: Link to issue #xxx
Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
Tests: Added/updated if needed and all pass
Localization: All end user facing strings can be localized
Dev docs: Added/updated if needed
Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Copilot

Pull request overview

This PR updates the WSLC Client SDK’s container port-mapping path to support custom host binding addresses (including IPv6) instead of always binding to 127.0.0.1, and relaxes/updates validation around the windowsAddress field.

Changes:

Convert windowsAddress (IPv4/IPv6) into a string binding address for WSLCPortMapping during container creation.
Add input validation for windowsAddress->ss_family in WslcSetContainerSettingsPortMappings.

src/windows/WslcSDK/wslcsdk.cpp

Copilot · 2026-03-30T16:42:58Z

src/windows/WslcSDK/wslcsdk.cpp

+            if (internalPort.windowsAddress != nullptr)
+            {
+                char addrBuf[INET6_ADDRSTRLEN]{};
+                if (internalPort.windowsAddress->ss_family == AF_INET)
+                {
+                    const auto* addr4 = reinterpret_cast<const sockaddr_in*>(internalPort.windowsAddress);
+                    THROW_HR_IF_NULL(E_UNEXPECTED, inet_ntop(AF_INET, &addr4->sin_addr, addrBuf, sizeof(addrBuf)));
+                    convertedPort.Family = AF_INET;
+                }
+                else
+                {
+                    const auto* addr6 = reinterpret_cast<const sockaddr_in6*>(internalPort.windowsAddress);
+                    THROW_HR_IF_NULL(E_UNEXPECTED, inet_ntop(AF_INET6, &addr6->sin6_addr, addrBuf, sizeof(addrBuf)));
+                    convertedPort.Family = AF_INET6;
+                }
+                bindingAddressStrings[i] = addrBuf;
+                convertedPort.BindingAddress = bindingAddressStrings[i].c_str();
+            }
+            else
+            {
+                convertedPort.Family = AF_INET;
+                convertedPort.BindingAddress = "127.0.0.1";
+            }


This change adds support for custom binding addresses (including IPv6) when creating containers, but there are existing TAEF SDK tests for port mapping that currently only cover the default IPv4 localhost TCP case. Please add/extend tests to cover: (1) a mapping with windowsAddress set (IPv4 and IPv6) and (2) UDP mappings, to prevent regressions in address-family handling and string conversion.

src/windows/WslcSDK/wslcsdk.cpp

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

Copilot · 2026-04-01T17:10:11Z

src/windows/WslcSDK/wslcsdk.cpp

            WSLCPortMapping& convertedPort = convertedPorts[i];

            convertedPort.HostPort = internalPort.windowsPort;
            convertedPort.ContainerPort = internalPort.containerPort;


convertedPort.Family and convertedPort.BindingAddress are no longer initialized when internalPort.windowsAddress == nullptr. Previously these were always set (AF_INET + 127.0.0.1), so this change risks leaving uninitialized fields and producing undefined behavior downstream. Set explicit defaults before the if (e.g., loopback + AF_INET) and only override them when windowsAddress is provided.

Defaulting to:
convertedPort.Family = AF_UNSPEC;
convertedPort.BindingAddress = nullptr;

If not specified by the caller. Runtime will need to interpretive these value appropriately,

src/windows/WslcSDK/wslcsdk.cpp

Copilot · 2026-04-01T17:10:11Z

src/windows/WslcSDK/wslcsdk.cpp

    std::unique_ptr<WSLCPortMapping[]> convertedPorts;
+    std::vector<std::string> bindingAddressStrings;
    if (internalContainerSettings->ports && internalContainerSettings->portsCount)
    {
        convertedPorts = std::make_unique<WSLCPortMapping[]>(internalContainerSettings->portsCount);
+        bindingAddressStrings.resize(internalContainerSettings->portsCount);
        for (uint32_t i = 0; i < internalContainerSettings->portsCount; ++i)
        {


convertedPort.BindingAddress is set to bindingAddressStrings[i].c_str(), but bindingAddressStrings is local to this scope. If the downstream API referenced by containerOptions stores the pointer beyond the immediate call (rather than copying the string), this becomes a dangling pointer once the function exits. To make lifetime explicit/safe, prefer storage that is guaranteed to outlive the API usage (e.g., embed a fixed-size buffer in the runtime struct, or keep per-port heap-allocated buffers owned alongside convertedPorts, or confirm/ensure the callee deep-copies the string before returning).

Valid point, but I don't think warrants a change. The callee copies the data so we are good.

Copilot · 2026-04-01T17:10:11Z

src/windows/WslcSDK/wslcsdk.cpp

+                {
+                    const auto* addr4 = reinterpret_cast<const sockaddr_in*>(internalPort.windowsAddress);
+
+                    THROW_HR_IF_NULL(E_UNEXPECTED, inet_ntop(AF_INET, &addr4->sin_addr, addrBuf, sizeof(addrBuf)));


Using E_UNEXPECTED for inet_ntop failures makes troubleshooting difficult and can misclassify user input errors. Consider returning an HRESULT derived from the actual socket error (e.g., using WSAGetLastError() + HRESULT_FROM_WIN32) or E_INVALIDARG when the address is malformed, so callers get a more accurate failure reason.

We need to provide more exacting errors via ETW.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 6 comments.

Copilot · 2026-04-01T18:05:30Z

src/windows/WslcSDK/wslcsdk.cpp

+                // If no binding address is provided, default to wildcard.
+                convertedPort.Family = AF_UNSPEC;
+                convertedPort.BindingAddress = nullptr;


This changes the default binding behavior from the previous explicit loopback-only bind (127.0.0.1) to a wildcard/default bind (AF_UNSPEC + null), which can unintentionally expose forwarded ports on external interfaces. To preserve the prior security posture and avoid surprising behavior changes, default to loopback (e.g., 127.0.0.1 for IPv4 and/or ::1 for IPv6) unless the caller explicitly requests wildcard binding.

Suggested change

// If no binding address is provided, default to wildcard.

convertedPort.Family = AF_UNSPEC;

convertedPort.BindingAddress = nullptr;

// If no binding address is provided, default to loopback to avoid exposing ports externally.

bindingAddressStrings[i] = "127.0.0.1";

convertedPort.Family = AF_INET;

convertedPort.BindingAddress = bindingAddressStrings[i].c_str();

Copilot · 2026-04-01T18:05:31Z

src/windows/WslcSDK/wslcsdk.cpp

+                {
+                    const auto* addr4 = reinterpret_cast<const sockaddr_in*>(internalPort.windowsAddress);
+
+                    THROW_HR_IF_NULL(E_UNEXPECTED, inet_ntop(AF_INET, &addr4->sin_addr, addrBuf, sizeof(addrBuf)));


Using E_UNEXPECTED for inet_ntop failures makes diagnostics and caller handling harder. Prefer returning an error that reflects the underlying cause (e.g., converting WSAGetLastError() to an HRESULT, or using E_INVALIDARG when the input is malformed/unsupported), so failures are actionable.

Copilot · 2026-04-01T18:05:31Z

src/windows/WslcSDK/wslcsdk.cpp

+                {
+                    const auto* addr6 = reinterpret_cast<const sockaddr_in6*>(internalPort.windowsAddress);
+
+                    THROW_HR_IF_NULL(E_UNEXPECTED, inet_ntop(AF_INET6, &addr6->sin6_addr, addrBuf, sizeof(addrBuf)));


Using E_UNEXPECTED for inet_ntop failures makes diagnostics and caller handling harder. Prefer returning an error that reflects the underlying cause (e.g., converting WSAGetLastError() to an HRESULT, or using E_INVALIDARG when the input is malformed/unsupported), so failures are actionable.

Copilot · 2026-04-01T18:05:31Z

src/windows/WslcSDK/wslcsdk.cpp

    }

    std::unique_ptr<WSLCPortMapping[]> convertedPorts;
+    std::vector<std::string> bindingAddressStrings;


convertedPort.BindingAddress is set to a pointer into bindingAddressStrings storage. That pointer becomes invalid as soon as bindingAddressStrings is destroyed (end of scope), and also depends on the called API not retaining the pointer beyond the immediate call. To make the lifetime unambiguous and safe, store the address strings in memory owned by the same structure/lifetime as convertedPorts (e.g., a parallel heap allocation / fixed-size buffers per port) or ensure the downstream API copies the string and document that requirement here.

Copilot · 2026-04-01T18:05:31Z

src/windows/WslcSDK/wslcsdk.cpp

    if (internalContainerSettings->ports && internalContainerSettings->portsCount)
    {
        convertedPorts = std::make_unique<WSLCPortMapping[]>(internalContainerSettings->portsCount);
+        bindingAddressStrings.resize(internalContainerSettings->portsCount);


convertedPort.BindingAddress is set to a pointer into bindingAddressStrings storage. That pointer becomes invalid as soon as bindingAddressStrings is destroyed (end of scope), and also depends on the called API not retaining the pointer beyond the immediate call. To make the lifetime unambiguous and safe, store the address strings in memory owned by the same structure/lifetime as convertedPorts (e.g., a parallel heap allocation / fixed-size buffers per port) or ensure the downstream API copies the string and document that requirement here.

Copilot · 2026-04-01T18:05:32Z

src/windows/WslcSDK/wslcsdk.cpp

+                bindingAddressStrings[i] = addrBuf;
+                convertedPort.BindingAddress = bindingAddressStrings[i].c_str();


convertedPort.BindingAddress is set to a pointer into bindingAddressStrings storage. That pointer becomes invalid as soon as bindingAddressStrings is destroyed (end of scope), and also depends on the called API not retaining the pointer beyond the immediate call. To make the lifetime unambiguous and safe, store the address strings in memory owned by the same structure/lifetime as convertedPorts (e.g., a parallel heap allocation / fixed-size buffers per port) or ensure the downstream API copies the string and document that requirement here.

JohnMcPMS

Almost certainly needs a test update to prevent a negative test from failing when it passes in an address. Not sure if we can expect IPv6 everywhere, but at least passing in 127.0.0.1 (or maybe IPv6 loopback works even if no external IPv6 address is available?).

[Edit: Search suggests that IPv6 loopback is always available if device supports IPv6.]

OneBlue

Added a couple comments, I would also recommend:

Updating the pull request title
Adding test coverage for the binding address wiring (at least for localhost, since that's the only thing that the service supports today)

You'll probably have to update existing tests as well

OneBlue · 2026-04-01T19:16:13Z

src/windows/WslcSDK/wslcsdk.cpp

+
+                default:
+                    // Reject unsupported or malformed address families
+                    THROW_HR(E_INVALIDARG);


nit: I recommend THROW_HR_MSG() so we can log the address family we received to make debugging easier

OneBlue · 2026-04-01T19:18:30Z

src/windows/WslcSDK/wslcsdk.cpp

+            else
+            {
+                // If no binding address is provided, default to wildcard.
+                convertedPort.Family = AF_UNSPEC;


Won't this fail later if we leave AF_UNSPEC here ?

I would recommend either defaulting to localhost, or requiring the caller to pass in the binding address.

The later would be my preference, so that way there's no ambiguity

richfr added 2 commits March 30, 2026 09:21

Portmapping: add windowsAddress support

fe043b9

Portmapping: add windowsAddress support

04a4541

Copilot AI review requested due to automatic review settings March 30, 2026 16:38

Copilot started reviewing on behalf of 1wizkid March 30, 2026 16:39 View session

Copilot AI reviewed Mar 30, 2026

View reviewed changes

JohnMcPMS reviewed Mar 30, 2026

View reviewed changes

src/windows/WslcSDK/wslcsdk.cpp Outdated Show resolved Hide resolved

richfr added 2 commits April 1, 2026 09:32

Addressing PR feedback: update wslcsdk.cpp

cf34fb8

Addressing clang formatting error

954d1a0

Copilot AI review requested due to automatic review settings April 1, 2026 17:03

Copilot AI reviewed Apr 1, 2026

View reviewed changes

Copilot started reviewing on behalf of 1wizkid April 1, 2026 17:21 View session

richfr and others added 2 commits April 1, 2026 10:33

Providing default values for port bindings: wslcsdk.cpp

31e28b4

Merge branch 'feature/wsl-for-apps' into richfr/portmapping

7ca8e51

Copilot AI review requested due to automatic review settings April 1, 2026 17:58

1wizkid marked this pull request as ready for review April 1, 2026 18:00

1wizkid requested a review from a team as a code owner April 1, 2026 18:00

Copilot AI reviewed Apr 1, 2026

View reviewed changes

Copilot started reviewing on behalf of 1wizkid April 1, 2026 18:13 View session

Merge branch 'feature/wsl-for-apps' into richfr/portmapping

7f1eca4

1wizkid enabled auto-merge (squash) April 1, 2026 18:27

JohnMcPMS reviewed Apr 1, 2026

View reviewed changes

OneBlue requested changes Apr 1, 2026

View reviewed changes

		bindingAddressStrings[i] = addrBuf;
		convertedPort.BindingAddress = bindingAddressStrings[i].c_str();

Conversation

1wizkid commented Mar 30, 2026

Summary of the Pull Request

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

1wizkid Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

1wizkid Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

1wizkid Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

JohnMcPMS left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OneBlue left a comment

Choose a reason for hiding this comment

Uh oh!

OneBlue Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

OneBlue Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

1wizkid Apr 1, 2026 •

edited

Loading

1wizkid Apr 1, 2026 •

edited

Loading

JohnMcPMS left a comment •

edited

Loading