diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 04c1683073851..c1b0a46256ae7 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -1536,7 +1536,7 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { commitMessage.WriteString(" <<" + src.Path) data := src.Data - f, err := system.CheckSystemDriveAndRemoveDriveLetter(src.Path, d.platform.OS) + f, err := system.CheckSystemDriveAndRemoveDriveLetter(src.Path, d.platform.OS, false) if err != nil { return errors.Wrap(err, "removing drive letter") } @@ -1804,13 +1804,15 @@ func pathRelativeToWorkingDir(s llb.State, p string, platform ocispecs.Platform) return "", err } - p, err = system.CheckSystemDriveAndRemoveDriveLetter(p, platform.OS) + keepSlash := true + + p, err = system.CheckSystemDriveAndRemoveDriveLetter(p, platform.OS, keepSlash) if err != nil { return "", errors.Wrap(err, "removing drive letter") } if system.IsAbs(p, platform.OS) { - return system.NormalizePath("/", p, platform.OS, true) + return system.NormalizePath("/", p, platform.OS, keepSlash) } // add slashes for "" and "." paths @@ -1818,7 +1820,7 @@ func pathRelativeToWorkingDir(s llb.State, p string, platform ocispecs.Platform) if p == "." || p == "" { p = "./" } - return system.NormalizePath(dir, p, platform.OS, true) + return system.NormalizePath(dir, p, platform.OS, keepSlash) } func addEnv(env []string, k, v string) []string { diff --git a/frontend/dockerfile/dockerfile_test.go b/frontend/dockerfile/dockerfile_test.go index 2647826444371..7c67104f43944 100644 --- a/frontend/dockerfile/dockerfile_test.go +++ b/frontend/dockerfile/dockerfile_test.go @@ -155,6 +155,7 @@ var allTests = integration.TestFuncs( testNamedMultiplatformInputContext, testNamedFilteredContext, testEmptyDestDir, + testPreserveDestDirSlash, testCopyLinkDotDestDir, testCopyLinkEmptyDestDir, testCopyChownCreateDest, @@ -551,6 +552,42 @@ RUN cmd /V:on /C "set /p tfcontent= Fail -func CheckSystemDriveAndRemoveDriveLetter(path string, inputOS string) (string, error) { +func CheckSystemDriveAndRemoveDriveLetter(path string, inputOS string, keepSlash bool) (string, error) { if inputOS == "" { inputOS = "linux" } @@ -193,9 +181,10 @@ func CheckSystemDriveAndRemoveDriveLetter(path string, inputOS string) (string, } parts := strings.SplitN(path, ":", 2) + // Path does not have a drive letter. Just return it. if len(parts) < 2 { - return ToSlash(filepath.Clean(path), inputOS), nil + return ToSlash(cleanPath(path, inputOS, keepSlash), inputOS), nil } // We expect all paths to be in C: @@ -220,5 +209,31 @@ func CheckSystemDriveAndRemoveDriveLetter(path string, inputOS string) (string, // // We must return the second element of the split path, as is, without attempting to convert // it to an absolute path. We have no knowledge of the CWD; that is treated elsewhere. - return ToSlash(filepath.Clean(parts[1]), inputOS), nil + return ToSlash(cleanPath(parts[1], inputOS, keepSlash), inputOS), nil +} + +// An adaptation of filepath.Clean to allow an option to +// retain the trailing slash, on either of the platforms. +// Returns path with platform specific separators. +// See https://github.com/moby/buildkit/issues/5249 +func cleanPath(origPath, inputOS string, keepSlash bool) string { + if !keepSlash { + return filepath.Clean(origPath) + } + + // so as to handle cases like \\a\\b\\..\\c\\ + // on Linux, when inputOS is Windows + origPath = ToSlash(origPath, inputOS) + + cleanedPath := filepath.Clean(origPath) + // Windows supports both \\ and / as path separator. + hasTrailingSlash := strings.HasSuffix(origPath, "/") + if inputOS == "windows" { + hasTrailingSlash = hasTrailingSlash || strings.HasSuffix(origPath, "\\") + } + + if len(cleanedPath) > 1 && hasTrailingSlash { + return cleanedPath + "/" + } + return cleanedPath } diff --git a/util/system/path_test.go b/util/system/path_test.go index c8d97c72d5ed3..2563108238885 100644 --- a/util/system/path_test.go +++ b/util/system/path_test.go @@ -87,8 +87,9 @@ func TestNormalizeWorkdir(t *testing.T) { // TestCheckSystemDriveAndRemoveDriveLetter tests CheckSystemDriveAndRemoveDriveLetter func TestCheckSystemDriveAndRemoveDriveLetter(t *testing.T) { + keepSlash := false // Fails if not C drive. - _, err := CheckSystemDriveAndRemoveDriveLetter(`d:\`, "windows") + _, err := CheckSystemDriveAndRemoveDriveLetter(`d:\`, "windows", keepSlash) if err == nil || err.Error() != "The specified path is not on the system drive (C:)" { t.Fatalf("Expected error for d:") } @@ -96,7 +97,7 @@ func TestCheckSystemDriveAndRemoveDriveLetter(t *testing.T) { var path string // Single character is unchanged - if path, err = CheckSystemDriveAndRemoveDriveLetter("z", "windows"); err != nil { + if path, err = CheckSystemDriveAndRemoveDriveLetter("z", "windows", keepSlash); err != nil { t.Fatalf("Single character should pass") } if path != "z" { @@ -104,7 +105,7 @@ func TestCheckSystemDriveAndRemoveDriveLetter(t *testing.T) { } // Two characters without colon is unchanged - if path, err = CheckSystemDriveAndRemoveDriveLetter("AB", "windows"); err != nil { + if path, err = CheckSystemDriveAndRemoveDriveLetter("AB", "windows", keepSlash); err != nil { t.Fatalf("2 characters without colon should pass") } if path != "AB" { @@ -112,7 +113,7 @@ func TestCheckSystemDriveAndRemoveDriveLetter(t *testing.T) { } // Abs path without drive letter - if path, err = CheckSystemDriveAndRemoveDriveLetter(`\l`, "windows"); err != nil { + if path, err = CheckSystemDriveAndRemoveDriveLetter(`\l`, "windows", keepSlash); err != nil { t.Fatalf("abs path no drive letter should pass") } if path != `/l` { @@ -120,7 +121,7 @@ func TestCheckSystemDriveAndRemoveDriveLetter(t *testing.T) { } // Abs path without drive letter, linux style - if path, err = CheckSystemDriveAndRemoveDriveLetter(`/l`, "windows"); err != nil { + if path, err = CheckSystemDriveAndRemoveDriveLetter(`/l`, "windows", keepSlash); err != nil { t.Fatalf("abs path no drive letter linux style should pass") } if path != `/l` { @@ -128,7 +129,7 @@ func TestCheckSystemDriveAndRemoveDriveLetter(t *testing.T) { } // Drive-colon should be stripped - if path, err = CheckSystemDriveAndRemoveDriveLetter(`c:\`, "windows"); err != nil { + if path, err = CheckSystemDriveAndRemoveDriveLetter(`c:\`, "windows", keepSlash); err != nil { t.Fatalf("An absolute path should pass") } if path != `/` { @@ -136,7 +137,7 @@ func TestCheckSystemDriveAndRemoveDriveLetter(t *testing.T) { } // Verify with a linux-style path - if path, err = CheckSystemDriveAndRemoveDriveLetter(`c:/`, "windows"); err != nil { + if path, err = CheckSystemDriveAndRemoveDriveLetter(`c:/`, "windows", keepSlash); err != nil { t.Fatalf("An absolute path should pass") } if path != `/` { @@ -144,7 +145,7 @@ func TestCheckSystemDriveAndRemoveDriveLetter(t *testing.T) { } // Failure on c: - if path, err = CheckSystemDriveAndRemoveDriveLetter(`c:`, "windows"); err == nil { + if path, err = CheckSystemDriveAndRemoveDriveLetter(`c:`, "windows", keepSlash); err == nil { t.Fatalf("c: should fail") } if err.Error() != `No relative path specified in "c:"` { @@ -152,7 +153,7 @@ func TestCheckSystemDriveAndRemoveDriveLetter(t *testing.T) { } // Failure on d: - if path, err = CheckSystemDriveAndRemoveDriveLetter(`d:`, "windows"); err == nil { + if path, err = CheckSystemDriveAndRemoveDriveLetter(`d:`, "windows", keepSlash); err == nil { t.Fatalf("c: should fail") } if err.Error() != `No relative path specified in "d:"` { @@ -160,9 +161,45 @@ func TestCheckSystemDriveAndRemoveDriveLetter(t *testing.T) { } // UNC path should fail. - if _, err = CheckSystemDriveAndRemoveDriveLetter(`\\.\C$\test`, "windows"); err == nil { + if _, err = CheckSystemDriveAndRemoveDriveLetter(`\\.\C$\test`, "windows", keepSlash); err == nil { t.Fatalf("UNC path should fail") } + + // also testing for keepSlash = true + keepSlash = true + origPath := "\\a\\b\\..\\c\\" + if path, err = CheckSystemDriveAndRemoveDriveLetter(origPath, "windows", keepSlash); err != nil { + t.Fatalf("windows relative paths should be cleaned and should pass") + } + // When input OS is Windows, the path should be properly cleaned + if path != "/a/c/" { + t.Fatalf("Path was not cleaned successfully") + } + + if path, err = CheckSystemDriveAndRemoveDriveLetter(origPath, "windows", false); err != nil { + t.Fatalf("windows relative paths should be cleaned and should pass [keepSlash = false]") + } + // When input OS is Windows, the path should be properly cleaned + if path != "/a/c" { + t.Fatalf("Path was not cleaned successfully [keepSlash = false]") + } + + // windows-style relative paths on linux + if path, err = CheckSystemDriveAndRemoveDriveLetter(origPath, "linux", keepSlash); err != nil { + t.Fatalf("windows style relative paths should be considered a valid path element in linux and should pass") + } + // When input OS is Linux, this is a valid path element name. + if path != "\\a\\b\\..\\c\\" { + t.Fatalf("Path was not cleaned successfully") + } + + if path, err = CheckSystemDriveAndRemoveDriveLetter(origPath, "linux", false); err != nil { + t.Fatalf("windows style relative paths should be considered a valid path element in linux and should pass") + } + // When input OS is Linux, this is a valid path element name. + if path != "\\a\\b\\..\\c\\" { + t.Fatalf("Path was not cleaned successfully [keepSlash = false]") + } } // TestNormalizeWorkdir tests NormalizeWorkdir