Fix windows path handling in podman cp

Signed-off-by: David Negstad <[email protected]>

Author: danegsta

cmd/podman/containers/cp.go

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

pkg/copy/fileinfo.go

@@ -83,20 +83,20 @@ func ResolveHostPath(path string) (*FileInfo, error) {
 // is preserved.  The filepath API among tends to clean up a bit too much but
 // we *must* preserve this data by all means.
 func PreserveBasePath(original, resolved string) string {
-	// Handle "/"
-	if strings.HasSuffix(original, "/") {
-		if !strings.HasSuffix(resolved, "/") {
-			resolved += "/"
+	// Handle "/" or "\\" on Windows.
+	if strings.HasSuffix(original, string(os.PathSeparator)) {
+		if !strings.HasSuffix(resolved, string(os.PathSeparator)) {
+			resolved += string(os.PathSeparator)
 		}
 		return resolved
 	}
 
 	// Handle "/."
-	if strings.HasSuffix(original, "/.") {
-		if strings.HasSuffix(resolved, "/") { // could be root!
+	if strings.HasSuffix(original, string(os.PathSeparator)+".") {
+		if strings.HasSuffix(resolved, string(os.PathSeparator)) { // could be root!
 			resolved += "."
-		} else if !strings.HasSuffix(resolved, "/.") {
-			resolved += "/."
+		} else if !strings.HasSuffix(resolved, string(os.PathSeparator)+".") {
+			resolved += string(os.PathSeparator) + "."
 		}
 		return resolved
 	}

pkg/copy/parse.go

@@ -2,6 +2,7 @@ package copy
 
 import (
 	"fmt"
+	"path/filepath"
 	"strings"
 )
 
@@ -40,6 +41,14 @@ func parseUserInput(input string) (container string, path string) {
 		return
 	}
 
+	// If the input is an absolute path, it cannot refer to a container.
+	// This is necessary because absolute paths on Windows will include
+	// a colon, which would cause the drive letter to be parsed as a
+	// container name.
+	if filepath.IsAbs(input) {
+		return
+	}
+
 	if parsedContainer, parsedPath, ok := strings.Cut(path, ":"); ok {
 		container = parsedContainer
 		path = parsedPath

pkg/machine/e2e/basic_test.go

@@ -1,11 +1,8 @@
 package e2e_test
 
 import (
-	"archive/tar"
-	"bytes"
 	"fmt"
 	"io"
-	"io/fs"
 	"net"
 	"net/http"
 	"net/url"
@@ -262,87 +259,6 @@ var _ = Describe("run basic podman commands", func() {
 		Expect(run).To(Exit(0))
 		Expect(build.outputToString()).To(ContainSubstring(name))
 	})
-
-	It("Copy ops", func() {
-		var (
-			stdinDirectory = "stdin-dir"
-			stdinFile      = "file.txt"
-		)
-
-		now := time.Now()
-
-		tarBuffer := &bytes.Buffer{}
-		tw := tar.NewWriter(tarBuffer)
-
-		// Write a directory header to the tar
-		err := tw.WriteHeader(&tar.Header{
-			Name:       stdinDirectory,
-			Mode:       int64(0640 | fs.ModeDir),
-			Gid:        1000,
-			ModTime:    now,
-			ChangeTime: now,
-			AccessTime: now,
-			Typeflag:   tar.TypeDir,
-		})
-		Expect(err).ToNot(HaveOccurred())
-
-		// Write a file header to the tar
-		err = tw.WriteHeader(&tar.Header{
-			Name:       path.Join(stdinDirectory, stdinFile),
-			Mode:       0755,
-			Uid:        1000,
-			ModTime:    now,
-			ChangeTime: now,
-			AccessTime: now,
-		})
-		Expect(err).ToNot(HaveOccurred())
-
-		err = tw.Close()
-		Expect(err).ToNot(HaveOccurred())
-
-		name := randomString()
-		i := new(initMachine)
-		session, err := mb.setName(name).setCmd(i.withImage(mb.imagePath).withNow()).run()
-		Expect(err).ToNot(HaveOccurred())
-		Expect(session).To(Exit(0))
-
-		bm := basicMachine{}
-		newImgs, err := mb.setCmd(bm.withPodmanCommand([]string{"pull", TESTIMAGE})).run()
-		Expect(err).ToNot(HaveOccurred())
-		Expect(newImgs).To(Exit(0))
-		Expect(newImgs.outputToStringSlice()).To(HaveLen(1))
-
-		createAlp, err := mb.setCmd(bm.withPodmanCommand([]string{"create", TESTIMAGE, "top"})).run()
-		Expect(err).ToNot(HaveOccurred())
-		Expect(createAlp).To(Exit(0))
-		Expect(createAlp.outputToStringSlice()).To(HaveLen(1))
-
-		// Testing stdin copy with archive mode disabled (ownership will be determined by the tar file)
-		containerID := createAlp.outputToStringSlice()[0]
-		cpTar, err := mb.setCmd(bm.withPodmanCommand([]string{"cp", "-a=false", "-", containerID + ":/tmp"})).setStdin(tarBuffer).run()
-		Expect(err).ToNot(HaveOccurred())
-		Expect(cpTar).To(Exit(0))
-
-		start, err := mb.setCmd(bm.withPodmanCommand([]string{"start", containerID})).run()
-		Expect(err).ToNot(HaveOccurred())
-		Expect(start).To(Exit(0))
-
-		// Check the directory is created with the appropriate mode, uid, gid
-		exec, err := mb.setCmd(bm.withPodmanCommand([]string{"exec", containerID, "stat", "-c", "%a %u %g", "/tmp/stdin-dir"})).run()
-		Expect(err).ToNot(HaveOccurred())
-		Expect(exec).To(Exit(0))
-		execStdOut := exec.outputToStringSlice()
-		Expect(execStdOut).To(HaveLen(1))
-		Expect(execStdOut[0]).To(Equal("640 0 1000"))
-
-		// Check the file is created with the appropriate mode, uid, gid
-		exec, err = mb.setCmd(bm.withPodmanCommand([]string{"exec", containerID, "stat", "-c", "%a %u %g", "/tmp/stdin-dir/file.txt"})).run()
-		Expect(err).ToNot(HaveOccurred())
-		Expect(exec).To(Exit(0))
-		execStdOut = exec.outputToStringSlice()
-		Expect(execStdOut).To(HaveLen(1))
-		Expect(execStdOut[0]).To(Equal("755 1000 0"))
-	})
 })
 
 func testHTTPServer(port string, shouldErr bool, expectedResponse string) {