Repository: goreleaser/nfpm
Pull request #411 (2021-12-01 11:48:57)

fix: Prevent duplicate folders from being created.
Currently a package can contain duplicate folders depending on the way a user specified a dir-type content destination (folder name). This PR fixes that by keeping track of created folders in the already existing `created` map with a **consistent naming convention**. It also adds related tests.

Another bug that's also fixed by this PR is that auto-created folders did not have a owner username and group name.

codecov[bot]: # [Codecov](https://codecov.io/gh/goreleaser/nfpm/pull/411?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=goreleaser) Report
> Merging [#411](https://codecov.io/gh/goreleaser/nfpm/pull/411?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=goreleaser) (abc5257) into [main](https://codecov.io/gh/goreleaser/nfpm/commit/9570f5234fee197c5037ede97029c830cb894249?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=goreleaser) (9570f52) will **increase** coverage by `0.48%`.
> The diff coverage is `86.20%`.

[![Impacted file tree graph](https://codecov.io/gh/goreleaser/nfpm/pull/411/graphs/tree.svg?width=650&height=150&src=pr&token=9QKGIxn7wg&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=goreleaser)](https://codecov.io/gh/goreleaser/nfpm/pull/411?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=goreleaser)

```diff
@@            Coverage Diff             @@
##             main     #411      +/-   ##
==========================================
+ Coverage   66.19%   66.68%   +0.48%     
==========================================
  Files          15       15              
  Lines        1855     1876      +21     
==========================================
+ Hits         1228     1251      +23     
+ Misses        488      487       -1     
+ Partials      139      138       -1     
```


| [Impacted Files](https://codecov.io/gh/goreleaser/nfpm/pull/411?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=goreleaser) | Coverage Δ | |
|---|---|---|
| [files/files.go](https://codecov.io/gh/goreleaser/nfpm/pull/411/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=goreleaser#diff-ZmlsZXMvZmlsZXMuZ28=) | `88.46% <20.00%> (-2.88%)` | :arrow_down: |
| [apk/apk.go](https://codecov.io/gh/goreleaser/nfpm/pull/411/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=goreleaser#diff-YXBrL2Fway5nbw==) | `72.70% <100.00%> (+0.89%)` | :arrow_up: |
| [deb/deb.go](https://codecov.io/gh/goreleaser/nfpm/pull/411/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=goreleaser#diff-ZGViL2RlYi5nbw==) | `70.60% <100.00%> (+1.69%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/goreleaser/nfpm/pull/411?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=goreleaser).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=goreleaser)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/goreleaser/nfpm/pull/411?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=goreleaser). Last update [9570f52...abc5257](https://codecov.io/gh/goreleaser/nfpm/pull/411?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=goreleaser). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=goreleaser).


Patch:
diff --git a/apk/apk.go b/apk/apk.go
index 62e836c..c5eb590 100644
--- a/apk/apk.go
+++ b/apk/apk.go
@@ -414,8 +414,14 @@ func createFilesInsideTarGz(info *nfpm.Info, tw *tar.Writer, created map[string]
 			continue
 		}
 
+		normalizedName := normalizePath(strings.Trim(file.Destination, "/")) + "/"
+
+		if created[normalizedName] {
+			return fmt.Errorf("duplicate directory: %q", normalizedName)
+		}
+
 		err = tw.WriteHeader(&tar.Header{
-			Name:     files.ToNixPath(strings.Trim(file.Destination, "/") + "/"),
+			Name:     normalizedName,
 			Mode:     int64(file.FileInfo.Mode),
 			Typeflag: tar.TypeDir,
 			Format:   tar.FormatGNU,
@@ -427,7 +433,7 @@ func createFilesInsideTarGz(info *nfpm.Info, tw *tar.Writer, created map[string]
 			return err
 		}
 
-		created[strings.TrimPrefix(file.Destination, "/")] = true
+		created[normalizedName] = true
 	}
 
 	for _, file := range info.Contents {
@@ -483,7 +489,7 @@ func copyToTarAndDigest(file *files.Content, tw *tar.Writer, sizep *int64) error
 	// tar.FileInfoHeader only uses file.Mode().Perm() which masks the mode with
 	// 0o777 which we don't want because we want to be able to set the suid bit.
 	header.Mode = int64(file.Mode())
-	header.Name = files.ToNixPath(file.Destination[1:])
+	header.Name = normalizePath(file.Destination)
 	header.Uname = file.FileInfo.Owner
 	header.Gname = file.FileInfo.Group
 	if err = newItemInsideTarGz(tw, contents, header); err != nil {
@@ -494,20 +500,30 @@ func copyToTarAndDigest(file *files.Content, tw *tar.Writer, sizep *int64) error
 	return nil
 }
 
+// normalizePath returns a path separated by slashes without a leading slash.
+func normalizePath(src string) string {
+	return files.ToNixPath(strings.TrimLeft(src, "/"))
+}
+
 // this is needed because the data.tar.gz file should have the empty folders
 // as well, so we walk through the dst and create all subfolders.
 func createTree(tarw *tar.Writer, dst string, created map[string]bool) error {
 	for _, path := range pathsToCreate(dst) {
+		path = normalizePath(path) + "/"
+
 		if created[path] {
 			// skipping dir that was previously created inside the archive
 			// (eg: usr/)
 			continue
 		}
+
 		if err := tarw.WriteHeader(&tar.Header{
-			Name:     files.ToNixPath(path + "/"),
+			Name:     path,
 			Mode:     0o755,
 			Typeflag: tar.TypeDir,
 			Format:   tar.FormatGNU,
+			Uname:    "root",
+			Gname:    "root",
 		}); err != nil {
 			return fmt.Errorf("failed to create folder %s: %w", path, err)
 		}
diff --git a/deb/deb.go b/deb/deb.go
index c9e04b3..98eafa0 100644
--- a/deb/deb.go
+++ b/deb/deb.go
@@ -280,8 +280,14 @@ func createFilesInsideDataTar(info *nfpm.Info, tw *tar.Writer,
 			continue
 		}
 
+		normalizedName := normalizePath(strings.Trim(file.Destination, "/")) + "/"
+
+		if created[normalizedName] {
+			return md5buf, 0, fmt.Errorf("duplicate directory: %q", normalizedName)
+		}
+
 		err = tw.WriteHeader(&tar.Header{
-			Name:     normalizePath(strings.Trim(file.Destination, "/") + "/"),
+			Name:     normalizedName,
 			Mode:     int64(file.FileInfo.Mode),
 			Typeflag: tar.TypeDir,
 			Format:   tar.FormatGNU,
@@ -293,7 +299,7 @@ func createFilesInsideDataTar(info *nfpm.Info, tw *tar.Writer,
 			return md5buf, 0, err
 		}
 
-		created[strings.TrimPrefix(file.Destination, "/")] = true
+		created[normalizedName] = true
 	}
 
 	// create files and implicit directories
@@ -588,12 +594,15 @@ func createTree(tarw *tar.Writer, dst string, created map[string]bool) error {
 			// (eg: usr/)
 			continue
 		}
+
 		if err := tarw.WriteHeader(&tar.Header{
 			Name:     path,
 			Mode:     0o755,
 			Typeflag: tar.TypeDir,
 			Format:   tar.FormatGNU,
 			ModTime:  time.Now(),
+			Uname:    "root",
+			Gname:    "root",
 		}); err != nil {
 			return fmt.Errorf("failed to create folder: %w", err)
 		}
diff --git a/files/files.go b/files/files.go
index e837a4c..f2e60dc 100644
--- a/files/files.go
+++ b/files/files.go
@@ -205,8 +205,13 @@ func checkNoCollisions(contents Contents) error {
 	for _, elem := range contents {
 		present, ok := alreadyPresent[elem.Destination]
 		if ok && (present.Packager == "" || elem.Packager == "" || present.Packager == elem.Packager) {
+			if elem.Type == "dir" {
+				return fmt.Errorf("cannot add directory %q because it is already present: %w",
+					elem.Destination, ErrContentCollision)
+			}
+
 			return fmt.Errorf(
-				"cannot add %s because %s is already present at the same destination (%s): %w",
+				"cannot add %q because %q is already present at the same destination (%s): %w",
 				elem.Source, present.Source, present.Destination, ErrContentCollision)
 		}
 


Test patch:
diff --git a/apk/apk_test.go b/apk/apk_test.go
index 639ad2c..bf09992 100644
--- a/apk/apk_test.go
+++ b/apk/apk_test.go
@@ -410,6 +410,25 @@ func extractFromTar(t *testing.T, tarFile []byte, fileName string) []byte {
 	return nil
 }
 
+func tarContents(tb testing.TB, tarFile []byte) []string {
+	tb.Helper()
+
+	contents := []string{}
+
+	tr := tar.NewReader(bytes.NewReader(tarFile))
+	for {
+		hdr, err := tr.Next()
+		if errors.Is(err, io.EOF) {
+			break // End of archive
+		}
+		require.NoError(tb, err)
+
+		contents = append(contents, hdr.Name)
+	}
+
+	return contents
+}
+
 func TestAPKConventionalFileName(t *testing.T) {
 	apkName := "default"
 	testCases := []struct {
@@ -516,6 +535,66 @@ func TestDirectories(t *testing.T) {
 	require.Equal(t, h.Typeflag, byte(tar.TypeDir))
 }
 
+func TestNoDuplicateAutocreatedDirectories(t *testing.T) {
+	info := exampleInfo()
+	info.DisableGlobbing = true
+	info.Contents = []*files.Content{
+		{
+			Source:      "../testdata/fake",
+			Destination: "/etc/foo/bar",
+		},
+		{
+			Type:        "dir",
+			Destination: "/etc/foo",
+		},
+	}
+	require.NoError(t, info.Validate())
+
+	expected := map[string]bool{
+		"etc/":        true,
+		"etc/foo/":    true,
+		"etc/foo/bar": true,
+	}
+
+	var buf bytes.Buffer
+	size := int64(0)
+	err := createFilesInsideTarGz(info, tar.NewWriter(&buf), make(map[string]bool), &size)
+	require.NoError(t, err)
+
+	contents := tarContents(t, buf.Bytes())
+
+	if len(expected) != len(contents) {
+		t.Fatalf("contents has %d entries instead of %d: %#v", len(contents), len(expected), contents)
+	}
+
+	for _, entry := range contents {
+		if !expected[entry] {
+			t.Fatalf("unexpected content: %q", entry)
+		}
+	}
+}
+
+func TestNoDuplicateDirectories(t *testing.T) {
+	info := exampleInfo()
+	info.DisableGlobbing = true
+	info.Contents = []*files.Content{
+		{
+			Type:        "dir",
+			Destination: "/etc/foo",
+		},
+		{
+			Type:        "dir",
+			Destination: "/etc/foo/",
+		},
+	}
+	require.NoError(t, info.Validate())
+
+	var buf bytes.Buffer
+	size := int64(0)
+	err := createFilesInsideTarGz(info, tar.NewWriter(&buf), make(map[string]bool), &size)
+	require.Error(t, err)
+}
+
 func TestNoDuplicateContents(t *testing.T) {
 	info := exampleInfo()
 	info.Contents = []*files.Content{
diff --git a/deb/deb_test.go b/deb/deb_test.go
index 189f849..deee52d 100644
--- a/deb/deb_test.go
+++ b/deb/deb_test.go
@@ -947,6 +947,62 @@ func TestDisableGlobbing(t *testing.T) {
 	require.Equal(t, expectedContent, actualContent)
 }
 
+func TestNoDuplicateAutocreatedDirectories(t *testing.T) {
+	info := exampleInfo()
+	info.DisableGlobbing = true
+	info.Contents = []*files.Content{
+		{
+			Source:      "../testdata/fake",
+			Destination: "/etc/foo/bar",
+		},
+		{
+			Type:        "dir",
+			Destination: "/etc/foo",
+		},
+	}
+	require.NoError(t, info.Validate())
+
+	expected := map[string]bool{
+		"./etc/":        true,
+		"./etc/foo/":    true,
+		"./etc/foo/bar": true,
+	}
+
+	dataTarball, _, _, tarballName, err := createDataTarball(info)
+	require.NoError(t, err)
+
+	contents := tarContents(t, inflate(t, tarballName, dataTarball))
+
+	if len(expected) != len(contents) {
+		t.Fatalf("contents has %d entries instead of %d: %#v", len(contents), len(expected), contents)
+	}
+
+	for _, entry := range contents {
+		if !expected[entry] {
+			t.Fatalf("unexpected content: %q", entry)
+		}
+	}
+}
+
+func TestNoDuplicateDirectories(t *testing.T) {
+	info := exampleInfo()
+	info.DisableGlobbing = true
+	info.Contents = []*files.Content{
+		{
+			Type:        "dir",
+			Destination: "/etc/foo",
+		},
+		{
+			Type:        "dir",
+			Destination: "/etc/foo/",
+		},
+	}
+	require.NoError(t, info.Validate())
+
+	_, _, _, _, err := createDataTarball(info)
+	require.Error(t, err)
+}
+
 func TestCompressionAlgorithms(t *testing.T) {
 	testCases := []struct {
 		algorithm       string
@@ -1028,6 +1084,25 @@ func tarContains(tb testing.TB, tarFile []byte, filename string) bool {
 	return false
 }
 
+func tarContents(tb testing.TB, tarFile []byte) []string {
+	tb.Helper()
+
+	contents := []string{}
+
+	tr := tar.NewReader(bytes.NewReader(tarFile))
+	for {
+		hdr, err := tr.Next()
+		if errors.Is(err, io.EOF) {
+			break // End of archive
+		}
+		require.NoError(tb, err)
+
+		contents = append(contents, hdr.Name)
+	}
+
+	return contents
+}
+
 func extractFileHeaderFromTar(tb testing.TB, tarFile []byte, filename string) *tar.Header {
 	tb.Helper()