Rethink image parsing logic by OneBlue · Pull Request #14496 · microsoft/WSL

OneBlue · 2026-03-21T02:18:11Z

Summary of the Pull Request

This change fixes various issues with PullImage() not correctly handling the reference format. Most notably:

wslc pull debian would pull every single image in the debian repo. Now the tag is defaulted to latest if not specified.

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 improves WSLA image reference handling by centralizing image-reference parsing and fixing PullImage() behavior when no tag is specified (defaulting to latest instead of pulling all tags).

Changes:

Moved image reference parsing into wsl::windows::common::wslutil::ParseImage() with support for tags, digests, and registry ports.
Updated WSLASession::PullImage() to default missing tags to latest, and switched other call sites to the shared parser.
Added tests for image parsing and added (currently skipped) advanced pull scenarios.

Reviewed changes

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

File	Description
test/windows/WSLATests.cpp	Adds `ImageParsing` coverage and an advanced pull test (currently unconditionally skipped).
src/windows/wslasession/WSLASession.cpp	Uses the shared parser; defaults missing pull tags to `latest`.
src/windows/common/wslutil.h	Declares `wslutil::ParseImage()`.
src/windows/common/wslutil.cpp	Implements `wslutil::ParseImage()` to correctly split repo vs tag/digest (handling registry ports and digests).

Copilot · 2026-03-21T02:22:55Z

src/windows/wslasession/WSLASession.cpp

                // and lookup corresponding digest from the map
-                auto repoName = ParseImage(tag).first;
+                auto repoName = wslutil::ParseImage(tag).first;
                size_t colonPos = tag.find(':');


In ListImages(), colonPos is now unused after switching to wslutil::ParseImage(tag).first. This will trigger an unused-local warning (and may break builds if warnings are treated as errors). Remove colonPos or use it for the intended logic.

Suggested change

size_t colonPos = tag.find(':');

src/windows/wslasession/WSLASession.cpp

test/windows/WSLATests.cpp

…oneblue/parse-image

kevpar · 2026-03-23T18:20:40Z

Does this supersede #14484 ?

OneBlue · 2026-03-23T18:37:05Z

Does this supersede #14484 ?

It does indeed !

Copilot

Pull request overview

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

src/windows/wslasession/WSLASession.cpp

test/windows/WSLATests.cpp

kevpar · 2026-03-23T22:14:36Z

src/windows/wslasession/WSLASession.cpp

    RETURN_HR_IF_NULL(E_POINTER, ImageUri);

-    auto [repo, tag] = ParseImage(ImageUri);
+    auto [repo, tag] = wslutil::ParseImage(ImageUri);


Suggest renaming tag to tagOrDigest, and do the same in the Docker client PullImage signature.

src/windows/wslasession/WSLASession.cpp

kevpar · 2026-03-23T22:20:42Z

As an aside, I noticed we are implicitly prepending library here: https://github.com/microsoft/WSL/blob/feature/wsl-for-apps/src/windows/wslasession/DockerHTTPClient.cpp#L124

I don't think this is correct to do other than for Docker Hub images. Has image pull been tested with other registries?

OneBlue · 2026-03-23T22:58:45Z

As an aside, I noticed we are implicitly prepending library here: https://github.com/microsoft/WSL/blob/feature/wsl-for-apps/src/windows/wslasession/DockerHTTPClient.cpp#L124

I don't think this is correct to do other than for Docker Hub images. Has image pull been tested with other registries?

We don't support custom registries yet, but added a TODO to track that.

Copilot

Pull request overview

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

Copilot · 2026-03-23T23:02:50Z

src/windows/wslasession/WSLASession.cpp

+    }

    auto lock = m_lock.lock_shared();



PullImage() dereferences m_dockerClient without checking it has a value. Since Terminate() resets m_dockerClient, callers that invoke PullImage() after termination (or on a partially-initialized/failed session) can hit a crash/UB here. Add the same invalid-state guard used by the other image APIs (e.g., THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !m_dockerClient.has_value());) after taking the shared lock and before calling through it.

Suggested change

THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !m_dockerClient.has_value());

src/windows/wslasession/DockerHTTPClient.h

src/windows/wslasession/DockerHTTPClient.cpp

test/windows/WSLATests.cpp

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Copilot · 2026-03-24T01:51:43Z

test/windows/WSLATests.cpp

            VERIFY_ARE_EQUAL(expectedError, comError->Message.get());
        }
+
+        // Validat that PullImage() returns the appropriate error if the session is terminated.


Typo in comment: "Validat" should be "Validate".

Suggested change

// Validat that PullImage() returns the appropriate error if the session is terminated.

// Validate that PullImage() returns the appropriate error if the session is terminated.

Copilot · 2026-03-24T01:51:44Z

src/windows/common/wslutil.cpp

+    std::string identifier = "([a-f0-9]{64})";
+    std::string shortIdentifier = "([a-f0-9]{6,64})";


BuildImageReferenceRegex() declares identifier and shortIdentifier but never uses them. With /WX enabled in the build, this will fail compilation due to unreferenced local variable warnings. Remove these variables or use them in the constructed regex.

Suggested change

std::string identifier = "([a-f0-9]{64})";

std::string shortIdentifier = "([a-f0-9]{6,64})";

Copilot · 2026-03-24T01:51:44Z

