From e05fc8ca351f384daebb55e9de921b6a62454603 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 26 Jul 2021 21:08:06 +0200 Subject: [PATCH] builder/dockerfile: make normalizeDest() platform-specific again Removing various bits that were added in https://github.com/moby/moby/commit/7a7357dae1bcccb17e9b2d4c7c8f5c025fce56ca Signed-off-by: Sebastiaan van Stijn --- builder/dockerfile/copy_unix.go | 18 ++++++ builder/dockerfile/copy_windows.go | 46 ++++++++++++++ builder/dockerfile/internals.go | 63 +------------------- builder/dockerfile/internals_windows_test.go | 2 +- 4 files changed, 66 insertions(+), 63 deletions(-) diff --git a/builder/dockerfile/copy_unix.go b/builder/dockerfile/copy_unix.go index bccc5b9ef7..6cc85e53f3 100644 --- a/builder/dockerfile/copy_unix.go +++ b/builder/dockerfile/copy_unix.go @@ -4,7 +4,9 @@ package dockerfile // import "github.com/docker/docker/builder/dockerfile" import ( "os" + "path" "path/filepath" + "strings" "github.com/docker/docker/pkg/containerfs" "github.com/docker/docker/pkg/idtools" @@ -43,6 +45,22 @@ func fixPermissions(source, destination string, identity idtools.Identity, overr }) } +// 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)) + + if !path.IsAbs(requested) { + dest = path.Join("/", filepath.ToSlash(workingDir), dest) + // Make sure we preserve any trailing slash + if endsInSlash { + dest += "/" + } + } + return dest, nil +} + func containsWildcards(name string) bool { for i := 0; i < len(name); i++ { ch := name[i] diff --git a/builder/dockerfile/copy_windows.go b/builder/dockerfile/copy_windows.go index 247a6663b8..1a3a488516 100644 --- a/builder/dockerfile/copy_windows.go +++ b/builder/dockerfile/copy_windows.go @@ -79,6 +79,52 @@ func fixPermissionsWindows(source, destination, SID string) error { return windows.SetNamedSecurityInfo(destination, windows.SE_FILE_OBJECT, windows.OWNER_SECURITY_INFORMATION|windows.DACL_SECURITY_INFORMATION, sid, nil, dacl, nil) } +// 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 +} + func containsWildcards(name string) bool { for i := 0; i < len(name); i++ { ch := name[i] diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go index fec1709b9e..0bc9d0e9ef 100644 --- a/builder/dockerfile/internals.go +++ b/builder/dockerfile/internals.go @@ -8,9 +8,6 @@ import ( "encoding/hex" "fmt" "io" - "os" - "path" - "path/filepath" "strings" "github.com/docker/docker/api/types" @@ -23,7 +20,6 @@ import ( "github.com/docker/docker/pkg/containerfs" "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/stringid" - "github.com/docker/docker/pkg/system" "github.com/docker/go-connections/nat" specs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" @@ -224,7 +220,7 @@ func (b *Builder) performCopy(req dispatchRequest, inst copyInstruction) error { func createDestInfo(workingDir string, inst copyInstruction, rwLayer builder.RWLayer, platform string) (copyInfo, error) { // Twiddle the destination when it's a relative path - meaning, make it // relative to the WORKINGDIR - dest, err := normalizeDest(workingDir, inst.dest, platform) + dest, err := normalizeDest(workingDir, inst.dest) if err != nil { return copyInfo{}, errors.Wrapf(err, "invalid %s", inst.cmdName) } @@ -232,63 +228,6 @@ func createDestInfo(workingDir string, inst copyInstruction, rwLayer builder.RWL return copyInfo{root: rwLayer.Root(), path: dest}, nil } -// normalizeDest normalises the destination of a COPY/ADD command in a -// platform semantically consistent way. -func normalizeDest(workingDir, requested string, platform string) (string, error) { - dest := filepath.FromSlash(requested) - endsInSlash := strings.HasSuffix(dest, string(os.PathSeparator)) - - if platform != "windows" { - if !path.IsAbs(requested) { - dest = path.Join("/", filepath.ToSlash(workingDir), dest) - // Make sure we preserve any trailing slash - if endsInSlash { - dest += "/" - } - } - return dest, nil - } - - // 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 -} - // For backwards compat, if there's just one info then use it as the // cache look-up string, otherwise hash 'em all into one func getSourceHashFromInfos(infos []copyInfo) string { diff --git a/builder/dockerfile/internals_windows_test.go b/builder/dockerfile/internals_windows_test.go index 1694ee8ac0..a05e1c6899 100644 --- a/builder/dockerfile/internals_windows_test.go +++ b/builder/dockerfile/internals_windows_test.go @@ -40,7 +40,7 @@ func TestNormalizeDest(t *testing.T) { } for _, testcase := range tests { msg := fmt.Sprintf("Input: %s, %s", testcase.current, testcase.requested) - actual, err := normalizeDest(testcase.current, testcase.requested, "windows") + actual, err := normalizeDest(testcase.current, testcase.requested) if testcase.etext == "" { if !assert.Check(t, err, msg) { continue