Split image into repo and tag arguments when relevant by OneBlue · Pull Request #14484 · microsoft/WSL

OneBlue · 2026-03-19T21:23:35Z

Summary of the Pull Request

This change splits the single "image" string into two "repo" and "tags" arguments when pulling or importing images.

This allows us to easily default to "latest" in these operations, and have the correct behavior when doing something like:

$ wslc pull debian # Now behaves as 'wslc pull debian:latest'

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

…oneblue/split-repo-tag

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.

Copilot · 2026-03-20T01:48:38Z

src/windows/common/wslutil.cpp

+    size_t separator = Input.find_last_of(':');
+    if (separator == std::string::npos)
+    {
+        return {Input, {}};
+    }
+
+    THROW_HR_WITH_USER_ERROR_IF(E_INVALIDARG, Localization::MessageWslaInvalidImage(Input), separator >= Input.size() - 1 || separator == 0);
+
+    return {Input.substr(0, separator), Input.substr(separator + 1)};


ParseImage splits on the last ':' unconditionally. This mis-parses valid image references that include a registry port but no explicit tag (e.g., "localhost:5000/repo"), treating "5000/repo" as the tag and preventing defaulting to "latest". Consider implementing Docker reference parsing rules (only treat ':' as a tag separator if it occurs after the last '/', and handle digests like "@sha256:") so repo/tag are derived correctly.

Suggested change

size_t separator = Input.find_last_of(':');

if (separator == std::string::npos)

{

return {Input, {}};

}

THROW_HR_WITH_USER_ERROR_IF(E_INVALIDARG, Localization::MessageWslaInvalidImage(Input), separator >= Input.size() - 1 || separator == 0);

return {Input.substr(0, separator), Input.substr(separator + 1)};

//

// Docker-like parsing rules:

// - A ':' is treated as a tag separator only if it occurs after the last '/'.

// - Any digest (e.g. "@sha256:...") comes after '@' and may contain ':'; these

// colons must not be treated as tag separators.

//

const size_t digestSeparator = Input.find('@');

// Portion used to locate a potential tag (before any '@' digest).

const std::string nameForTag = (digestSeparator == std::string::npos) ? Input : Input.substr(0, digestSeparator);

// Portion containing any digest (including '@'), which should remain attached to the repository.

const std::string digestPart = (digestSeparator == std::string::npos) ? std::string() : Input.substr(digestSeparator);

const size_t lastSlash = nameForTag.rfind('/');

const size_t lastColon = nameForTag.rfind(':');

// No ':' suitable for a tag separator -> no explicit tag, return image as-is.

if (lastColon == std::string::npos || (lastSlash != std::string::npos && lastColon <= lastSlash))

{

return {Input, {}};

}

// Validate that the tag separator is not the first character and that a tag is present.

THROW_HR_WITH_USER_ERROR_IF(

E_INVALIDARG,

Localization::MessageWslaInvalidImage(Input),

lastColon == 0 || lastColon >= nameForTag.size() - 1);

const std::string repoBase = nameForTag.substr(0, lastColon);

const std::string tag = nameForTag.substr(lastColon + 1);

// Reattach any digest part to the repository.

const std::string repo = repoBase + digestPart;

return {repo, tag};

Copilot · 2026-03-20T01:48:39Z

test/windows/WslcSdkTests.cpp

+        // Negative: null repo inside options must fail.
+        {
+            WslcPullImageOptions opts{};
+            opts.repo = "repo";
+            opts.tag = nullptr;


This negative test comment is duplicated: the second block is validating a null tag, not a null repo. Updating the comment will avoid confusion when interpreting failures.

Copilot · 2026-03-20T01:48:39Z

test/windows/WslcSdkTests.cpp

@@ -496,32 +503,43 @@ class WslcSdkTests
            VERIFY_IS_TRUE(GetFileSizeEx(imageTarFileHandle.get(), &fileSize));

            VERIFY_SUCCEEDED(WslcImportSessionImage(
-                m_defaultSession, c_handleImportedImageName, imageTarFileHandle.get(), static_cast<uint64_t>(fileSize.QuadPart), nullptr, nullptr));
+                m_defaultSession, "my-hello-world-handle", "test", imageTarFileHandle.get(), static_cast<uint64_t>(fileSize.QuadPart), nullptr, nullptr));

-            auto output = RunContainerAndCapture(m_defaultSession, c_handleImportedImageName, {"/hello"});
+            auto cleanup = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [&]() {
+                LOG_IF_FAILED(WslcDeleteSessionImage(m_defaultSession, "my-hello-world-handle:test", nullptr));
+            });


The import tests no longer delete the target image before attempting the import, and the cleanup scope_exit is only created after a successful import. If the image already exists from a previous run (or the import fails after partially creating it), this test can become flaky. Consider deleting the image upfront (ignore failures) and/or creating the cleanup guard before calling WslcImportSessionImage/WslcImportSessionImageFromFile so cleanup runs even when the import fails.

