Fix container cp rejecting paths that contain a colon - #1970
Open
OrtegaMatias wants to merge 1 commit into
Open
Conversation
`ContainerCopy.parsePathRef` split a "<id>:<path>" reference on every colon and rejected anything with more than one part, so `container cp` failed with "invalid path given" on a container path whose filename contains a colon — e.g. an ISO-8601 timestamp such as /var/log/app-2026-07-20T10:30:00.log (the file exists; only the parse fails). Split on the first colon only — a container id never contains a path separator, but the path may contain colons — and treat a reference whose pre-colon part contains '/' as a local path. This also lets local paths that contain a colon be copied.
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type of Change
Motivation and Context
container cprejected acontainer:pathreference whenever the path contained a colon — e.g. an ISO-8601 timestamp filename such as/var/log/app-2026-07-20T10:30:00.log— failing withinvalid path giveneven though the file exists.docker cpsplits the reference on the first colon only.Fixes #1969.
Description
ContainerCopy.parsePathRefusedcomponents(separatedBy: ":")and rejected any reference producing more than two parts. It now:/as a local path, which additionally lets local paths containing a colon be copied.Error behavior for genuinely invalid references (empty id, non-absolute container path) is unchanged.
PathRefis nowEquatableto support the added unit tests.Testing
Added
Tests/ContainerCommandsTests/ContainerCopyPathRefTests.swiftcovering: a plain local path, a container reference, a container path containing colons (the regression), a local path containing colons, an empty id, and a non-absolute container path.Transparency note: I was unable to run the in-repo test suite locally — this machine only has an Xcode 27 beta toolchain (which fails building a transitive dependency under strict concurrency) and the Command Line Tools, which lack the test frameworks. I verified the parser change in isolation against all the cases above and confirmed it compiles as part of the
ContainerCommandstarget. CI runs the added tests.