src/windows/wslasession/WSLASession.cpp

+    RETURN_HR_IF_NULL(E_POINTER, Image);

    auto lock = m_lock.lock_shared();
+    THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !m_dockerClient.has_value());
+
+    auto [repo, tagOrDigest] = wslutil::ParseImage(Image);
+
+    if (!tagOrDigest.has_value())
+    {
+        tagOrDigest = "latest";
+    }


WSLASession::PullImage() validates Image != nullptr but does not enforce WSLA_MAX_IMAGE_NAME_LENGTH like other image APIs (e.g., ImportImage, InspectImage). Since this now runs a fairly complex std::regex parse, a very long input could cause unnecessary CPU/memory usage. Add a length check (returning E_INVALIDARG) before calling wslutil::ParseImage.

Copilot · 2026-03-24T01:51:44Z

src/windows/common/wslutil.cpp

    }
 }

+std::regex BuildImageReferenceRegex()


BuildImageReferenceRegex() is defined in the global namespace even though it appears to be a file-local helper. Consider moving it into an anonymous namespace or making it static to avoid exporting an unintended global symbol.

Suggested change

std::regex BuildImageReferenceRegex()

static std::regex BuildImageReferenceRegex()

Copilot · 2026-03-24T01:51:45Z

src/windows/common/wslutil.cpp

+
+    THROW_HR_IF_MSG(E_UNEXPECTED, !repo.matched, "Unexpected regex match. Input: %hs", Input.c_str());
+
+    if (digest.matched) // <repo>:[tag]@<digest] (If both digest and tag are specified, digest takes precedence).


Minor typo in comment: the reference format example has mismatched brackets (<repo>:[tag]@<digest]). Update it to a correctly bracketed form to avoid confusion.

Suggested change

if (digest.matched) // <repo>:[tag]@<digest] (If both digest and tag are specified, digest takes precedence).

if (digest.matched) // <repo>:[tag]@<digest> (If both digest and tag are specified, digest takes precedence).

OneBlue added 3 commits March 20, 2026 17:08

Save state

8b20f4f

Rethink image parsing to correctly handle different reference formats

570cfa6

Foramt

c8a716b

OneBlue requested a review from a team as a code owner March 21, 2026 02:18

Copilot AI review requested due to automatic review settings March 21, 2026 02:18

Copilot started reviewing on behalf of OneBlue March 21, 2026 02:18 View session

Copilot AI reviewed Mar 21, 2026

View reviewed changes

OneBlue changed the title ~~User/oneblue/parse image~~ Rethink image parsing logic Mar 23, 2026

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

4195d99

…oneblue/parse-image

Fix merge

b925ddd

Copilot AI review requested due to automatic review settings March 23, 2026 18:36

OneBlue mentioned this pull request Mar 23, 2026

Split image into repo and tag arguments when relevant #14484

Closed

6 tasks

Copilot started reviewing on behalf of OneBlue March 23, 2026 18:39 View session

Copilot AI reviewed Mar 23, 2026

View reviewed changes

src/windows/wslasession/WSLASession.cpp Outdated Show resolved Hide resolved

test/windows/WSLATests.cpp Outdated Show resolved Hide resolved

OneBlue added 2 commits March 23, 2026 12:53

Apply PR feedback

a10af01

Format

3a00480

kevpar reviewed Mar 23, 2026

View reviewed changes

src/windows/wslasession/WSLASession.cpp Show resolved Hide resolved

Apply PR feedback

c01098d

Copilot AI review requested due to automatic review settings March 23, 2026 22:58

Copilot started reviewing on behalf of OneBlue March 23, 2026 22:59 View session

Copilot AI reviewed Mar 23, 2026

View reviewed changes

kevpar approved these changes Mar 23, 2026

View reviewed changes

OneBlue added 2 commits March 23, 2026 17:53

Use a regex based parser

c15c137

Format

7ff1209

benhillis approved these changes Mar 24, 2026

View reviewed changes

OneBlue added 2 commits March 23, 2026 18:40

Reset the test session after termination

49ed3a0

Fix test case

fbf58a1

Copilot AI review requested due to automatic review settings March 24, 2026 01:47

Copilot started reviewing on behalf of OneBlue March 24, 2026 01:48 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

Apply PR feedback

591f81a

OneBlue merged commit 72295b8 into feature/wsl-for-apps Mar 24, 2026
7 checks passed



	THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !m_dockerClient.has_value());

	// Validat that PullImage() returns the appropriate error if the session is terminated.
	// Validate that PullImage() returns the appropriate error if the session is terminated.

		std::string identifier = "([a-f0-9]{64})";
		std::string shortIdentifier = "([a-f0-9]{6,64})";

	std::regex BuildImageReferenceRegex()
	static std::regex BuildImageReferenceRegex()


		THROW_HR_IF_MSG(E_UNEXPECTED, !repo.matched, "Unexpected regex match. Input: %hs", Input.c_str());

		if (digest.matched) // <repo>:[tag]@<digest] (If both digest and tag are specified, digest takes precedence).

Conversation

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

Reviewed changes

Uh oh!

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kevpar commented Mar 23, 2026

Uh oh!

OneBlue commented Mar 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

kevpar Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kevpar commented Mar 23, 2026

Uh oh!

OneBlue commented Mar 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants