Skip to content

Commit bed61f7

Browse files
johnstcnjohnstcn
authored
fix(coderd): correctly handle tar dir entries with missing path separator (coder#12479)
* coderd: add test to reproduce trailing directory issue * coderd: add trailing path separator to dir entries when converting to zip * provisionersdk: add trailing path separator to directory entries
1 parent 21d1873 commit bed61f7

File tree

3 files changed

+64
-14
lines changed

3 files changed

+64
-14
lines changed

‎coderd/fileszip.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"errors"
88
"io"
99
"log"
10+
"strings"
1011
)
1112

1213
func CreateTarFromZip(zipReader *zip.Reader) ([]byte, error) {
@@ -87,6 +88,12 @@ func WriteZipArchive(w io.Writer, tarReader *tar.Reader) error {
8788
return err
8889
}
8990
zipHeader.Name = tarHeader.Name
91+
// Some versions of unzip do not check the mode on a file entry and
92+
// simply assume that entries with a trailing path separator (/) are
93+
// directories, and that everything else is a file. Give them a hint.
94+
if tarHeader.FileInfo().IsDir() && !strings.HasSuffix(tarHeader.Name, "/") {
95+
zipHeader.Name += "/"
96+
}
9097

9198
zipEntry, err := zipWriter.CreateHeader(zipHeader)
9299
if err != nil {

‎coderd/fileszip_test.go

Lines changed: 52 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -58,26 +58,64 @@ func TestCreateZipFromTar(t *testing.T) {
5858
if runtime.GOOS != "linux" {
5959
t.Skip("skipping this test on non-Linux platform")
6060
}
61+
t.Run("OK", func(t *testing.T) {
62+
t.Parallel()
63+
tarBytes, err := os.ReadFile(filepath.Join(".", "testdata", "test.tar"))
64+
require.NoError(t, err, "failed to read sample tar file")
6165

62-
tarBytes, err := os.ReadFile(filepath.Join("testdata", "test.tar"))
63-
require.NoError(t, err, "failed to read sample tar file")
66+
tr := tar.NewReader(bytes.NewReader(tarBytes))
67+
zipBytes, err := coderd.CreateZipFromTar(tr)
68+
require.NoError(t, err)
6469

65-
tr := tar.NewReader(bytes.NewReader(tarBytes))
66-
zipBytes, err := coderd.CreateZipFromTar(tr)
67-
require.NoError(t, err)
70+
assertSampleZipFile(t, zipBytes)
6871

69-
assertSampleZipFile(t, zipBytes)
72+
tempDir := t.TempDir()
73+
tempFilePath := filepath.Join(tempDir, "test.zip")
74+
err = os.WriteFile(tempFilePath, zipBytes, 0o600)
75+
require.NoError(t, err, "failed to write converted zip file")
7076

71-
tempDir := t.TempDir()
72-
tempFilePath := filepath.Join(tempDir, "test.zip")
73-
err = os.WriteFile(tempFilePath, zipBytes, 0o600)
74-
require.NoError(t, err, "failed to write converted zip file")
77+
ctx := testutil.Context(t, testutil.WaitShort)
78+
cmd := exec.CommandContext(ctx, "unzip", tempFilePath, "-d", tempDir)
79+
require.NoError(t, cmd.Run(), "failed to extract converted zip file")
7580

76-
ctx := testutil.Context(t, testutil.WaitShort)
77-
cmd := exec.CommandContext(ctx, "unzip", tempFilePath, "-d", tempDir)
78-
require.NoError(t, cmd.Run(), "failed to extract converted zip file")
81+
assertExtractedFiles(t, tempDir, false)
82+
})
7983

80-
assertExtractedFiles(t, tempDir, false)
84+
t.Run("MissingSlashInDirectoryHeader", func(t *testing.T) {
85+
t.Parallel()
86+
87+
// Given: a tar archive containing a directory entry that has the directory
88+
// mode bit set but the name is missing a trailing slash
89+
90+
var tarBytes bytes.Buffer
91+
tw := tar.NewWriter(&tarBytes)
92+
tw.WriteHeader(&tar.Header{
93+
Name: "dir",
94+
Typeflag: tar.TypeDir,
95+
Size: 0,
96+
})
97+
require.NoError(t, tw.Flush())
98+
require.NoError(t, tw.Close())
99+
100+
// When: we convert this to a zip
101+
tr := tar.NewReader(&tarBytes)
102+
zipBytes, err := coderd.CreateZipFromTar(tr)
103+
require.NoError(t, err)
104+
105+
// Then: the resulting zip should contain a corresponding directory
106+
zr, err := zip.NewReader(bytes.NewReader(zipBytes), int64(len(zipBytes)))
107+
require.NoError(t, err)
108+
for _, zf := range zr.File {
109+
switch zf.Name {
110+
case "dir":
111+
require.Fail(t, "missing trailing slash in directory name")
112+
case "dir/":
113+
require.True(t, zf.Mode().IsDir(), "should be a directory")
114+
default:
115+
require.Fail(t, "unexpected file in archive")
116+
}
117+
}
118+
})
81119
}
82120

83121
// nolint:revive // this is a control flag but it's in a unit test

‎provisionersdk/archive.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,11 @@ func Tar(w io.Writer, logger slog.Logger, directory string, limit int64) error {
107107
}
108108
// Use unix paths in the tar archive.
109109
header.Name = filepath.ToSlash(rel)
110+
// tar.FileInfoHeader() will do this, but filepath.Rel() calls filepath.Clean()
111+
// which strips trailing path separators for directories.
112+
if fileInfo.IsDir() {
113+
header.Name += "/"
114+
}
110115
if err := tarWriter.WriteHeader(header); err != nil {
111116
return err
112117
}

0 commit comments

Comments
 (0)