Repository: otiai10/copy
Pull request #70 (2021-12-08 08:41:06)

Feature/optional temporary perm change
Fix #69 


Patch:
diff --git a/.gitignore b/.gitignore
index d65ce17..3b65243 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,4 +1,5 @@
 test/data.copy
+test/owned-by-root
 coverage.txt
 vendor
 .vagrant
diff --git a/README.md b/README.md
index 2305fbf..105a592 100644
--- a/README.md
+++ b/README.md
@@ -34,9 +34,18 @@ type Options struct {
 	// Skip can specify which files should be skipped
 	Skip func(src string) (bool, error)
 
-	// AddPermission to every entry,
-	// NO MORE THAN 0777
-	AddPermission os.FileMode
+	// PermissionControl can control permission of
+	// every entry.
+	// When you want to add permission 0222, do like
+	//
+	//		PermissionControl = AddPermission(0222)
+	//
+	// or if you even don't want to touch permission,
+	//
+	//		PermissionControl = DoNothing
+	//
+	// By default, PermissionControl = PreservePermission
+	PermissionControl PermissionControlFunc
 
 	// Sync file after copy.
 	// Useful in case when file must be on the disk
diff --git a/copy.go b/copy.go
index ae04036..81e1ecc 100644
--- a/copy.go
+++ b/copy.go
@@ -8,13 +8,6 @@ import (
 	"time"
 )
 
-const (
-	// tmpPermissionForDirectory makes the destination directory writable,
-	// so that stuff can be copied recursively even if any original directory is NOT writable.
-	// See https://github.com/otiai10/copy/pull/9 for more information.
-	tmpPermissionForDirectory = os.FileMode(0755)
-)
-
 type timespec struct {
 	Mtime time.Time
 	Atime time.Time
@@ -27,7 +20,7 @@ func Copy(src, dest string, opt ...Options) error {
 	if err != nil {
 		return err
 	}
-	return switchboard(src, dest, info, assure(src, dest, opt...))
+	return switchboard(src, dest, info, assureOptions(src, dest, opt...))
 }
 
 // switchboard switches proper copy functions regarding file type, etc...
@@ -76,9 +69,11 @@ func fcopy(src, dest string, info os.FileInfo, opt Options) (err error) {
 	}
 	defer fclose(f, &err)
 
-	if err = os.Chmod(f.Name(), info.Mode()|opt.AddPermission); err != nil {
-		return
+	chmodfunc, err := opt.PermissionControl(info, dest)
+	if err != nil {
+		return err
 	}
+	chmodfunc(&err)
 
 	s, err := os.Open(src)
 	if err != nil {
@@ -137,14 +132,12 @@ func dcopy(srcdir, destdir string, info os.FileInfo, opt Options) (err error) {
 		return err // Unwelcome error type...!
 	}
 
-	originalMode := info.Mode()
-
 	// Make dest dir with 0755 so that everything writable.
-	if err = os.MkdirAll(destdir, tmpPermissionForDirectory); err != nil {
-		return
+	chmodfunc, err := opt.PermissionControl(info, destdir)
+	if err != nil {
+		return err
 	}
-	// Recover dir mode with original one.
-	defer chmod(destdir, originalMode|opt.AddPermission, &err)
+	defer chmodfunc(&err)
 
 	contents, err := ioutil.ReadDir(srcdir)
 	if err != nil {
@@ -214,30 +207,3 @@ func fclose(f *os.File, reported *error) {
 		*reported = err
 	}
 }
-
-// chmod ANYHOW changes file mode,
-// with asiging error raised during Chmod,
-// BUT respecting the error already reported.
-func chmod(dir string, mode os.FileMode, reported *error) {
-	if err := os.Chmod(dir, mode); *reported == nil {
-		*reported = err
-	}
-}
-
-// assure Options struct, should be called only once.
-// All optional values MUST NOT BE nil/zero after assured.
-func assure(src, dest string, opts ...Options) Options {
-	defopt := getDefaultOptions(src, dest)
-	if len(opts) == 0 {
-		return defopt
-	}
-	if opts[0].OnSymlink == nil {
-		opts[0].OnSymlink = defopt.OnSymlink
-	}
-	if opts[0].Skip == nil {
-		opts[0].Skip = defopt.Skip
-	}
-	opts[0].intent.src = defopt.intent.src
-	opts[0].intent.dest = defopt.intent.dest
-	return opts[0]
-}
diff --git a/fileinfo.go b/fileinfo_go1.15.go
similarity index 77%
rename from fileinfo.go
rename to fileinfo_go1.15.go
index 0b32abe..d0909b3 100644
--- a/fileinfo.go
+++ b/fileinfo_go1.15.go
@@ -1,12 +1,16 @@
+//+build !go1.16
+
 package copy
 
+import "os"
+
 // This is a cloned definition of os.FileInfo (go1.15) or fs.FileInfo (go1.16~)
 // A FileInfo describes a file and is returned by Stat.
 type fileInfo interface {
 	// Name() string       // base name of the file
 	// Size() int64        // length in bytes for regular files; system-dependent for others
-	// Mode() FileMode     // file mode bits
+	Mode() os.FileMode // file mode bits
 	// ModTime() time.Time // modification time
-	// IsDir() bool        // abbreviation for Mode().IsDir()
+	IsDir() bool      // abbreviation for Mode().IsDir()
 	Sys() interface{} // underlying data source (can return nil)
 }
diff --git a/fileinfo_go1.16.go b/fileinfo_go1.16.go
new file mode 100644
index 0000000..3bd2bcd
--- /dev/null
+++ b/fileinfo_go1.16.go
@@ -0,0 +1,16 @@
+//+build go1.16
+
+package copy
+
+import "io/fs"
+
+// This is a cloned definition of os.FileInfo (go1.15) or fs.FileInfo (go1.16~)
+// A FileInfo describes a file and is returned by Stat.
+type fileInfo interface {
+	// Name() string       // base name of the file
+	// Size() int64        // length in bytes for regular files; system-dependent for others
+	Mode() fs.FileMode // file mode bits
+	// ModTime() time.Time // modification time
+	IsDir() bool      // abbreviation for Mode().IsDir()
+	Sys() interface{} // underlying data source (can return nil)
+}
diff --git a/options.go b/options.go
index f598fc1..0045813 100644
--- a/options.go
+++ b/options.go
@@ -16,8 +16,18 @@ type Options struct {
 
 	// AddPermission to every entities,
 	// NO MORE THAN 0777
+	// @OBSOLETE
+	// Use `PermissionControl = AddPermission(perm)` instead
 	AddPermission os.FileMode
 
+	// PermissionControl can preserve or even add permission to
+	// every entries, for example
+	//
+	//		opt.PermissionControl = AddPermission(0222)
+	//
+	// See permission_control.go for more detail.
+	PermissionControl PermissionControlFunc
+
 	// Sync file after copy.
 	// Useful in case when file must be on the disk
 	// (in case crash happens, for example),
@@ -77,13 +87,37 @@ func getDefaultOptions(src, dest string) Options {
 		Skip: func(string) (bool, error) {
 			return false, nil // Don't skip
 		},
-		AddPermission:  0,     // Add nothing
-		Sync:           false, // Do not sync
-		PreserveTimes:  false, // Do not preserve the modification time
-		CopyBufferSize: 0,     // Do not specify, use default bufsize (32*1024)
+		AddPermission:     0,                  // Add nothing
+		PermissionControl: PerservePermission, // Just preserve permission
+		Sync:              false,              // Do not sync
+		PreserveTimes:     false,              // Do not preserve the modification time
+		CopyBufferSize:    0,                  // Do not specify, use default bufsize (32*1024)
 		intent: struct {
 			src  string
 			dest string
 		}{src, dest},
 	}
 }
+
+// assureOptions struct, should be called only once.
+// All optional values MUST NOT BE nil/zero after assured.
+func assureOptions(src, dest string, opts ...Options) Options {
+	defopt := getDefaultOptions(src, dest)
+	if len(opts) == 0 {
+		return defopt
+	}
+	if opts[0].OnSymlink == nil {
+		opts[0].OnSymlink = defopt.OnSymlink
+	}
+	if opts[0].Skip == nil {
+		opts[0].Skip = defopt.Skip
+	}
+	if opts[0].AddPermission > 0 {
+		opts[0].PermissionControl = AddPermission(opts[0].AddPermission)
+	} else if opts[0].PermissionControl == nil {
+		opts[0].PermissionControl = PerservePermission
+	}
+	opts[0].intent.src = defopt.intent.src
+	opts[0].intent.dest = defopt.intent.dest
+	return opts[0]
+}
diff --git a/permission_control.go b/permission_control.go
new file mode 100644
index 0000000..97ae12d
--- /dev/null
+++ b/permission_control.go
@@ -0,0 +1,48 @@
+package copy
+
+import (
+	"os"
+)
+
+const (
+	// tmpPermissionForDirectory makes the destination directory writable,
+	// so that stuff can be copied recursively even if any original directory is NOT writable.
+	// See https://github.com/otiai10/copy/pull/9 for more information.
+	tmpPermissionForDirectory = os.FileMode(0755)
+)
+
+type PermissionControlFunc func(srcinfo fileInfo, dest string) (chmodfunc func(*error), err error)
+
+var (
+	AddPermission = func(perm os.FileMode) PermissionControlFunc {
+		return func(srcinfo fileInfo, dest string) (func(*error), error) {
+			orig := srcinfo.Mode()
+			if srcinfo.IsDir() {
+				if err := os.MkdirAll(dest, tmpPermissionForDirectory); err != nil {
+					return func(*error) {}, err
+				}
+			}
+			return func(err *error) {
+				chmod(dest, orig|perm, err)
+			}, nil
+		}
+	}
+	PerservePermission PermissionControlFunc = AddPermission(0)
+	DoNothing          PermissionControlFunc = func(srcinfo fileInfo, dest string) (func(*error), error) {
+		if srcinfo.IsDir() {
+			if err := os.MkdirAll(dest, srcinfo.Mode()); err != nil {
+				return func(*error) {}, err
+			}
+		}
+		return func(*error) {}, nil
+	}
+)
+
+// chmod ANYHOW changes file mode,
+// with asiging error raised during Chmod,
+// BUT respecting the error already reported.
+func chmod(dir string, mode os.FileMode, reported *error) {
+	if err := os.Chmod(dir, mode); *reported == nil {
+		*reported = err
+	}
+}


Test patch:
diff --git a/all_test.go b/all_test.go
index f4a2069..6fb2f69 100644
--- a/all_test.go
+++ b/all_test.go
@@ -22,6 +22,7 @@ func teardown(m *testing.M) {
 	os.RemoveAll("test/data/case03/case01")
 	os.RemoveAll("test/data.copy")
 	os.RemoveAll("test/data.copyTime")
+	os.RemoveAll("test/owned-by-root") // Do not check the error ;)
 }
 
 func TestCopy(t *testing.T) {
@@ -29,6 +30,7 @@ func TestCopy(t *testing.T) {
 	Expect(t, err).ToBe(nil)
 	info, err := os.Stat("./test/data.copy/case00/README.md")
 	Expect(t, err).ToBe(nil)
+	Expect(t, info).Not().ToBe(nil)
 	Expect(t, info.IsDir()).ToBe(false)
 
 	When(t, "specified src doesn't exist", func(t *testing.T) {
@@ -195,7 +197,7 @@ func TestOptions_Skip(t *testing.T) {
 	})
 }
 
-func TestOptions_AddPermission(t *testing.T) {
+func TestOptions_PermissionControl(t *testing.T) {
 	info, err := os.Stat("test/data/case07/dir_0555")
 	Expect(t, err).ToBe(nil)
 	Expect(t, info.Mode()).ToBe(os.FileMode(0555) | os.ModeDir)
@@ -204,7 +206,7 @@ func TestOptions_AddPermission(t *testing.T) {
 	Expect(t, err).ToBe(nil)
 	Expect(t, info.Mode()).ToBe(os.FileMode(0444))
 
-	opt := Options{AddPermission: 0222}
+	opt := Options{PermissionControl: AddPermission(0222)}
 	err = Copy("test/data/case07", "test/data.copy/case07", opt)
 	Expect(t, err).ToBe(nil)
 
@@ -215,6 +217,13 @@ func TestOptions_AddPermission(t *testing.T) {
 	info, err = os.Stat("test/data.copy/case07/file_0444")
 	Expect(t, err).ToBe(nil)
 	Expect(t, info.Mode()).ToBe(os.FileMode(0444 | 0222))
+
+	When(t, "try to copy a dir owned by other users", func(t *testing.T) {
+		err := Copy("test/data/case14", "test/owned-by-root", Options{
+			PermissionControl: DoNothing, // ONLY docker tests fail when you comment out this line
+		})
+		Expect(t, err).ToBe(nil)
+	})
 }
 
 func TestOptions_Sync(t *testing.T) {
diff --git a/test/data/case14/README.md b/test/data/case14/README.md
new file mode 100644
index 0000000..98940b6
--- /dev/null
+++ b/test/data/case14/README.md
@@ -0,0 +1,14 @@
+# Case 14
+
+The destination folder is not owned by the process running the `Copy` operation, but is writeable by that process.
+
+In this case, the Copy operation requires the `Options.PermissionControl = DoNothing` option to be set, otherwise a `chmod` operation is attempted on the destination dir, which will fail.
+
+See https://github.com/otiai10/copy/pull/69.
+
+`PermissionControl` can replace `AddPermission` option and `AddPermission` option is obsolete.
+Use following instead:
+
+```go
+opt.PermissionControl = AddPermission(0222)
+```
\ No newline at end of file
diff --git a/test/images/alpine.Dockerfile b/test/images/alpine.Dockerfile
index 8e4c4e0..854bd82 100644
--- a/test/images/alpine.Dockerfile
+++ b/test/images/alpine.Dockerfile
@@ -1,14 +1,22 @@
 FROM alpine:3.14
 
+WORKDIR /app
+
 RUN apk update
 RUN apk add \
   g++ \
   git \
   go
 
-ENV GOPATH=${HOME}/go
-ENV GO111MODULE=on
-ADD . ${GOPATH}/src/github.com/otiai10/copy
-WORKDIR ${GOPATH}/src/github.com/otiai10/copy
+RUN adduser -g "" -D someuser
+
+# root-owned directory setup for test case 14
+RUN mkdir -p test/owned-by-root \
+  && chown :someuser test/owned-by-root \
+  && chmod 775 test/owned-by-root
+
+USER someuser
+
+COPY --chown=someuser:someuser . .
 
 CMD ["go", "test", "-v", "-cover", "./..."]
diff --git a/test/images/archlinux.Dockerfile b/test/images/archlinux.Dockerfile
index 4b6c58b..6459d51 100644
--- a/test/images/archlinux.Dockerfile
+++ b/test/images/archlinux.Dockerfile
@@ -1,14 +1,22 @@
 FROM archlinux:base-20201220.0.11678
 
+WORKDIR /app
+
 RUN pacman -Sy -q --noconfirm \
   glibc \
   git \
   gcc \
   go
 
-ENV GOPATH=${HOME}/go
-ENV GO111MODULE=on
-ADD . ${GOPATH}/src/github.com/otiai10/copy
-WORKDIR ${GOPATH}/src/github.com/otiai10/copy
+RUN useradd -m someuser
+
+# root-owned directory setup for test case 14
+RUN mkdir -p test/owned-by-root \
+  && chown :someuser test/owned-by-root \
+  && chmod 775 test/owned-by-root
+
+USER someuser
+
+COPY --chown=someuser:someuser . .
 
 CMD ["go", "test", "-v", "-cover", "./..."]
diff --git a/test/images/centos.Dockerfile b/test/images/centos.Dockerfile
index 05cf959..d7a34d5 100644
--- a/test/images/centos.Dockerfile
+++ b/test/images/centos.Dockerfile
@@ -1,13 +1,21 @@
 FROM centos:centos8
 
+WORKDIR /app
+
 RUN yum update -y -q \
     && yum install -y --quiet \
       git \
       go
 
-ENV GOPATH=${HOME}/go
-ENV GO111MODULE=on
-ADD . ${GOPATH}/src/github.com/otiai10/copy
-WORKDIR ${GOPATH}/src/github.com/otiai10/copy
+RUN useradd -m someuser
+
+# root-owned directory setup for test case 14
+RUN mkdir -p test/owned-by-root \
+  && chown :someuser test/owned-by-root \
+  && chmod 775 test/owned-by-root
+
+USER someuser
+
+COPY --chown=someuser:someuser . .
 
 CMD ["go", "test", "-v", "-cover", "./..."]