Skip to content

Commit 9216c7f

Browse files
committed
Fix windows path handling in podman cp
Fixes: containers#14862 Signed-off-by: David Negstad <[email protected]>
1 parent 53be17d commit 9216c7f

File tree

5 files changed

+226
-100
lines changed

5 files changed

+226
-100
lines changed

cmd/podman/containers/cp.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,11 @@ func copyToContainer(container string, containerPath string, hostPath string) er
428428
// rename accordingly.
429429
getOptions.Rename = map[string]string{filepath.Base(hostTarget): containerBaseName}
430430
}
431-
if err := buildahCopiah.Get("/", "", getOptions, []string{hostTarget}, writer); err != nil {
431+
432+
// On Windows, the root path needs to be <drive>:\, while otherwise
433+
// it needs to be /. Combining filepath.VolumeName() + string(os.PathSeparator)
434+
// gives us the correct path for the current OS.
435+
if err := buildahCopiah.Get(filepath.VolumeName(hostTarget)+string(os.PathSeparator), "", getOptions, []string{hostTarget}, writer); err != nil {
432436
return fmt.Errorf("copying from host: %w", err)
433437
}
434438
return nil

pkg/copy/fileinfo.go

+18-13
Original file line numberDiff line numberDiff line change
@@ -83,23 +83,28 @@ func ResolveHostPath(path string) (*FileInfo, error) {
8383
// is preserved. The filepath API among tends to clean up a bit too much but
8484
// we *must* preserve this data by all means.
8585
func PreserveBasePath(original, resolved string) string {
86-
// Handle "/"
87-
if strings.HasSuffix(original, "/") {
88-
if !strings.HasSuffix(resolved, "/") {
89-
resolved += "/"
86+
// Ensure paths are in platform semantics (replace / with \ on Windows)
87+
resolved = filepath.FromSlash(resolved)
88+
original = filepath.FromSlash(original)
89+
90+
if filepath.Base(resolved) != "." && filepath.Base(original) == "." {
91+
if !hasTrailingPathSeparator(resolved) {
92+
// Add a separator if it doesn't already end with one (a cleaned
93+
// path would only end in a separator if it is the root).
94+
resolved += string(filepath.Separator)
9095
}
91-
return resolved
96+
resolved += "."
9297
}
9398

94-
// Handle "/."
95-
if strings.HasSuffix(original, "/.") {
96-
if strings.HasSuffix(resolved, "/") { // could be root!
97-
resolved += "."
98-
} else if !strings.HasSuffix(resolved, "/.") {
99-
resolved += "/."
100-
}
101-
return resolved
99+
if !hasTrailingPathSeparator(resolved) && hasTrailingPathSeparator(original) {
100+
resolved += string(filepath.Separator)
102101
}
103102

104103
return resolved
105104
}
105+
106+
// hasTrailingPathSeparator returns whether the given
107+
// path ends with the system's path separator character.
108+
func hasTrailingPathSeparator(path string) bool {
109+
return len(path) > 0 && os.IsPathSeparator(path[len(path)-1])
110+
}

pkg/copy/parse.go

+9
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package copy
22

33
import (
44
"fmt"
5+
"path/filepath"
56
"strings"
67
)
78

@@ -40,6 +41,14 @@ func parseUserInput(input string) (container string, path string) {
4041
return
4142
}
4243

44+
// If the input is an absolute path, it cannot refer to a container.
45+
// This is necessary because absolute paths on Windows will include
46+
// a colon, which would cause the drive letter to be parsed as a
47+
// container name.
48+
if filepath.IsAbs(input) {
49+
return
50+
}
51+
4352
if parsedContainer, parsedPath, ok := strings.Cut(path, ":"); ok {
4453
container = parsedContainer
4554
path = parsedPath

pkg/machine/e2e/basic_test.go

-84
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
11
package e2e_test
22

33
import (
4-
"archive/tar"
5-
"bytes"
64
"fmt"
75
"io"
8-
"io/fs"
96
"net"
107
"net/http"
118
"net/url"
@@ -288,87 +285,6 @@ var _ = Describe("run basic podman commands", func() {
288285
Expect(run).To(Exit(0))
289286
Expect(build.outputToString()).To(ContainSubstring(name))
290287
})
291-
292-
It("Copy ops", func() {
293-
var (
294-
stdinDirectory = "stdin-dir"
295-
stdinFile = "file.txt"
296-
)
297-
298-
now := time.Now()
299-
300-
tarBuffer := &bytes.Buffer{}
301-
tw := tar.NewWriter(tarBuffer)
302-
303-
// Write a directory header to the tar
304-
err := tw.WriteHeader(&tar.Header{
305-
Name: stdinDirectory,
306-
Mode: int64(0640 | fs.ModeDir),
307-
Gid: 1000,
308-
ModTime: now,
309-
ChangeTime: now,
310-
AccessTime: now,
311-
Typeflag: tar.TypeDir,
312-
})
313-
Expect(err).ToNot(HaveOccurred())
314-
315-
// Write a file header to the tar
316-
err = tw.WriteHeader(&tar.Header{
317-
Name: path.Join(stdinDirectory, stdinFile),
318-
Mode: 0755,
319-
Uid: 1000,
320-
ModTime: now,
321-
ChangeTime: now,
322-
AccessTime: now,
323-
})
324-
Expect(err).ToNot(HaveOccurred())
325-
326-
err = tw.Close()
327-
Expect(err).ToNot(HaveOccurred())
328-
329-
name := randomString()
330-
i := new(initMachine)
331-
session, err := mb.setName(name).setCmd(i.withImage(mb.imagePath).withNow()).run()
332-
Expect(err).ToNot(HaveOccurred())
333-
Expect(session).To(Exit(0))
334-
335-
bm := basicMachine{}
336-
newImgs, err := mb.setCmd(bm.withPodmanCommand([]string{"pull", TESTIMAGE})).run()
337-
Expect(err).ToNot(HaveOccurred())
338-
Expect(newImgs).To(Exit(0))
339-
Expect(newImgs.outputToStringSlice()).To(HaveLen(1))
340-
341-
createAlp, err := mb.setCmd(bm.withPodmanCommand([]string{"create", TESTIMAGE, "top"})).run()
342-
Expect(err).ToNot(HaveOccurred())
343-
Expect(createAlp).To(Exit(0))
344-
Expect(createAlp.outputToStringSlice()).To(HaveLen(1))
345-
346-
// Testing stdin copy with archive mode disabled (ownership will be determined by the tar file)
347-
containerID := createAlp.outputToStringSlice()[0]
348-
cpTar, err := mb.setCmd(bm.withPodmanCommand([]string{"cp", "-a=false", "-", containerID + ":/tmp"})).setStdin(tarBuffer).run()
349-
Expect(err).ToNot(HaveOccurred())
350-
Expect(cpTar).To(Exit(0))
351-
352-
start, err := mb.setCmd(bm.withPodmanCommand([]string{"start", containerID})).run()
353-
Expect(err).ToNot(HaveOccurred())
354-
Expect(start).To(Exit(0))
355-
356-
// Check the directory is created with the appropriate mode, uid, gid
357-
exec, err := mb.setCmd(bm.withPodmanCommand([]string{"exec", containerID, "stat", "-c", "%a %u %g", "/tmp/stdin-dir"})).run()
358-
Expect(err).ToNot(HaveOccurred())
359-
Expect(exec).To(Exit(0))
360-
execStdOut := exec.outputToStringSlice()
361-
Expect(execStdOut).To(HaveLen(1))
362-
Expect(execStdOut[0]).To(Equal("640 0 1000"))
363-
364-
// Check the file is created with the appropriate mode, uid, gid
365-
exec, err = mb.setCmd(bm.withPodmanCommand([]string{"exec", containerID, "stat", "-c", "%a %u %g", "/tmp/stdin-dir/file.txt"})).run()
366-
Expect(err).ToNot(HaveOccurred())
367-
Expect(exec).To(Exit(0))
368-
execStdOut = exec.outputToStringSlice()
369-
Expect(execStdOut).To(HaveLen(1))
370-
Expect(execStdOut[0]).To(Equal("755 1000 0"))
371-
})
372288
})
373289

374290
func testHTTPServer(port string, shouldErr bool, expectedResponse string) {

pkg/machine/e2e/cp_test.go

+194-2
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,210 @@
11
package e2e_test
22

33
import (
4+
"archive/tar"
5+
"bytes"
46
"fmt"
7+
"io/fs"
58
"os"
9+
"path"
610
"path/filepath"
711
"runtime"
12+
"time"
813

914
. "github.com/onsi/ginkgo/v2"
1015
. "github.com/onsi/gomega"
1116
. "github.com/onsi/gomega/gexec"
1217
)
1318

14-
var _ = Describe("podman machine cp", func() {
15-
It("all tests", func() {
19+
var _ = Describe("run cp commands", func() {
20+
It("podman cp", func() {
21+
var (
22+
file = "foo.txt"
23+
24+
directory = "foo-dir"
25+
26+
fileInDirectory = "bar.txt"
27+
28+
stdinFile = "file.txt"
29+
30+
stdinDirectory = "stdin-dir"
31+
)
32+
33+
sourceDir, err := os.MkdirTemp("", "host-source")
34+
Expect(err).ToNot(HaveOccurred())
35+
36+
destinationDir, err := os.MkdirTemp("", "host-destination")
37+
Expect(err).ToNot(HaveOccurred())
38+
39+
f, err := os.Create(filepath.Join(sourceDir, file))
40+
Expect(err).ToNot(HaveOccurred())
41+
err = f.Close()
42+
Expect(err).ToNot(HaveOccurred())
43+
44+
// Get the file stat to check permissions later
45+
sourceFileStat, err := os.Stat(filepath.Join(sourceDir, file))
46+
Expect(err).ToNot(HaveOccurred())
47+
48+
err = os.MkdirAll(filepath.Join(sourceDir, directory), 0755)
49+
Expect(err).ToNot(HaveOccurred())
50+
51+
// Get the directory stat to check permissions later
52+
sourceDirStat, err := os.Stat(filepath.Join(sourceDir, directory))
53+
Expect(err).ToNot(HaveOccurred())
54+
55+
f, err = os.Create(filepath.Join(sourceDir, directory, fileInDirectory))
56+
Expect(err).ToNot(HaveOccurred())
57+
err = f.Close()
58+
Expect(err).ToNot(HaveOccurred())
59+
60+
// Get the file in directory stat to check permissions later
61+
sourceFileInDirStat, err := os.Stat(filepath.Join(sourceDir, directory, fileInDirectory))
62+
Expect(err).ToNot(HaveOccurred())
63+
64+
By("copy from host to container")
65+
name := randomString()
66+
i := new(initMachine)
67+
session, err := mb.setName(name).setCmd(i.withImage(mb.imagePath).withNow()).run()
68+
Expect(err).ToNot(HaveOccurred())
69+
Expect(session).To(Exit(0))
70+
71+
bm := basicMachine{}
72+
newImgs, err := mb.setCmd(bm.withPodmanCommand([]string{"pull", TESTIMAGE})).run()
73+
Expect(err).ToNot(HaveOccurred())
74+
Expect(newImgs).To(Exit(0))
75+
Expect(newImgs.outputToStringSlice()).To(HaveLen(1))
76+
77+
createAlp, err := mb.setCmd(bm.withPodmanCommand([]string{"create", TESTIMAGE, "top"})).run()
78+
Expect(err).ToNot(HaveOccurred())
79+
Expect(createAlp).To(Exit(0))
80+
Expect(createAlp.outputToStringSlice()).To(HaveLen(1))
81+
82+
containerID := createAlp.outputToStringSlice()[0]
83+
84+
// Copy a single file into the guest
85+
cpFile, err := mb.setCmd(bm.withPodmanCommand([]string{"cp", filepath.Join(sourceDir, file), containerID + ":/tmp/"})).run()
86+
Expect(err).ToNot(HaveOccurred())
87+
Expect(cpFile).To(Exit(0))
88+
89+
// Copy a directory into the guest
90+
cpDir, err := mb.setCmd(bm.withPodmanCommand([]string{"cp", filepath.Join(sourceDir, directory), containerID + ":/tmp"})).run()
91+
Expect(err).ToNot(HaveOccurred())
92+
Expect(cpDir).To(Exit(0))
93+
94+
start, err := mb.setCmd(bm.withPodmanCommand([]string{"start", containerID})).run()
95+
Expect(err).ToNot(HaveOccurred())
96+
Expect(start).To(Exit(0))
97+
98+
// Check the single file is created with the appropriate mode, uid, gid
99+
exec, err := mb.setCmd(bm.withPodmanCommand([]string{"exec", containerID, "stat", "-c", "%a %u %g", path.Join("/tmp", file)})).run()
100+
Expect(err).ToNot(HaveOccurred())
101+
Expect(exec).To(Exit(0))
102+
execStdOut := exec.outputToStringSlice()
103+
Expect(execStdOut).To(HaveLen(1))
104+
Expect(execStdOut[0]).To(Equal(fmt.Sprintf("%o %d %d", sourceFileStat.Mode().Perm(), 0, 0)))
105+
106+
// Check the directory is created with the appropriate mode, uid, gid
107+
exec, err = mb.setCmd(bm.withPodmanCommand([]string{"exec", containerID, "stat", "-c", "%a %u %g", path.Join("/tmp", directory)})).run()
108+
Expect(err).ToNot(HaveOccurred())
109+
Expect(exec).To(Exit(0))
110+
execStdOut = exec.outputToStringSlice()
111+
Expect(execStdOut).To(HaveLen(1))
112+
Expect(execStdOut[0]).To(Equal(fmt.Sprintf("%o %d %d", sourceDirStat.Mode().Perm(), 0, 0)))
113+
114+
// Check the file in the directory is created with the appropriate mode, uid, gid
115+
exec, err = mb.setCmd(bm.withPodmanCommand([]string{"exec", containerID, "stat", "-c", "%a %u %g", path.Join("/tmp", directory, fileInDirectory)})).run()
116+
Expect(err).ToNot(HaveOccurred())
117+
Expect(exec).To(Exit(0))
118+
execStdOut = exec.outputToStringSlice()
119+
Expect(execStdOut).To(HaveLen(1))
120+
Expect(execStdOut[0]).To(Equal(fmt.Sprintf("%o %d %d", sourceFileInDirStat.Mode().Perm(), 0, 0)))
121+
122+
By("copy from container to host")
123+
// Copy the file back from the container to the host
124+
cpFile, err = mb.setCmd(bm.withPodmanCommand([]string{"cp", containerID + ":" + path.Join("/tmp", file), destinationDir + string(os.PathSeparator)})).run()
125+
Expect(err).ToNot(HaveOccurred())
126+
Expect(cpFile).To(Exit(0))
127+
128+
// Get the file stat of the copied file to compare against the original
129+
destinationFileStat, err := os.Stat(filepath.Join(destinationDir, file))
130+
Expect(err).ToNot(HaveOccurred())
131+
Expect(destinationFileStat.Mode()).To(Equal(sourceFileStat.Mode()))
132+
// Compare the modification time of the file in the container and the host (with second level precision)
133+
Expect(destinationFileStat.ModTime()).To(BeTemporally("~", sourceFileStat.ModTime(), time.Second))
134+
135+
// Copy a directory back from the container to the host
136+
cpDir, err = mb.setCmd(bm.withPodmanCommand([]string{"cp", containerID + ":" + path.Join("/tmp", directory), destinationDir + string(os.PathSeparator)})).run()
137+
Expect(err).ToNot(HaveOccurred())
138+
Expect(cpDir).To(Exit(0))
139+
140+
// Get the stat of the copied directory to compare against the original
141+
destinationDirStat, err := os.Stat(filepath.Join(destinationDir, directory))
142+
Expect(err).ToNot(HaveOccurred())
143+
Expect(destinationDirStat.Mode()).To(Equal(sourceDirStat.Mode()))
144+
// Compare the modification time of the folder in the container and the host (with second level precision)
145+
Expect(destinationDirStat.ModTime()).To(BeTemporally("~", sourceDirStat.ModTime(), time.Second))
146+
147+
// Get the stat of the copied file in the directory to compare against the original
148+
destinationFileInDirStat, err := os.Stat(filepath.Join(sourceDir, directory, fileInDirectory))
149+
Expect(err).ToNot(HaveOccurred())
150+
Expect(destinationFileInDirStat.Mode()).To(Equal(sourceFileInDirStat.Mode()))
151+
// Compare the modification time of the file in the container and the host (with second level precision)
152+
Expect(destinationFileInDirStat.ModTime()).To(BeTemporally("~", sourceFileInDirStat.ModTime(), time.Second))
153+
154+
By("copy stdin to container")
155+
now := time.Now()
156+
tarBuffer := &bytes.Buffer{}
157+
tw := tar.NewWriter(tarBuffer)
158+
159+
// Write a directory header to the tar
160+
err = tw.WriteHeader(&tar.Header{
161+
Name: stdinDirectory,
162+
Mode: int64(0640 | fs.ModeDir),
163+
Gid: 1000,
164+
ModTime: now,
165+
ChangeTime: now,
166+
AccessTime: now,
167+
Typeflag: tar.TypeDir,
168+
})
169+
Expect(err).ToNot(HaveOccurred())
170+
171+
// Write a file header to the tar
172+
err = tw.WriteHeader(&tar.Header{
173+
Name: path.Join(stdinDirectory, stdinFile),
174+
Mode: 0755,
175+
Uid: 1000,
176+
ModTime: now,
177+
ChangeTime: now,
178+
AccessTime: now,
179+
})
180+
Expect(err).ToNot(HaveOccurred())
181+
182+
err = tw.Close()
183+
Expect(err).ToNot(HaveOccurred())
184+
185+
// Testing stdin copy with archive mode disabled (ownership will be determined by the tar file)
186+
cpTar, err := mb.setCmd(bm.withPodmanCommand([]string{"cp", "-a=false", "-", containerID + ":/tmp"})).setStdin(tarBuffer).run()
187+
Expect(err).ToNot(HaveOccurred())
188+
Expect(cpTar).To(Exit(0))
189+
190+
// Check the directory is created with the appropriate mode, uid, gid
191+
exec, err = mb.setCmd(bm.withPodmanCommand([]string{"exec", containerID, "stat", "-c", "%a %u %g", "/tmp/stdin-dir"})).run()
192+
Expect(err).ToNot(HaveOccurred())
193+
Expect(exec).To(Exit(0))
194+
execStdOut = exec.outputToStringSlice()
195+
Expect(execStdOut).To(HaveLen(1))
196+
Expect(execStdOut[0]).To(Equal("640 0 1000"))
197+
198+
// Check the file is created with the appropriate mode, uid, gid
199+
exec, err = mb.setCmd(bm.withPodmanCommand([]string{"exec", containerID, "stat", "-c", "%a %u %g", "/tmp/stdin-dir/file.txt"})).run()
200+
Expect(err).ToNot(HaveOccurred())
201+
Expect(exec).To(Exit(0))
202+
execStdOut = exec.outputToStringSlice()
203+
Expect(execStdOut).To(HaveLen(1))
204+
Expect(execStdOut[0]).To(Equal("755 1000 0"))
205+
})
206+
207+
It("podman machine cp", func() {
16208
// HOST FILE SYSTEM
17209
// ~/<ginkgo_tmp>
18210
// * foo.txt

0 commit comments

Comments
 (0)