Copilot · 2026-03-20T01:48:39Z

src/windows/wslasession/WSLASession.cpp

+    RETURN_HR_IF_NULL(E_POINTER, Repo);
+    RETURN_HR_IF_NULL(E_POINTER, Tag);

-    auto [repo, tag] = ParseImage(ImageUri);
+    std::string repo{Repo};
+    std::string tag{Tag};



WSLASession::PullImage no longer parses/validates "repo:tag" and now accepts Repo/Tag separately, but it only checks for null pointers. This allows empty Tag/Repo (e.g., repo="foo", tag="") which previously would have been rejected by ParseImage. Consider adding explicit validation that Repo and Tag are non-empty (and possibly reusing the same invalid-image localization) to preserve input validation behavior and avoid passing invalid refs to dockerd.

Copilot · 2026-03-20T01:48:39Z

src/windows/wslasession/WSLASession.cpp

+    RETURN_HR_IF_NULL(E_POINTER, Repo);
+    RETURN_HR_IF_NULL(E_POINTER, Tag);
+    RETURN_HR_IF(E_INVALIDARG, strlen(Repo) + strlen(Tag) + 1 > WSLA_MAX_IMAGE_NAME_LENGTH);

-    THROW_HR_IF_MSG(E_INVALIDARG, !tag.has_value(), "Expected tag for image import: %hs", ImageName);
+    std::string repo{Repo};
+    std::string tag{Tag};


WSLASession::ImportImage now takes Repo/Tag separately but only validates null pointers and combined length. It should also reject empty Repo/Tag strings (e.g., tag="") to avoid importing under an invalid name and to match the prior ParseImage validation that ensured both parts were non-empty.

Copilot · 2026-03-20T01:48:40Z

src/windows/service/inc/wslaservice.idl

+    HRESULT PullImage([in] LPCSTR Repo, [in] LPCSTR Tag, [in, unique] const  WslaRegistryAuthInformation* RegistryAuthenticationInformation, [in, unique] IProgressCallback* ProgressCallback);
    HRESULT BuildImage([in] const WSLABuildImageOptions* Options, [in, unique] IProgressCallback* ProgressCallback);
    HRESULT LoadImage([in] ULONG ImageHandle, [in, unique] IProgressCallback* ProgressCallback, [in] ULONGLONG ContentLength);
-    HRESULT ImportImage([in] ULONG ImageHandle, [in] LPCSTR ImageName, [in, unique] IProgressCallback* ProgressCallback, [in] ULONGLONG ContentLength);
+    HRESULT ImportImage([in] ULONG ImageHandle, [in] LPCSTR Repo, [in] LPCSTR Tag, [in, unique] IProgressCallback* ProgressCallback, [in] ULONGLONG ContentLength);


Changing the IWSLASession COM method signatures (PullImage/ImportImage) is ABI-breaking for any existing clients compiled against the previous IDL. With the IID left unchanged, old clients can end up calling into the new vtable slot with a mismatched stack layout. Consider introducing a new interface (e.g., IWSLASession2 with a new uuid) or adding new methods while keeping the existing ones for backward compatibility, and update callers accordingly.

Copilot · 2026-03-20T01:48:40Z

src/windows/WslcSDK/wslcsdk.h

@@ -376,14 +377,20 @@ typedef struct WslcImportImageOptions

 STDAPI WslcImportSessionImage(
    _In_ WslcSession session,
-    _In_z_ PCSTR imageName,
+    _In_z_ PCSTR repo,
+    _In_z_ PCSTR tag,
    _In_ HANDLE imageContent,
    _In_ uint64_t imageContentLength,
    _In_opt_ const WslcImportImageOptions* options,
    _Outptr_opt_result_z_ PWSTR* errorMessage);

 STDAPI WslcImportSessionImageFromFile(
-    _In_ WslcSession session, _In_z_ PCSTR imageName, _In_z_ PCWSTR path, _In_opt_ const WslcImportImageOptions* options, _Outptr_opt_result_z_ PWSTR* errorMessage);
+    _In_ WslcSession session,
+    _In_z_ PCSTR repo,
+    _In_z_ PCSTR tag,
+    _In_z_ PCWSTR path,
+    _In_opt_ const WslcImportImageOptions* options,
+    _Outptr_opt_result_z_ PWSTR* errorMessage);


WslcSDK is a C ABI surface: changing WslcPullImageOptions layout (uri -> repo/tag) and the import function signatures is a binary breaking change for existing applications built against previous headers. Consider preserving the old entrypoints/struct (e.g., add new *V2 functions/options structs) or otherwise versioning the DLL/API so older clients fail gracefully rather than misinterpreting struct fields or calling with the wrong signature.

