Use separate pathSpec for local and remote to properly handle cleaning paths

staging/src/k8s.io/kubectl/pkg/cmd/cp/cp.go

@@ -24,11 +24,8 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
-	"path"
-	"path/filepath"
 	"strings"
 
-	"github.com/lithammer/dedent"
 	"github.com/spf13/cobra"
 
 	"k8s.io/cli-runtime/pkg/genericclioptions"
@@ -67,12 +64,6 @@ var (
 
 		# Copy /tmp/foo from a remote pod to /tmp/bar locally
 		kubectl cp <some-namespace>/<some-pod>:/tmp/foo /tmp/bar`))
-
-	cpUsageStr = dedent.Dedent(`
-		expected 'cp <file-spec-src> <file-spec-dest> [-c container]'.
-		<file-spec> is:
-		[namespace/]pod-name:/file/path for a remote file
-		/file/path for a local file`)
 )
 
 // CopyOptions have the data required to perform the copy operation
@@ -158,6 +149,7 @@ func NewCmdCp(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *cobra.C
 		},
 		Run: func(cmd *cobra.Command, args []string) {
 			cmdutil.CheckErr(o.Complete(f, cmd))
+			cmdutil.CheckErr(o.Validate(cmd, args))
 			cmdutil.CheckErr(o.Run(args))
 		},
 	}
@@ -167,27 +159,22 @@ func NewCmdCp(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *cobra.C
 	return cmd
 }
 
-type fileSpec struct {
-	PodNamespace string
-	PodName      string
-	File         string
-}
-
 var (
 	errFileSpecDoesntMatchFormat = errors.New("filespec must match the canonical format: [[namespace/]pod:]file/path")
-	errFileCannotBeEmpty         = errors.New("filepath can not be empty")
 )
 
 func extractFileSpec(arg string) (fileSpec, error) {
 	i := strings.Index(arg, ":")
 
-	if i == -1 {
-		return fileSpec{File: arg}, nil
-	}
 	// filespec starting with a semicolon is invalid
 	if i == 0 {
 		return fileSpec{}, errFileSpecDoesntMatchFormat
 	}
+	if i == -1 {
+		return fileSpec{
+			File: newLocalPath(arg),
+		}, nil
+	}
 
 	pod, file := arg[:i], arg[i+1:]
 	pieces := strings.Split(pod, "/")
@@ -195,13 +182,13 @@ func extractFileSpec(arg string) (fileSpec, error) {
 	case 1:
 		return fileSpec{
 			PodName: pieces[0],
-			File:    file,
+			File:    newRemotePath(file),
 		}, nil
 	case 2:
 		return fileSpec{
 			PodNamespace: pieces[0],
 			PodName:      pieces[1],
-			File:         file,
+			File:         newRemotePath(file),
 		}, nil
 	default:
 		return fileSpec{}, errFileSpecDoesntMatchFormat
@@ -235,16 +222,13 @@ func (o *CopyOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error {
 // Validate makes sure provided values for CopyOptions are valid
 func (o *CopyOptions) Validate(cmd *cobra.Command, args []string) error {
 	if len(args) != 2 {
-		return cmdutil.UsageErrorf(cmd, cpUsageStr)
+		return fmt.Errorf("source and destination are required")
 	}
 	return nil
 }
 
 // Run performs the execution
 func (o *CopyOptions) Run(args []string) error {
-	if len(args) < 2 {
-		return fmt.Errorf("source and destination are required")
-	}
 	srcSpec, err := extractFileSpec(args[0])
 	if err != nil {
 		return err
@@ -257,6 +241,9 @@ func (o *CopyOptions) Run(args []string) error {
 	if len(srcSpec.PodName) != 0 && len(destSpec.PodName) != 0 {
 		return fmt.Errorf("one of src or dest must be a local file specification")
 	}
+	if len(srcSpec.File.String()) == 0 || len(destSpec.File.String()) == 0 {
+		return errors.New("filepath can not be empty")
+	}
 
 	if len(srcSpec.PodName) != 0 {
 		return o.copyFromPod(srcSpec, destSpec)
@@ -283,7 +270,7 @@ func (o *CopyOptions) checkDestinationIsDir(dest fileSpec) error {
 			PodName:   dest.PodName,
 		},
 
-		Command:  []string{"test", "-d", dest.File},
+		Command:  []string{"test", "-d", dest.File.String()},
 		Executor: &exec.DefaultRemoteExecutor{},
 	}
 
@@ -291,29 +278,24 @@ func (o *CopyOptions) checkDestinationIsDir(dest fileSpec) error {
 }
 
 func (o *CopyOptions) copyToPod(src, dest fileSpec, options *exec.ExecOptions) error {
-	if len(src.File) == 0 || len(dest.File) == 0 {
-		return errFileCannotBeEmpty
-	}
-	if _, err := os.Stat(src.File); err != nil {
+	if _, err := os.Stat(src.File.String()); err != nil {
 		return fmt.Errorf("%s doesn't exist in local filesystem", src.File)
 	}
 	reader, writer := io.Pipe()
 
-	// strip trailing slash (if any)
-	if dest.File != "/" && strings.HasSuffix(string(dest.File[len(dest.File)-1]), "/") {
-		dest.File = dest.File[:len(dest.File)-1]
-	}
+	srcFile := src.File.(localPath)
+	destFile := dest.File.(remotePath)
 
 	if err := o.checkDestinationIsDir(dest); err == nil {
 		// If no error, dest.File was found to be a directory.
 		// Copy specified src into it
-		dest.File = dest.File + "/" + path.Base(src.File)
+		destFile = destFile.Join(srcFile.Base())
 	}
 
-	go func() {
+	go func(src localPath, dest remotePath, writer io.WriteCloser) {
 		defer writer.Close()
-		cmdutil.CheckErr(makeTar(src.File, dest.File, writer))
-	}()
+		cmdutil.CheckErr(makeTar(src, dest, writer))
+	}(srcFile, destFile, writer)
 	var cmdArr []string
 
 	// TODO: Improve error messages by first testing if 'tar' is present in the container?
@@ -322,9 +304,9 @@ func (o *CopyOptions) copyToPod(src, dest fileSpec, options *exec.ExecOptions) e
 	} else {
 		cmdArr = []string{"tar", "-xmf", "-"}
 	}
-	destDir := path.Dir(dest.File)
-	if len(destDir) > 0 {
-		cmdArr = append(cmdArr, "-C", destDir)
+	destFileDir := destFile.Dir().String()
+	if len(destFileDir) > 0 {
+		cmdArr = append(cmdArr, "-C", destFileDir)
 	}
 
 	options.StreamOptions = exec.StreamOptions{
@@ -345,10 +327,6 @@ func (o *CopyOptions) copyToPod(src, dest fileSpec, options *exec.ExecOptions) e
 }
 
 func (o *CopyOptions) copyFromPod(src, dest fileSpec) error {
-	if len(src.File) == 0 || len(dest.File) == 0 {
-		return errFileCannotBeEmpty
-	}
-
 	reader, outStream := io.Pipe()
 	options := &exec.ExecOptions{
 		StreamOptions: exec.StreamOptions{
@@ -363,7 +341,7 @@ func (o *CopyOptions) copyFromPod(src, dest fileSpec) error {
 		},
 
 		// TODO: Improve error messages by first testing if 'tar' is present in the container?
-		Command:  []string{"tar", "cf", "-", src.File},
+		Command:  []string{"tar", "cf", "-", src.File.String()},
 		Executor: &exec.DefaultRemoteExecutor{},
 	}
 
@@ -371,49 +349,28 @@ func (o *CopyOptions) copyFromPod(src, dest fileSpec) error {
 		defer outStream.Close()
 		cmdutil.CheckErr(o.execute(options))
 	}()
-	prefix := getPrefix(src.File)
-	prefix = path.Clean(prefix)
+
+	srcFile := src.File.(remotePath)
+	destFile := dest.File.(localPath)
+
 	// remove extraneous path shortcuts - these could occur if a path contained extra "../"
 	// and attempted to navigate beyond "/" in a remote filesystem
-	prefix = stripPathShortcuts(prefix)
-	return o.untarAll(src, reader, dest.File, prefix)
+	prefix := stripPathShortcuts(srcFile.StripSlashes().Clean().String())
+	return o.untarAll(src.PodNamespace, src.PodName, prefix, srcFile, destFile, reader)
 }
 
-// stripPathShortcuts removes any leading or trailing "../" from a given path
-func stripPathShortcuts(p string) string {
-	newPath := path.Clean(p)
-	trimmed := strings.TrimPrefix(newPath, "../")
-
-	for trimmed != newPath {
-		newPath = trimmed
-		trimmed = strings.TrimPrefix(newPath, "../")
-	}
-
-	// trim leftover {".", ".."}
-	if newPath == "." || newPath == ".." {
-		newPath = ""
-	}
-
-	if len(newPath) > 0 && string(newPath[0]) == "/" {
-		return newPath[1:]
-	}
-
-	return newPath
-}
-
-func makeTar(srcPath, destPath string, writer io.Writer) error {
+func makeTar(src localPath, dest remotePath, writer io.Writer) error {
 	// TODO: use compression here?
 	tarWriter := tar.NewWriter(writer)
 	defer tarWriter.Close()
 
-	srcPath = path.Clean(srcPath)
-	destPath = path.Clean(destPath)
-	return recursiveTar(path.Dir(srcPath), path.Base(srcPath), path.Dir(destPath), path.Base(destPath), tarWriter)
+	srcPath := src.Clean()
+	destPath := dest.Clean()
+	return recursiveTar(srcPath.Dir(), srcPath.Base(), destPath.Dir(), destPath.Base(), tarWriter)
 }
 
-func recursiveTar(srcBase, srcFile, destBase, destFile string, tw *tar.Writer) error {
-	srcPath := path.Join(srcBase, srcFile)
-	matchedPaths, err := filepath.Glob(srcPath)
+func recursiveTar(srcDir, srcFile localPath, destDir, destFile remotePath, tw *tar.Writer) error {
+	matchedPaths, err := srcDir.Join(srcFile).Glob()
 	if err != nil {
 		return err
 	}
@@ -430,13 +387,14 @@ func recursiveTar(srcBase, srcFile, destBase, destFile string, tw *tar.Writer) e
 			if len(files) == 0 {
 				//case empty directory
 				hdr, _ := tar.FileInfoHeader(stat, fpath)
-				hdr.Name = destFile
+				hdr.Name = destFile.String()
 				if err := tw.WriteHeader(hdr); err != nil {
 					return err
 				}
 			}
 			for _, f := range files {
-				if err := recursiveTar(srcBase, path.Join(srcFile, f.Name()), destBase, path.Join(destFile, f.Name()), tw); err != nil {
+				if err := recursiveTar(srcDir, srcFile.Join(newLocalPath(f.Name())),
+					destDir, destFile.Join(newRemotePath(f.Name())), tw); err != nil {
 					return err
 				}
 			}
@@ -450,7 +408,7 @@ func recursiveTar(srcBase, srcFile, destBase, destFile string, tw *tar.Writer) e
 			}
 
 			hdr.Linkname = target
-			hdr.Name = destFile
+			hdr.Name = destFile.String()
 			if err := tw.WriteHeader(hdr); err != nil {
 				return err
 			}
@@ -460,7 +418,7 @@ func recursiveTar(srcBase, srcFile, destBase, destFile string, tw *tar.Writer) e
 			if err != nil {
 				return err
 			}
-			hdr.Name = destFile
+			hdr.Name = destFile.String()
 
 			if err := tw.WriteHeader(hdr); err != nil {
 				return err
@@ -481,7 +439,7 @@ func recursiveTar(srcBase, srcFile, destBase, destFile string, tw *tar.Writer) e
 	return nil
 }
 
-func (o *CopyOptions) untarAll(src fileSpec, reader io.Reader, destDir, prefix string) error {
+func (o *CopyOptions) untarAll(ns, pod string, prefix string, src remotePath, dest localPath, reader io.Reader) error {
 	symlinkWarningPrinted := false
 	// TODO: use compression here?
 	tarReader := tar.NewReader(reader)
@@ -505,19 +463,21 @@ func (o *CopyOptions) untarAll(src fileSpec, reader io.Reader, destDir, prefix s
 
 		// basic file information
 		mode := header.FileInfo().Mode()
-		destFileName := filepath.Join(destDir, header.Name[len(prefix):])
+		// header.Name is a name of the REMOTE file, so we need to create
+		// a remotePath so that it goes through appropriate processing related
+		// with cleaning remote paths
+		destFileName := dest.Join(newRemotePath(header.Name[len(prefix):]))
 
-		if !isDestRelative(destDir, destFileName) {
+		if !isRelative(dest, destFileName) {
 			fmt.Fprintf(o.IOStreams.ErrOut, "warning: file %q is outside target destination, skipping\n", destFileName)
 			continue
 		}
 
-		baseName := filepath.Dir(destFileName)
-		if err := os.MkdirAll(baseName, 0755); err != nil {
+		if err := os.MkdirAll(destFileName.Dir().String(), 0755); err != nil {
 			return err
 		}
 		if header.FileInfo().IsDir() {
-			if err := os.MkdirAll(destFileName, 0755); err != nil {
+			if err := os.MkdirAll(destFileName.String(), 0755); err != nil {
 				return err
 			}
 			continue
@@ -525,14 +485,16 @@ func (o *CopyOptions) untarAll(src fileSpec, reader io.Reader, destDir, prefix s
 
 		if mode&os.ModeSymlink != 0 {
 			if !symlinkWarningPrinted && len(o.ExecParentCmdName) > 0 {
-				fmt.Fprintf(o.IOStreams.ErrOut, "warning: skipping symlink: %q -> %q (consider using \"%s exec -n %q %q -- tar cf - %q | tar xf -\")\n", destFileName, header.Linkname, o.ExecParentCmdName, src.PodNamespace, src.PodName, src.File)
+				fmt.Fprintf(o.IOStreams.ErrOut,
+					"warning: skipping symlink: %q -> %q (consider using \"%s exec -n %q %q -- tar cf - %q | tar xf -\")\n",
+					destFileName, header.Linkname, o.ExecParentCmdName, ns, pod, src)
 				symlinkWarningPrinted = true
 				continue
 			}
 			fmt.Fprintf(o.IOStreams.ErrOut, "warning: skipping symlink: %q -> %q\n", destFileName, header.Linkname)
 			continue
 		}
-		outFile, err := os.Create(destFileName)
+		outFile, err := os.Create(destFileName.String())
 		if err != nil {
 			return err
 		}
@@ -548,21 +510,6 @@ func (o *CopyOptions) untarAll(src fileSpec, reader io.Reader, destDir, prefix s
 	return nil
 }
 
-// isDestRelative returns true if dest is pointing outside the base directory,
-// false otherwise.
-func isDestRelative(base, dest string) bool {
-	relative, err := filepath.Rel(base, dest)
-	if err != nil {
-		return false
-	}
-	return relative == "." || relative == stripPathShortcuts(relative)
-}
-
-func getPrefix(file string) string {
-	// tar strips the leading '/' if it's there, so we will too
-	return strings.TrimLeft(file, "/")
-}
-
 func (o *CopyOptions) execute(options *exec.ExecOptions) error {
 	if len(options.Namespace) == 0 {
 		options.Namespace = o.Namespace

staging/src/k8s.io/kubectl/pkg/cmd/cp/cp_test.go

@@ -24,7 +24,6 @@ import (
 	"io/ioutil"
 	"net/http"
 	"os"
-	"path"
 	"path/filepath"
 	"reflect"
 	"strings"
@@ -105,30 +104,42 @@ func TestExtractFileSpec(t *testing.T) {
 		if spec.PodNamespace != test.expectedNamespace {
 			t.Errorf("expected: %s, saw: %s", test.expectedNamespace, spec.PodNamespace)
 		}
-		if spec.File != test.expectedFile {
-			t.Errorf("expected: %s, saw: %s", test.expectedFile, spec.File)
+		specFile := ""
+		if spec.File != nil {
+			specFile = spec.File.String()
+		}
+		if specFile != test.expectedFile {
+			t.Errorf("expected: %s, saw: %s", test.expectedFile, specFile)
 		}
 	}
 }
 
 func TestGetPrefix(t *testing.T) {
+	remoteSeparator := '/'
+	osSeparator := os.PathSeparator
 	tests := []struct {
 		input    string
 		expected string
 	}{
 		{
-			input:    "/foo/bar",
-			expected: "foo/bar",
+			input:    "%[1]cfoo%[1]cbar",
+			expected: "foo%[1]cbar",
 		},
 		{
-			input:    "foo/bar",
-			expected: "foo/bar",
+			input:    "foo%[1]cbar",
+			expected: "foo%[1]cbar",
 		},
 	}
 	for _, test := range tests {
-		out := getPrefix(test.input)
-		if out != test.expected {
-			t.Errorf("expected: %s, saw: %s", test.expected, out)
+		outRemote := newRemotePath(fmt.Sprintf(test.input, remoteSeparator)).StripSlashes()
+		expectedRemote := fmt.Sprintf(test.expected, remoteSeparator)
+		if outRemote.String() != expectedRemote {
+			t.Errorf("remote expected: %s, saw: %s", expectedRemote, outRemote.String())
+		}
+		outLocal := newLocalPath(fmt.Sprintf(test.input, osSeparator)).StripSlashes()
+		expectedLocal := fmt.Sprintf(test.expected, osSeparator)
+		if outLocal.String() != expectedLocal {
+			t.Errorf("local expected: %s, saw: %s", expectedLocal, outLocal.String())
 		}
 	}
 }
@@ -144,36 +155,71 @@ func TestStripPathShortcuts(t *testing.T) {
 			input:    "../foo/bar",
 			expected: "foo/bar",
 		},
+		{
+			name:     "test single path shortcut prefix",
+			input:    `..\foo\bar`,
+			expected: "foo/bar",
+		},
 		{
 			name:     "test multiple path shortcuts",
 			input:    "../../foo/bar",
 			expected: "foo/bar",
 		},
+		{
+			name:     "test multiple path shortcuts",
+			input:    `..\..\foo\bar`,
+			expected: "foo/bar",
+		},
 		{
 			name:     "test multiple path shortcuts with absolute path",
 			input:    "/tmp/one/two/../../foo/bar",
 			expected: "tmp/foo/bar",
 		},
+		{
+			name:     "test multiple path shortcuts with absolute path",
+			input:    `\tmp\one\two\..\..\foo\bar`,
+			expected: "tmp/foo/bar",
+		},
 		{
 			name:     "test multiple path shortcuts with no named directory",
 			input:    "../../",
 			expected: "",
 		},
+		{
+			name:     "test multiple path shortcuts with no named directory",
+			input:    `..\..\`,
+			expected: "",
+		},
 		{
 			name:     "test multiple path shortcuts with no named directory and no trailing slash",
 			input:    "../..",
 			expected: "",
 		},
+		{
+			name:     "test multiple path shortcuts with no named directory and no trailing slash",
+			input:    `..\..`,
+			expected: "",
+		},
 		{
 			name:     "test multiple path shortcuts with absolute path and filename containing leading dots",
 			input:    "/tmp/one/two/../../foo/..bar",
 			expected: "tmp/foo/..bar",
 		},
+		{
+			name:     "test multiple path shortcuts with absolute path and filename containing leading dots",
+			input:    `\tmp\one\two\..\..\foo\..bar`,
+			expected: "tmp/foo/..bar",
+		},
 		{
 			name:     "test multiple path shortcuts with no named directory and filename containing leading dots",
 			input:    "../...foo",
 			expected: "...foo",
 		},
+		{
+			name:     "test multiple path shortcuts with no named directory and filename containing leading dots",
+			input:    `..\...foo`,
+			expected: "...foo",
+		},
 		{
 			name:     "test filename containing leading dots",
 			input:    "...foo",
@@ -184,12 +230,17 @@ func TestStripPathShortcuts(t *testing.T) {
 			input:    "/",
 			expected: "",
 		},
+		{
+			name:     "test root directory",
+			input:    `\`,
+			expected: "",
+		},
 	}
 
-	for _, test := range tests {
-		out := stripPathShortcuts(test.input)
-		if out != test.expected {
-			t.Errorf("expected: %s, saw: %s", test.expected, out)
+	for i, test := range tests {
+		out := newRemotePath(test.input).StripShortcuts()
+		if out.String() != test.expected {
+			t.Errorf("expected[%d]: %s, saw: %s", i, test.expected, out)
 		}
 	}
 }
@@ -243,7 +294,7 @@ func TestIsDestRelative(t *testing.T) {
 
 	for i, test := range tests {
 		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
-			if test.relative != isDestRelative(test.base, test.dest) {
+			if test.relative != isRelative(newLocalPath(test.base), newLocalPath(test.dest)) {
 				t.Errorf("unexpected result for: base %q, dest %q", test.base, test.dest)
 			}
 		})
@@ -260,12 +311,15 @@ func checkErr(t *testing.T, err error) {
 func TestTarUntar(t *testing.T) {
 	dir, err := ioutil.TempDir("", "input")
 	checkErr(t, err)
+	dir = dir + "/"
+
 	dir2, err := ioutil.TempDir("", "output")
 	checkErr(t, err)
+	dir2 = dir2 + "/"
+
 	dir3, err := ioutil.TempDir("", "dir")
 	checkErr(t, err)
 
-	dir = dir + "/"
 	defer func() {
 		os.RemoveAll(dir)
 		os.RemoveAll(dir2)
@@ -290,7 +344,7 @@ func TestTarUntar(t *testing.T) {
 			fileType: RegularFile,
 		},
 		{
-			name:     "some/other/directory/",
+			name:     "some/other/directory",
 			data:     "with more data here",
 			fileType: RegularFile,
 		},
@@ -307,13 +361,13 @@ func TestTarUntar(t *testing.T) {
 		},
 		{
 			name:     "relative_to_dest",
-			data:     path.Join(dir2, "foo"),
+			data:     dir2 + "/foo",
 			omitted:  true,
 			fileType: SymLink,
 		},
 		{
 			name:     "tricky_relative",
-			data:     path.Join(dir3, "xyz"),
+			data:     dir3 + "/xyz",
 			omitted:  true,
 			fileType: SymLink,
 		},
@@ -332,20 +386,20 @@ func TestTarUntar(t *testing.T) {
 	}
 
 	for _, file := range files {
-		filepath := path.Join(dir, file.name)
-		if err := os.MkdirAll(path.Dir(filepath), 0755); err != nil {
+		completePath := dir + file.name
+		if err := os.MkdirAll(filepath.Dir(completePath), 0755); err != nil {
 			t.Fatalf("unexpected error: %v", err)
 		}
 		if file.fileType == RegularFile {
-			createTmpFile(t, filepath, file.data)
+			createTmpFile(t, completePath, file.data)
 		} else if file.fileType == SymLink {
-			err := os.Symlink(file.data, filepath)
+			err := os.Symlink(file.data, completePath)
 			if err != nil {
 				t.Fatalf("unexpected error: %v", err)
 			}
 		} else if file.fileType == RegexFile {
 			for _, fileName := range file.nameList {
-				createTmpFile(t, path.Join(dir, fileName), file.data)
+				createTmpFile(t, dir+fileName, file.data)
 			}
 		} else {
 			t.Fatalf("unexpected file type: %v", file)
@@ -355,18 +409,18 @@ func TestTarUntar(t *testing.T) {
 	opts := NewCopyOptions(genericclioptions.NewTestIOStreamsDiscard())
 
 	writer := &bytes.Buffer{}
-	if err := makeTar(dir, dir, writer); err != nil {
+	if err := makeTar(newLocalPath(dir), newRemotePath(dir), writer); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
 
 	reader := bytes.NewBuffer(writer.Bytes())
-	if err := opts.untarAll(fileSpec{}, reader, dir2, ""); err != nil {
+	if err := opts.untarAll("", "", "", remotePath{}, newLocalPath(dir2), reader); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
 
 	for _, file := range files {
-		absPath := filepath.Join(dir2, strings.TrimPrefix(dir, os.TempDir()))
-		filePath := filepath.Join(absPath, file.name)
+		absPath := dir2 + strings.TrimPrefix(dir, os.TempDir())
+		filePath := absPath + file.name
 
 		if file.fileType == RegularFile {
 			cmpFileData(t, filePath, file.data)
@@ -387,7 +441,7 @@ func TestTarUntar(t *testing.T) {
 			}
 		} else if file.fileType == RegexFile {
 			for _, fileName := range file.nameList {
-				cmpFileData(t, path.Join(dir, fileName), file.data)
+				cmpFileData(t, dir+fileName, file.data)
 			}
 		} else {
 			t.Fatalf("unexpected file type: %v", file)
@@ -398,30 +452,31 @@ func TestTarUntar(t *testing.T) {
 func TestTarUntarWrongPrefix(t *testing.T) {
 	dir, err := ioutil.TempDir("", "input")
 	checkErr(t, err)
+	dir = dir + "/"
+
 	dir2, err := ioutil.TempDir("", "output")
 	checkErr(t, err)
 
-	dir = dir + "/"
 	defer func() {
 		os.RemoveAll(dir)
 		os.RemoveAll(dir2)
 	}()
 
-	filepath := path.Join(dir, "foo")
-	if err := os.MkdirAll(path.Dir(filepath), 0755); err != nil {
+	completePath := dir + "foo"
+	if err := os.MkdirAll(filepath.Dir(completePath), 0755); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	createTmpFile(t, filepath, "sample data")
+	createTmpFile(t, completePath, "sample data")
 
 	opts := NewCopyOptions(genericclioptions.NewTestIOStreamsDiscard())
 
 	writer := &bytes.Buffer{}
-	if err := makeTar(dir, dir, writer); err != nil {
+	if err := makeTar(newLocalPath(dir), newRemotePath(dir), writer); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
 
 	reader := bytes.NewBuffer(writer.Bytes())
-	err = opts.untarAll(fileSpec{}, reader, dir2, "verylongprefix-showing-the-tar-was-tempered-with")
+	err = opts.untarAll("", "", "verylongprefix-showing-the-tar-was-tempered-with", remotePath{}, newLocalPath(dir2), reader)
 	if err == nil || !strings.Contains(err.Error(), "tar contents corrupted") {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -467,17 +522,17 @@ func TestTarDestinationName(t *testing.T) {
 
 	// ensure files exist on disk
 	for _, file := range files {
-		filepath := path.Join(dir, file.name)
-		if err := os.MkdirAll(path.Dir(filepath), 0755); err != nil {
+		completePath := dir + "/" + file.name
+		if err := os.MkdirAll(filepath.Dir(completePath), 0755); err != nil {
 			t.Errorf("unexpected error: %v", err)
 			t.FailNow()
 		}
-		createTmpFile(t, filepath, file.data)
+		createTmpFile(t, completePath, file.data)
 	}
 
 	reader, writer := io.Pipe()
 	go func() {
-		if err := makeTar(dir, dir2, writer); err != nil {
+		if err := makeTar(newLocalPath(dir), newRemotePath(dir2), writer); err != nil {
 			t.Errorf("unexpected error: %v", err)
 		}
 	}()
@@ -492,8 +547,8 @@ func TestTarDestinationName(t *testing.T) {
 			t.FailNow()
 		}
 
-		if !strings.HasPrefix(hdr.Name, path.Base(dir2)) {
-			t.Errorf("expected %q as destination filename prefix, saw: %q", path.Base(dir2), hdr.Name)
+		if !strings.HasPrefix(hdr.Name, filepath.Base(dir2)) {
+			t.Errorf("expected %q as destination filename prefix, saw: %q", filepath.Base(dir2), hdr.Name)
 		}
 	}
 }
@@ -536,13 +591,13 @@ func TestBadTar(t *testing.T) {
 	}
 
 	opts := NewCopyOptions(genericclioptions.NewTestIOStreamsDiscard())
-	if err := opts.untarAll(fileSpec{}, &buf, dir, "/prefix"); err != nil {
+	if err := opts.untarAll("", "", "/prefix", remotePath{}, newLocalPath(dir), &buf); err != nil {
 		t.Errorf("unexpected error: %v ", err)
 		t.FailNow()
 	}
 
 	for _, file := range files {
-		_, err := os.Stat(path.Join(dir, path.Clean(file.name[len("/prefix"):])))
+		_, err := os.Stat(dir + filepath.Clean(file.name[len("/prefix"):]))
 		if err != nil {
 			t.Errorf("Error finding file: %v", err)
 		}
@@ -596,7 +651,7 @@ func TestCopyToPod(t *testing.T) {
 			expectedErr: true,
 		},
 		"copy unexisting file": {
-			src:         path.Join(srcFile, "nope"),
+			src:         filepath.Join(srcFile, "nope"),
 			dest:        "/tmp",
 			expectedErr: true,
 		},
@@ -604,17 +659,9 @@ func TestCopyToPod(t *testing.T) {
 
 	for name, test := range tests {
 		opts := NewCopyOptions(ioStreams)
-		src := fileSpec{
-			File: test.src,
-		}
-		dest := fileSpec{
-			PodNamespace: "pod-ns",
-			PodName:      "pod-name",
-			File:         test.dest,
-		}
 		opts.Complete(tf, cmd)
 		t.Run(name, func(t *testing.T) {
-			err = opts.copyToPod(src, dest, &kexec.ExecOptions{})
+			err = opts.Run([]string{test.src, fmt.Sprintf("pod-ns/pod-name:%s", test.dest)})
 			//If error is NotFound error , it indicates that the
 			//request has been sent correctly.
 			//Treat this as no error.
@@ -669,12 +716,12 @@ func TestCopyToPodNoPreserve(t *testing.T) {
 	}
 	opts := NewCopyOptions(ioStreams)
 	src := fileSpec{
-		File: srcFile,
+		File: newLocalPath(srcFile),
 	}
 	dest := fileSpec{
 		PodNamespace: "pod-ns",
 		PodName:      "pod-name",
-		File:         "foo",
+		File:         newRemotePath("foo"),
 	}
 	opts.Complete(tf, cmd)
 
@@ -728,7 +775,7 @@ func TestUntar(t *testing.T) {
 	defer os.RemoveAll(testdir)
 	t.Logf("Test base: %s", testdir)
 
-	basedir := filepath.Join(testdir, "base")
+	basedir := testdir + "/" + "base"
 
 	type file struct {
 		path       string
@@ -737,29 +784,35 @@ func TestUntar(t *testing.T) {
 	}
 	files := []file{{
 		// Absolute file within dest
-		path:     filepath.Join(basedir, "abs"),
-		expected: filepath.Join(basedir, basedir, "abs"),
+		path:     basedir + "/" + "abs",
+		expected: basedir + basedir + "/" + "abs",
 	}, { // Absolute file outside dest
-		path:     filepath.Join(testdir, "abs-out"),
-		expected: filepath.Join(basedir, testdir, "abs-out"),
+		path:     testdir + "/" + "abs-out",
+		expected: basedir + testdir + "/" + "abs-out",
 	}, { // Absolute nested file within dest
-		path:     filepath.Join(basedir, "nested/nest-abs"),
-		expected: filepath.Join(basedir, basedir, "nested/nest-abs"),
+		path:     basedir + "/" + "nested/nest-abs",
+		expected: basedir + basedir + "/" + "nested/nest-abs",
 	}, { // Absolute nested file outside dest
-		path:     filepath.Join(basedir, "nested/../../nest-abs-out"),
-		expected: filepath.Join(basedir, testdir, "nest-abs-out"),
+		path:     basedir + "/" + "nested/../../nest-abs-out",
+		expected: basedir + testdir + "/" + "nest-abs-out",
 	}, { // Relative file inside dest
 		path:     "relative",
-		expected: filepath.Join(basedir, "relative"),
+		expected: basedir + "/" + "relative",
 	}, { // Relative file outside dest
 		path:     "../unrelative",
 		expected: "",
+	}, { // Relative file outside dest (windows)
+		path:     `..\unrelative-windows`,
+		expected: "",
 	}, { // Nested relative file inside dest
 		path:     "nested/nest-rel",
-		expected: filepath.Join(basedir, "nested/nest-rel"),
+		expected: basedir + "/" + "nested/nest-rel",
 	}, { // Nested relative file outside dest
 		path:     "nested/../../nest-unrelative",
 		expected: "",
+	}, { // Nested relative file outside dest (windows)
+		path:     `nested\..\..\nest-unrelative`,
+		expected: "",
 	}}
 
 	links := []file{}
@@ -770,15 +823,15 @@ func TestUntar(t *testing.T) {
 			expected:   "",
 		}, file{
 			path:       f.path + "-innerlink-abs",
-			linkTarget: filepath.Join(basedir, "link-target"),
+			linkTarget: basedir + "/" + "link-target",
 			expected:   "",
 		}, file{
 			path:       f.path + "-backlink",
-			linkTarget: filepath.Join("..", "link-target"),
+			linkTarget: ".." + "/" + "link-target",
 			expected:   "",
 		}, file{
 			path:       f.path + "-outerlink-abs",
-			linkTarget: filepath.Join(testdir, "link-target"),
+			linkTarget: testdir + "/" + "link-target",
 			expected:   "",
 		})
 
@@ -787,7 +840,7 @@ func TestUntar(t *testing.T) {
 			outerlink, _ := filepath.Rel(f.expected, testdir)
 			links = append(links, file{
 				path:       f.path + "outerlink",
-				linkTarget: filepath.Join(outerlink, "link-target"),
+				linkTarget: outerlink + "/" + "link-target",
 				expected:   "",
 			})
 		}
@@ -803,7 +856,7 @@ func TestUntar(t *testing.T) {
 		},
 		file{
 			path:     "nested/again/back-link/../../../back-link-file",
-			expected: filepath.Join(basedir, "back-link-file"),
+			expected: basedir + "/" + "back-link-file",
 		})
 
 	// Test chaining back-tick symlinks.
@@ -859,7 +912,7 @@ func TestUntar(t *testing.T) {
 	output := (*testWriter)(t)
 	opts := NewCopyOptions(genericclioptions.IOStreams{In: &bytes.Buffer{}, Out: output, ErrOut: output})
 
-	require.NoError(t, opts.untarAll(fileSpec{}, buf, filepath.Join(basedir), ""))
+	require.NoError(t, opts.untarAll("", "", "", remotePath{}, newLocalPath(basedir), buf))
 
 	filepath.Walk(testdir, func(path string, info os.FileInfo, err error) error {
 		if err != nil {
@@ -887,7 +940,7 @@ func TestUntar_SingleFile(t *testing.T) {
 	require.NoError(t, err)
 	defer os.RemoveAll(testdir)
 
-	dest := filepath.Join(testdir, "target")
+	dest := testdir + "/" + "target"
 
 	buf := &bytes.Buffer{}
 	tw := tar.NewWriter(buf)
@@ -910,7 +963,7 @@ func TestUntar_SingleFile(t *testing.T) {
 	output := (*testWriter)(t)
 	opts := NewCopyOptions(genericclioptions.IOStreams{In: &bytes.Buffer{}, Out: output, ErrOut: output})
 
-	require.NoError(t, opts.untarAll(fileSpec{}, buf, filepath.Join(dest), srcName))
+	require.NoError(t, opts.untarAll("", "", srcName, remotePath{}, newLocalPath(dest), buf))
 	cmpFileData(t, dest, content)
 }
 

staging/src/k8s.io/kubectl/pkg/cmd/cp/filespec.go

@@ -0,0 +1,161 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package cp
+
+import (
+	"path"
+	"path/filepath"
+	"strings"
+)
+
+type fileSpec struct {
+	PodName      string
+	PodNamespace string
+	File         pathSpec
+}
+
+type pathSpec interface {
+	String() string
+}
+
+// localPath represents a client-native path, which will differ based
+// on the client OS, its methods will use path/filepath package which
+// is OS dependant
+type localPath struct {
+	file string
+}
+
+func newLocalPath(fileName string) localPath {
+	file := stripTrailingSlash(fileName)
+	return localPath{file: file}
+}
+
+func (p localPath) String() string {
+	return p.file
+}
+
+func (p localPath) Dir() localPath {
+	return newLocalPath(filepath.Dir(p.file))
+}
+
+func (p localPath) Base() localPath {
+	return newLocalPath(filepath.Base(p.file))
+}
+
+func (p localPath) Clean() localPath {
+	return newLocalPath(filepath.Clean(p.file))
+}
+
+func (p localPath) Join(elem pathSpec) localPath {
+	return newLocalPath(filepath.Join(p.file, elem.String()))
+}
+
+func (p localPath) Glob() (matches []string, err error) {
+	return filepath.Glob(p.file)
+}
+
+func (p localPath) StripSlashes() localPath {
+	return newLocalPath(stripLeadingSlash(p.file))
+}
+
+func isRelative(base, target localPath) bool {
+	relative, err := filepath.Rel(base.String(), target.String())
+	if err != nil {
+		return false
+	}
+	return relative == "." || relative == stripPathShortcuts(relative)
+}
+
+// remotePath represents always UNIX path, its methods will use path
+// package which is always using `/`
+type remotePath struct {
+	file string
+}
+
+func newRemotePath(fileName string) remotePath {
+	// we assume remote file is a linux container but we need to convert
+	// windows path separators to unix style for consistent processing
+	file := strings.ReplaceAll(stripTrailingSlash(fileName), `\`, "/")
+	return remotePath{file: file}
+}
+
+func (p remotePath) String() string {
+	return p.file
+}
+
+func (p remotePath) Dir() remotePath {
+	return newRemotePath(path.Dir(p.file))
+}
+
+func (p remotePath) Base() remotePath {
+	return newRemotePath(path.Base(p.file))
+}
+
+func (p remotePath) Clean() remotePath {
+	return newRemotePath(path.Clean(p.file))
+}
+
+func (p remotePath) Join(elem pathSpec) remotePath {
+	return newRemotePath(path.Join(p.file, elem.String()))
+}
+
+func (p remotePath) StripShortcuts() remotePath {
+	p = p.Clean()
+	return newRemotePath(stripPathShortcuts(p.file))
+}
+
+func (p remotePath) StripSlashes() remotePath {
+	return newRemotePath(stripLeadingSlash(p.file))
+}
+
+// strips trailing slash (if any) both unix and windows style
+func stripTrailingSlash(file string) string {
+	if len(file) == 0 {
+		return file
+	}
+	if file != "/" && strings.HasSuffix(string(file[len(file)-1]), "/") {
+		return file[:len(file)-1]
+	}
+	return file
+}
+
+func stripLeadingSlash(file string) string {
+	// tar strips the leading '/' and '\' if it's there, so we will too
+	return strings.TrimLeft(file, `/\`)
+}
+
+// stripPathShortcuts removes any leading or trailing "../" from a given path
+func stripPathShortcuts(p string) string {
+	newPath := p
+	trimmed := strings.TrimPrefix(newPath, "../")
+
+	for trimmed != newPath {
+		newPath = trimmed
+		trimmed = strings.TrimPrefix(newPath, "../")
+	}
+
+	// trim leftover {".", ".."}
+	if newPath == "." || newPath == ".." {
+		newPath = ""
+	}
+
+	if len(newPath) > 0 && string(newPath[0]) == "/" {
+		return newPath[1:]
+	}
+
+	return newPath
+}