WCOW: trailing / ignored during COPY if destination dir not present

[profnandaa]

The trailing / is ignored and the destination directory is instead taken as a file. For example COPY file.txt dir/, instead copies file.txt into dir (file), when it should be dir/file.txt

Minimal repro dockerfile:

echo hello > test1.txt

FROM mcr.microsoft.com/windows/nanoserver:ltsc2022
COPY test1.txt /sample/
RUN type \sample\test1.txt

Run with buildctl:

...
#6 [2/3] COPY test1.txt /sample/
#6 DONE 0.3s

#7 [3/3] RUN type sampletest1.txt
#7 1.813 The directory name is invalid.
#7 ERROR: process "cmd /S /C type \\sample\\test1.txt" did not complete successfully: exit code: 1
------
 > [3/3] RUN type sampletest1.txt:
1.813 The directory name is invalid.
------
Dockerfile:3
--------------------
   1 |     FROM mcr.microsoft.com/windows/nanoserver:ltsc2022
   2 |     COPY test1.txt /sample/
   3 | >>> RUN type \sample\test1.txt
   4 |
--------------------
error: failed to solve: process "cmd /S /C type \\sample\\test1.txt" did not complete successfully: exit code: 1

The file is copied to C:\sample instead, which was to be a directory.

While it builds successfully with Docker (on Windows) or with equivalent dockerfile on Linux (with buildctl):

Step 3/3 : RUN type \sample\test1.txt
 ---> Running in d197cfe7b42d
hello
 ---> Removed intermediate container d197cfe7b42d
 ---> 14acdf8e60d2

If you add a step to create the destination directory, it runs successfully:

FROM mcr.microsoft.com/windows/nanoserver:ltsc2022
RUN mkdir sample # <-- added
COPY test1.txt /sample/
RUN type \sample\test1.txt

#8 [4/4] RUN type sampletest1.txt
#8 1.230 hello
#8 DONE 1.5s

Also this fails:

FROM mcr.microsoft.com/windows/nanoserver:ltsc2022
COPY test1.txt /another/sample/
RUN type \another\sample\test1.txt

The file is copied to C:\another\sample instead.

[profnandaa]

Found the place / is being stripped off, at filepath.Clean().

// ok, already dest has `/` stripped off.
b buildkit\frontend\dockerfile\dockerfile2llb\convert.go:1344

  1343: func dispatchCopy(d *dispatchState, cfg copyConfig) error {
=>1344:         dest, err := pathRelativeToWorkingDir(d.state, cfg.params.DestPath, *d.platform)
  1345:         if err != nil {

// step
=>1787:         p, err = system.CheckSystemDriveAndRemoveDriveLetter(p, platform.OS)
  1788:         if err != nil {
  1789:                 return "", errors.Wrap(err, "removing drive letter")
  1790:         }
  1791:
  1792:         if system.IsAbs(p, platform.OS) {
(dlv) p p
"/sample/"

// `/` being stripped out at L1787
  1787:         p, err = system.CheckSystemDriveAndRemoveDriveLetter(p, platform.OS)
=>1788:         if err != nil {
  1789:                 return "", errors.Wrap(err, "removing drive letter")
  1790:         }
  1791:
  1792:         if system.IsAbs(p, platform.OS) {
  1793:                 return system.NormalizePath("/", p, platform.OS, true)
(dlv) p p
"/sample"

// specifically:
// step at 1787
   197:         if len(parts) < 2 {
=> 198:                 return ToSlash(filepath.Clean(path), inputOS), nil
   199:         }
(dlv) p parts
[]string len: 1, cap: 2, [
        "/sample/",
]
// step at 198
    88: func ToSlash(inputPath, inputOS string) string {
=>  89:         if inputOS != "windows" {
    90:                 return inputPath
    91:         }
    92:         return strings.ReplaceAll(inputPath, "\\", "/")
    93: }
    94:
(dlv) p inputPath
"\\sample"
// `\` stripped out in filepath.Clean(path)

Worked on a fix to restore it afterwards, PR following. /cc. @tonistiigi

[thaJeztah]

Ah, yes, I recall the classic builder has some special handling for these;
https://github.com/moby/moby/blob/34182f6202507a1974e19b5140e2e36dc99892ff/builder/dockerfile/copy_unix.go#L46-L60

And the windows implementation for the above;
https://github.com/moby/moby/blob/34182f6202507a1974e19b5140e2e36dc99892ff/builder/dockerfile/copy_windows.go#L82-L126

// normalizeDest normalises the destination of a COPY/ADD command in a
// platform semantically consistent way.
func normalizeDest(workingDir, requested string) (string, error) {
	dest := filepath.FromSlash(requested)
	endsInSlash := strings.HasSuffix(dest, string(os.PathSeparator))

	// We are guaranteed that the working directory is already consistent,
	// However, Windows also has, for now, the limitation that ADD/COPY can
	// only be done to the system drive, not any drives that might be present
	// as a result of a bind mount.
	//
	// So... if the path requested is Linux-style absolute (/foo or \\foo),
	// we assume it is the system drive. If it is a Windows-style absolute
	// (DRIVE:\\foo), error if DRIVE is not C. And finally, ensure we
	// strip any configured working directories drive letter so that it
	// can be subsequently legitimately converted to a Windows volume-style
	// pathname.

	// Not a typo - filepath.IsAbs, not system.IsAbs on this next check as
	// we only want to validate where the DriveColon part has been supplied.
	if filepath.IsAbs(dest) {
		if strings.ToUpper(string(dest[0])) != "C" {
			return "", fmt.Errorf("Windows does not support destinations not on the system drive (C:)")
		}
		dest = dest[2:] // Strip the drive letter
	}

	// Cannot handle relative where WorkingDir is not the system drive.
	if len(workingDir) > 0 {
		if ((len(workingDir) > 1) && !system.IsAbs(workingDir[2:])) || (len(workingDir) == 1) {
			return "", fmt.Errorf("Current WorkingDir %s is not platform consistent", workingDir)
		}
		if !system.IsAbs(dest) {
			if string(workingDir[0]) != "C" {
				return "", fmt.Errorf("Windows does not support relative paths when WORKDIR is not the system drive")
			}
			dest = filepath.Join(string(os.PathSeparator), workingDir[2:], dest)
			// Make sure we preserve any trailing slash
			if endsInSlash {
				dest += string(os.PathSeparator)
			}
		}
	}
	return dest, nil
}

[profnandaa]

@thaJeztah -- this is really helpful, been wanting to know how the classic builder handles this. Thanks for sharing!
Will appreciate your review on my fix #5317