kevpar · 2026-03-20T04:07:17Z

src/windows/service/inc/wslaservice.idl


    // Image management.
-    HRESULT PullImage([in] LPCSTR ImageUri, [in, unique] const  WslaRegistryAuthInformation* RegistryAuthenticationInformation, [in, unique] IProgressCallback* ProgressCallback);
+    HRESULT PullImage([in] LPCSTR Repo, [in] LPCSTR Tag, [in, unique] const  WslaRegistryAuthInformation* RegistryAuthenticationInformation, [in, unique] IProgressCallback* ProgressCallback);


I think we should keep it so PullImage accepts just an image name, which is then parsed into repo and tag. Partly this is to make it so clients don't need to do the parsing themselves, but also because we need to support pulling an image by digest (e.g. docker pull ubuntu@sha256:66460d557b25769b102175144d538d88219c077c678a49af4afca6fbfc1b5252). You could make PullImage takes a repo, tag, and digest, as an alternative.

This comment from Copilot also touches on some of the parsing particularities. I don't know if there is a spec that defines exactly what form an image string can take, unfortunately.

Hmm that's a good point. The main reason for this change was that today if you run wslc pull debian, we pull every single image in the debian repo, which is obviously not the behavior that we want.

I'll rethink this. Maybe we can make this work by splitting the image into repo & tag on the service side and defaulting to latest if no tag is passed.

Sadly I don't think there's a way around us having to parse repo & tags

This looks like a good reference for an image parser: https://deepwiki.com/openshift/docker-distribution/9-image-references

OneBlue added 3 commits March 19, 2026 14:04

Save state

0369ffd

Prepare for PR

2372a1f

Format

ae0b361

Copilot AI review requested due to automatic review settings March 19, 2026 21:23

Copilot started reviewing on behalf of OneBlue March 19, 2026 21:25 View session

Copilot AI reviewed Mar 19, 2026

View reviewed changes

OneBlue added 3 commits March 19, 2026 18:16

Merge remote-tracking branch 'origin/feature/wsl-for-apps' into user/…

b87dfb6

…oneblue/split-repo-tag

Fix test

39874ce

Fix test

d88c5cd

OneBlue marked this pull request as ready for review March 20, 2026 01:42

OneBlue requested a review from a team as a code owner March 20, 2026 01:42

Copilot AI review requested due to automatic review settings March 20, 2026 01:42

Copilot started reviewing on behalf of OneBlue March 20, 2026 01:44 View session

Copilot AI reviewed Mar 20, 2026

View reviewed changes

kevpar reviewed Mar 20, 2026

View reviewed changes

-    size_t separator = Input.find_last_of(':');
-    if (separator == std::string::npos)
-    {
-        return {Input, {}};
-    }
-    THROW_HR_WITH_USER_ERROR_IF(E_INVALIDARG, Localization::MessageWslaInvalidImage(Input), separator >= Input.size() - 1 || separator == 0);
-    return {Input.substr(0, separator), Input.substr(separator + 1)};
+    //
+    // Docker-like parsing rules:
+    // - A ':' is treated as a tag separator only if it occurs after the last '/'.
+    // - Any digest (e.g. "@sha256:...") comes after '@' and may contain ':'; these
+    //   colons must not be treated as tag separators.
+    //
+    const size_t digestSeparator = Input.find('@');
+    // Portion used to locate a potential tag (before any '@' digest).
+    const std::string nameForTag = (digestSeparator == std::string::npos) ? Input : Input.substr(0, digestSeparator);
+    // Portion containing any digest (including '@'), which should remain attached to the repository.
+    const std::string digestPart = (digestSeparator == std::string::npos) ? std::string() : Input.substr(digestSeparator);
+    const size_t lastSlash = nameForTag.rfind('/');
+    const size_t lastColon = nameForTag.rfind(':');
+    // No ':' suitable for a tag separator -> no explicit tag, return image as-is.
+    if (lastColon == std::string::npos || (lastSlash != std::string::npos && lastColon <= lastSlash))
+    {
+        return {Input, {}};
+    }
+    // Validate that the tag separator is not the first character and that a tag is present.
+    THROW_HR_WITH_USER_ERROR_IF(
+        E_INVALIDARG,
+        Localization::MessageWslaInvalidImage(Input),
+        lastColon == 0 || lastColon >= nameForTag.size() - 1);
+    const std::string repoBase = nameForTag.substr(0, lastColon);
+    const std::string tag = nameForTag.substr(lastColon + 1);
+    // Reattach any digest part to the repository.
+    const std::string repo = repoBase + digestPart;
+    return {repo, tag};

Conversation

OneBlue commented Mar 19, 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

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

kevpar Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

kevpar Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

OneBlue Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

OneBlue Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants