From 554281c7a5a87f623e82928107a2599de39227a1 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 25 Oct 2024 12:20:51 +0100 Subject: [PATCH 1/4] chore(coderd): extract fileszip to its own package for reuse --- coderd/files.go | 5 +- coderd/files_test.go | 19 ++- fileszip/fileszip.go | 112 ++++++++++++++++++ {coderd => fileszip}/fileszip_test.go | 109 ++--------------- fileszip/filesziptest/filesziptest.go | 109 +++++++++++++++++ .../filesziptest}/testdata/test.tar | Bin .../filesziptest}/testdata/test.zip | Bin 7 files changed, 244 insertions(+), 110 deletions(-) create mode 100644 fileszip/fileszip.go rename {coderd => fileszip}/fileszip_test.go (62%) create mode 100644 fileszip/filesziptest/filesziptest.go rename {coderd => fileszip/filesziptest}/testdata/test.tar (100%) rename {coderd => fileszip/filesziptest}/testdata/test.zip (100%) diff --git a/coderd/files.go b/coderd/files.go index d16a3447a1d94..8e410fc5d1494 100644 --- a/coderd/files.go +++ b/coderd/files.go @@ -21,6 +21,7 @@ import ( "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/coderd/httpmw" "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/fileszip" ) const ( @@ -75,7 +76,7 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) { return } - data, err = CreateTarFromZip(zipReader) + data, err = fileszip.CreateTarFromZip(zipReader, httpFileMaxBytes) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error processing .zip archive.", @@ -181,7 +182,7 @@ func (api *API) fileByID(rw http.ResponseWriter, r *http.Request) { rw.Header().Set("Content-Type", codersdk.ContentTypeZip) rw.WriteHeader(http.StatusOK) - err = WriteZipArchive(rw, tar.NewReader(bytes.NewReader(file.Data))) + err = fileszip.WriteZipArchive(rw, tar.NewReader(bytes.NewReader(file.Data)), httpFileMaxBytes) if err != nil { api.Logger.Error(ctx, "invalid .zip archive", slog.F("file_id", fileID), slog.F("mimetype", file.Mimetype), slog.Error(err)) } diff --git a/coderd/files_test.go b/coderd/files_test.go index a5e6aab2498e1..fef30dfddf439 100644 --- a/coderd/files_test.go +++ b/coderd/files_test.go @@ -5,8 +5,6 @@ import ( "bytes" "context" "net/http" - "os" - "path/filepath" "testing" "github.com/google/uuid" @@ -15,6 +13,7 @@ import ( "github.com/coder/coder/v2/coderd" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/fileszip/filesziptest" "github.com/coder/coder/v2/testutil" ) @@ -84,8 +83,8 @@ func TestDownload(t *testing.T) { // given ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - tarball, err := os.ReadFile(filepath.Join("testdata", "test.tar")) - require.NoError(t, err) + + tarball := filesziptest.TestTarFileBytes() // when resp, err := client.Upload(ctx, codersdk.ContentTypeTar, bytes.NewReader(tarball)) @@ -97,7 +96,7 @@ func TestDownload(t *testing.T) { require.Len(t, data, len(tarball)) require.Equal(t, codersdk.ContentTypeTar, contentType) require.Equal(t, tarball, data) - assertSampleTarFile(t, data) + filesziptest.AssertSampleTarFile(t, data) }) t.Run("InsertZip_DownloadTar", func(t *testing.T) { @@ -106,8 +105,7 @@ func TestDownload(t *testing.T) { _ = coderdtest.CreateFirstUser(t, client) // given - zipContent, err := os.ReadFile(filepath.Join("testdata", "test.zip")) - require.NoError(t, err) + zipContent := filesziptest.TestZipFileBytes() ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() @@ -123,7 +121,7 @@ func TestDownload(t *testing.T) { // Note: creating a zip from a tar will result in some loss of information // as zip files do not store UNIX user:group data. - assertSampleTarFile(t, data) + filesziptest.AssertSampleTarFile(t, data) }) t.Run("InsertTar_DownloadZip", func(t *testing.T) { @@ -132,8 +130,7 @@ func TestDownload(t *testing.T) { _ = coderdtest.CreateFirstUser(t, client) // given - tarball, err := os.ReadFile(filepath.Join("testdata", "test.tar")) - require.NoError(t, err) + tarball := filesziptest.TestTarFileBytes() tarReader := tar.NewReader(bytes.NewReader(tarball)) expectedZip, err := coderd.CreateZipFromTar(tarReader) @@ -151,6 +148,6 @@ func TestDownload(t *testing.T) { // then require.Equal(t, codersdk.ContentTypeZip, contentType) require.Equal(t, expectedZip, data) - assertSampleZipFile(t, data) + filesziptest.AssertSampleZipFile(t, data) }) } diff --git a/fileszip/fileszip.go b/fileszip/fileszip.go new file mode 100644 index 0000000000000..a1d3f4c38b8dc --- /dev/null +++ b/fileszip/fileszip.go @@ -0,0 +1,112 @@ +package fileszip + +import ( + "archive/tar" + "archive/zip" + "bytes" + "errors" + "io" + "log" + "strings" +) + +func CreateTarFromZip(zipReader *zip.Reader, maxSize int64) ([]byte, error) { + var tarBuffer bytes.Buffer + err := writeTarArchive(&tarBuffer, zipReader, maxSize) + if err != nil { + return nil, err + } + return tarBuffer.Bytes(), nil +} + +func writeTarArchive(w io.Writer, zipReader *zip.Reader, maxSize int64) error { + tarWriter := tar.NewWriter(w) + defer tarWriter.Close() + + for _, file := range zipReader.File { + err := processFileInZipArchive(file, tarWriter, maxSize) + if err != nil { + return err + } + } + return nil +} + +func processFileInZipArchive(file *zip.File, tarWriter *tar.Writer, maxSize int64) error { + fileReader, err := file.Open() + if err != nil { + return err + } + defer fileReader.Close() + + err = tarWriter.WriteHeader(&tar.Header{ + Name: file.Name, + Size: file.FileInfo().Size(), + Mode: int64(file.Mode()), + ModTime: file.Modified, + // Note: Zip archives do not store ownership information. + Uid: 1000, + Gid: 1000, + }) + if err != nil { + return err + } + + n, err := io.CopyN(tarWriter, fileReader, maxSize) + log.Println(file.Name, n, err) + if errors.Is(err, io.EOF) { + err = nil + } + return err +} + +func CreateZipFromTar(tarReader *tar.Reader, maxSize int64) ([]byte, error) { + var zipBuffer bytes.Buffer + err := WriteZipArchive(&zipBuffer, tarReader, maxSize) + if err != nil { + return nil, err + } + return zipBuffer.Bytes(), nil +} + +func WriteZipArchive(w io.Writer, tarReader *tar.Reader, maxSize int64) error { + zipWriter := zip.NewWriter(w) + defer zipWriter.Close() + + for { + tarHeader, err := tarReader.Next() + if errors.Is(err, io.EOF) { + break + } + + if err != nil { + return err + } + + zipHeader, err := zip.FileInfoHeader(tarHeader.FileInfo()) + if err != nil { + return err + } + zipHeader.Name = tarHeader.Name + // Some versions of unzip do not check the mode on a file entry and + // simply assume that entries with a trailing path separator (/) are + // directories, and that everything else is a file. Give them a hint. + if tarHeader.FileInfo().IsDir() && !strings.HasSuffix(tarHeader.Name, "/") { + zipHeader.Name += "/" + } + + zipEntry, err := zipWriter.CreateHeader(zipHeader) + if err != nil { + return err + } + + _, err = io.CopyN(zipEntry, tarReader, maxSize) + if errors.Is(err, io.EOF) { + err = nil + } + if err != nil { + return err + } + } + return nil // don't need to flush as we call `writer.Close()` +} diff --git a/coderd/fileszip_test.go b/fileszip/fileszip_test.go similarity index 62% rename from coderd/fileszip_test.go rename to fileszip/fileszip_test.go index 1c3781c39d70b..221b3e924471f 100644 --- a/coderd/fileszip_test.go +++ b/fileszip/fileszip_test.go @@ -1,10 +1,9 @@ -package coderd_test +package fileszip_test import ( "archive/tar" "archive/zip" "bytes" - "io" "io/fs" "os" "os/exec" @@ -12,13 +11,12 @@ import ( "runtime" "strings" "testing" - "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "golang.org/x/xerrors" - "github.com/coder/coder/v2/coderd" + "github.com/coder/coder/v2/fileszip" + "github.com/coder/coder/v2/fileszip/filesziptest" "github.com/coder/coder/v2/testutil" ) @@ -30,18 +28,17 @@ func TestCreateTarFromZip(t *testing.T) { // Read a zip file we prepared earlier ctx := testutil.Context(t, testutil.WaitShort) - zipBytes, err := os.ReadFile(filepath.Join("testdata", "test.zip")) - require.NoError(t, err, "failed to read sample zip file") + zipBytes := filesziptest.TestZipFileBytes() // Assert invariant - assertSampleZipFile(t, zipBytes) + filesziptest.AssertSampleZipFile(t, zipBytes) zr, err := zip.NewReader(bytes.NewReader(zipBytes), int64(len(zipBytes))) require.NoError(t, err, "failed to parse sample zip file") - tarBytes, err := coderd.CreateTarFromZip(zr) + tarBytes, err := fileszip.CreateTarFromZip(zr, 10240) require.NoError(t, err, "failed to convert zip to tar") - assertSampleTarFile(t, tarBytes) + filesziptest.AssertSampleTarFile(t, tarBytes) tempDir := t.TempDir() tempFilePath := filepath.Join(tempDir, "test.tar") @@ -60,14 +57,13 @@ func TestCreateZipFromTar(t *testing.T) { } t.Run("OK", func(t *testing.T) { t.Parallel() - tarBytes, err := os.ReadFile(filepath.Join(".", "testdata", "test.tar")) - require.NoError(t, err, "failed to read sample tar file") + tarBytes := filesziptest.TestTarFileBytes() tr := tar.NewReader(bytes.NewReader(tarBytes)) - zipBytes, err := coderd.CreateZipFromTar(tr) + zipBytes, err := fileszip.CreateZipFromTar(tr, 10240) require.NoError(t, err) - assertSampleZipFile(t, zipBytes) + filesziptest.AssertSampleZipFile(t, zipBytes) tempDir := t.TempDir() tempFilePath := filepath.Join(tempDir, "test.zip") @@ -99,7 +95,7 @@ func TestCreateZipFromTar(t *testing.T) { // When: we convert this to a zip tr := tar.NewReader(&tarBytes) - zipBytes, err := coderd.CreateZipFromTar(tr) + zipBytes, err := fileszip.CreateZipFromTar(tr, 10240) require.NoError(t, err) // Then: the resulting zip should contain a corresponding directory @@ -133,7 +129,7 @@ func assertExtractedFiles(t *testing.T, dir string, checkModePerm bool) { if checkModePerm { assert.Equal(t, fs.ModePerm&0o755, stat.Mode().Perm(), "expected mode 0755 on directory") } - assert.Equal(t, archiveRefTime(t).UTC(), stat.ModTime().UTC(), "unexpected modtime of %q", path) + assert.Equal(t, filesziptest.ArchiveRefTime(t).UTC(), stat.ModTime().UTC(), "unexpected modtime of %q", path) case "/test/hello.txt": stat, err := os.Stat(path) assert.NoError(t, err, "failed to stat path %q", path) @@ -168,84 +164,3 @@ func assertExtractedFiles(t *testing.T, dir string, checkModePerm bool) { return nil }) } - -func assertSampleTarFile(t *testing.T, tarBytes []byte) { - t.Helper() - - tr := tar.NewReader(bytes.NewReader(tarBytes)) - for { - hdr, err := tr.Next() - if err != nil { - if err == io.EOF { - return - } - require.NoError(t, err) - } - - // Note: ignoring timezones here. - require.Equal(t, archiveRefTime(t).UTC(), hdr.ModTime.UTC()) - - switch hdr.Name { - case "test/": - require.Equal(t, hdr.Typeflag, byte(tar.TypeDir)) - case "test/hello.txt": - require.Equal(t, hdr.Typeflag, byte(tar.TypeReg)) - bs, err := io.ReadAll(tr) - if err != nil && !xerrors.Is(err, io.EOF) { - require.NoError(t, err) - } - require.Equal(t, "hello", string(bs)) - case "test/dir/": - require.Equal(t, hdr.Typeflag, byte(tar.TypeDir)) - case "test/dir/world.txt": - require.Equal(t, hdr.Typeflag, byte(tar.TypeReg)) - bs, err := io.ReadAll(tr) - if err != nil && !xerrors.Is(err, io.EOF) { - require.NoError(t, err) - } - require.Equal(t, "world", string(bs)) - default: - require.Failf(t, "unexpected file in tar", hdr.Name) - } - } -} - -func assertSampleZipFile(t *testing.T, zipBytes []byte) { - t.Helper() - - zr, err := zip.NewReader(bytes.NewReader(zipBytes), int64(len(zipBytes))) - require.NoError(t, err) - - for _, f := range zr.File { - // Note: ignoring timezones here. - require.Equal(t, archiveRefTime(t).UTC(), f.Modified.UTC()) - switch f.Name { - case "test/", "test/dir/": - // directory - case "test/hello.txt": - rc, err := f.Open() - require.NoError(t, err) - bs, err := io.ReadAll(rc) - _ = rc.Close() - require.NoError(t, err) - require.Equal(t, "hello", string(bs)) - case "test/dir/world.txt": - rc, err := f.Open() - require.NoError(t, err) - bs, err := io.ReadAll(rc) - _ = rc.Close() - require.NoError(t, err) - require.Equal(t, "world", string(bs)) - default: - require.Failf(t, "unexpected file in zip", f.Name) - } - } -} - -// archiveRefTime is the Go reference time. The contents of the sample tar and zip files -// in testdata/ all have their modtimes set to the below in some timezone. -func archiveRefTime(t *testing.T) time.Time { - locMST, err := time.LoadLocation("MST") - require.NoError(t, err, "failed to load MST timezone") - return time.Date(2006, 1, 2, 3, 4, 5, 0, locMST) -} diff --git a/fileszip/filesziptest/filesziptest.go b/fileszip/filesziptest/filesziptest.go new file mode 100644 index 0000000000000..df4902459ef49 --- /dev/null +++ b/fileszip/filesziptest/filesziptest.go @@ -0,0 +1,109 @@ +package filesziptest + +import ( + "archive/tar" + "archive/zip" + "bytes" + _ "embed" + "io" + "testing" + "time" + + "github.com/stretchr/testify/require" + "golang.org/x/xerrors" +) + +//go:embed testdata/test.tar +var testTarFileBytes []byte + +//go:embed testdata/test.zip +var testZipFileBytes []byte + +func TestTarFileBytes() []byte { + return append([]byte{}, testTarFileBytes...) +} + +func TestZipFileBytes() []byte { + return append([]byte{}, testZipFileBytes...) +} + +func AssertSampleTarFile(t *testing.T, tarBytes []byte) { + t.Helper() + + tr := tar.NewReader(bytes.NewReader(tarBytes)) + for { + hdr, err := tr.Next() + if err != nil { + if err == io.EOF { + return + } + require.NoError(t, err) + } + + // Note: ignoring timezones here. + require.Equal(t, ArchiveRefTime(t).UTC(), hdr.ModTime.UTC()) + + switch hdr.Name { + case "test/": + require.Equal(t, hdr.Typeflag, byte(tar.TypeDir)) + case "test/hello.txt": + require.Equal(t, hdr.Typeflag, byte(tar.TypeReg)) + bs, err := io.ReadAll(tr) + if err != nil && !xerrors.Is(err, io.EOF) { + require.NoError(t, err) + } + require.Equal(t, "hello", string(bs)) + case "test/dir/": + require.Equal(t, hdr.Typeflag, byte(tar.TypeDir)) + case "test/dir/world.txt": + require.Equal(t, hdr.Typeflag, byte(tar.TypeReg)) + bs, err := io.ReadAll(tr) + if err != nil && !xerrors.Is(err, io.EOF) { + require.NoError(t, err) + } + require.Equal(t, "world", string(bs)) + default: + require.Failf(t, "unexpected file in tar", hdr.Name) + } + } +} + +func AssertSampleZipFile(t *testing.T, zipBytes []byte) { + t.Helper() + + zr, err := zip.NewReader(bytes.NewReader(zipBytes), int64(len(zipBytes))) + require.NoError(t, err) + + for _, f := range zr.File { + // Note: ignoring timezones here. + require.Equal(t, ArchiveRefTime(t).UTC(), f.Modified.UTC()) + switch f.Name { + case "test/", "test/dir/": + // directory + case "test/hello.txt": + rc, err := f.Open() + require.NoError(t, err) + bs, err := io.ReadAll(rc) + _ = rc.Close() + require.NoError(t, err) + require.Equal(t, "hello", string(bs)) + case "test/dir/world.txt": + rc, err := f.Open() + require.NoError(t, err) + bs, err := io.ReadAll(rc) + _ = rc.Close() + require.NoError(t, err) + require.Equal(t, "world", string(bs)) + default: + require.Failf(t, "unexpected file in zip", f.Name) + } + } +} + +// archiveRefTime is the Go reference time. The contents of the sample tar and zip files +// in testdata/ all have their modtimes set to the below in some timezone. +func ArchiveRefTime(t *testing.T) time.Time { + locMST, err := time.LoadLocation("MST") + require.NoError(t, err, "failed to load MST timezone") + return time.Date(2006, 1, 2, 3, 4, 5, 0, locMST) +} diff --git a/coderd/testdata/test.tar b/fileszip/filesziptest/testdata/test.tar similarity index 100% rename from coderd/testdata/test.tar rename to fileszip/filesziptest/testdata/test.tar diff --git a/coderd/testdata/test.zip b/fileszip/filesziptest/testdata/test.zip similarity index 100% rename from coderd/testdata/test.zip rename to fileszip/filesziptest/testdata/test.zip From 804d9e8f544c020cb83c4feb9111a508355504ff Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 25 Oct 2024 13:08:57 +0100 Subject: [PATCH 2/4] fixup! chore(coderd): extract fileszip to its own package for reuse --- cli/templatepull_test.go | 4 +- coderd/files_test.go | 4 +- coderd/fileszip.go | 112 --------------------------------------- 3 files changed, 4 insertions(+), 116 deletions(-) delete mode 100644 coderd/fileszip.go diff --git a/cli/templatepull_test.go b/cli/templatepull_test.go index da981f6ad658f..9ba362b8d7773 100644 --- a/cli/templatepull_test.go +++ b/cli/templatepull_test.go @@ -14,9 +14,9 @@ import ( "github.com/stretchr/testify/require" "github.com/coder/coder/v2/cli/clitest" - "github.com/coder/coder/v2/coderd" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/rbac" + "github.com/coder/coder/v2/fileszip" "github.com/coder/coder/v2/provisioner/echo" "github.com/coder/coder/v2/provisionersdk" "github.com/coder/coder/v2/provisionersdk/proto" @@ -95,7 +95,7 @@ func TestTemplatePull_Stdout(t *testing.T) { // Verify .zip format tarReader := tar.NewReader(bytes.NewReader(expected)) - expectedZip, err := coderd.CreateZipFromTar(tarReader) + expectedZip, err := fileszip.CreateZipFromTar(tarReader, int64(len(expected))) require.NoError(t, err) inv, root = clitest.New(t, "templates", "pull", "--zip", template.Name) diff --git a/coderd/files_test.go b/coderd/files_test.go index fef30dfddf439..36fdd58682376 100644 --- a/coderd/files_test.go +++ b/coderd/files_test.go @@ -10,9 +10,9 @@ import ( "github.com/google/uuid" "github.com/stretchr/testify/require" - "github.com/coder/coder/v2/coderd" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/fileszip" "github.com/coder/coder/v2/fileszip/filesziptest" "github.com/coder/coder/v2/testutil" ) @@ -133,7 +133,7 @@ func TestDownload(t *testing.T) { tarball := filesziptest.TestTarFileBytes() tarReader := tar.NewReader(bytes.NewReader(tarball)) - expectedZip, err := coderd.CreateZipFromTar(tarReader) + expectedZip, err := fileszip.CreateZipFromTar(tarReader, 10240) require.NoError(t, err) ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) diff --git a/coderd/fileszip.go b/coderd/fileszip.go deleted file mode 100644 index 389e524746291..0000000000000 --- a/coderd/fileszip.go +++ /dev/null @@ -1,112 +0,0 @@ -package coderd - -import ( - "archive/tar" - "archive/zip" - "bytes" - "errors" - "io" - "log" - "strings" -) - -func CreateTarFromZip(zipReader *zip.Reader) ([]byte, error) { - var tarBuffer bytes.Buffer - err := writeTarArchive(&tarBuffer, zipReader) - if err != nil { - return nil, err - } - return tarBuffer.Bytes(), nil -} - -func writeTarArchive(w io.Writer, zipReader *zip.Reader) error { - tarWriter := tar.NewWriter(w) - defer tarWriter.Close() - - for _, file := range zipReader.File { - err := processFileInZipArchive(file, tarWriter) - if err != nil { - return err - } - } - return nil -} - -func processFileInZipArchive(file *zip.File, tarWriter *tar.Writer) error { - fileReader, err := file.Open() - if err != nil { - return err - } - defer fileReader.Close() - - err = tarWriter.WriteHeader(&tar.Header{ - Name: file.Name, - Size: file.FileInfo().Size(), - Mode: int64(file.Mode()), - ModTime: file.Modified, - // Note: Zip archives do not store ownership information. - Uid: 1000, - Gid: 1000, - }) - if err != nil { - return err - } - - n, err := io.CopyN(tarWriter, fileReader, httpFileMaxBytes) - log.Println(file.Name, n, err) - if errors.Is(err, io.EOF) { - err = nil - } - return err -} - -func CreateZipFromTar(tarReader *tar.Reader) ([]byte, error) { - var zipBuffer bytes.Buffer - err := WriteZipArchive(&zipBuffer, tarReader) - if err != nil { - return nil, err - } - return zipBuffer.Bytes(), nil -} - -func WriteZipArchive(w io.Writer, tarReader *tar.Reader) error { - zipWriter := zip.NewWriter(w) - defer zipWriter.Close() - - for { - tarHeader, err := tarReader.Next() - if errors.Is(err, io.EOF) { - break - } - - if err != nil { - return err - } - - zipHeader, err := zip.FileInfoHeader(tarHeader.FileInfo()) - if err != nil { - return err - } - zipHeader.Name = tarHeader.Name - // Some versions of unzip do not check the mode on a file entry and - // simply assume that entries with a trailing path separator (/) are - // directories, and that everything else is a file. Give them a hint. - if tarHeader.FileInfo().IsDir() && !strings.HasSuffix(tarHeader.Name, "/") { - zipHeader.Name += "/" - } - - zipEntry, err := zipWriter.CreateHeader(zipHeader) - if err != nil { - return err - } - - _, err = io.CopyN(zipEntry, tarReader, httpFileMaxBytes) - if errors.Is(err, io.EOF) { - err = nil - } - if err != nil { - return err - } - } - return nil // don't need to flush as we call `writer.Close()` -} From 33860ae68eeef475e2b9dc2a74b044a6b7430f03 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 25 Oct 2024 14:41:18 +0100 Subject: [PATCH 3/4] address PR comments --- fileszip/fileszip.go => archive/archive.go | 9 ++++--- .../archive_test.go | 24 +++++++++--------- .../archivetest/archivetest.go | 6 ++++- .../archivetest}/testdata/test.tar | Bin .../archivetest}/testdata/test.zip | Bin cli/templatepull_test.go | 4 +-- coderd/files.go | 6 ++--- coderd/files_test.go | 18 ++++++------- 8 files changed, 37 insertions(+), 30 deletions(-) rename fileszip/fileszip.go => archive/archive.go (87%) rename fileszip/fileszip_test.go => archive/archive_test.go (87%) rename fileszip/filesziptest/filesziptest.go => archive/archivetest/archivetest.go (89%) rename {fileszip/filesziptest => archive/archivetest}/testdata/test.tar (100%) rename {fileszip/filesziptest => archive/archivetest}/testdata/test.zip (100%) diff --git a/fileszip/fileszip.go b/archive/archive.go similarity index 87% rename from fileszip/fileszip.go rename to archive/archive.go index a1d3f4c38b8dc..db78b8c700010 100644 --- a/fileszip/fileszip.go +++ b/archive/archive.go @@ -1,4 +1,4 @@ -package fileszip +package archive import ( "archive/tar" @@ -10,6 +10,7 @@ import ( "strings" ) +// CreateTarFromZip converts the given zipReader to a tar archive. func CreateTarFromZip(zipReader *zip.Reader, maxSize int64) ([]byte, error) { var tarBuffer bytes.Buffer err := writeTarArchive(&tarBuffer, zipReader, maxSize) @@ -60,16 +61,18 @@ func processFileInZipArchive(file *zip.File, tarWriter *tar.Writer, maxSize int6 return err } +// CreateZipFromTar converts the given tarReader to a zip archive. func CreateZipFromTar(tarReader *tar.Reader, maxSize int64) ([]byte, error) { var zipBuffer bytes.Buffer - err := WriteZipArchive(&zipBuffer, tarReader, maxSize) + err := WriteZip(&zipBuffer, tarReader, maxSize) if err != nil { return nil, err } return zipBuffer.Bytes(), nil } -func WriteZipArchive(w io.Writer, tarReader *tar.Reader, maxSize int64) error { +// WriteZip writes the given tarReader to w. +func WriteZip(w io.Writer, tarReader *tar.Reader, maxSize int64) error { zipWriter := zip.NewWriter(w) defer zipWriter.Close() diff --git a/fileszip/fileszip_test.go b/archive/archive_test.go similarity index 87% rename from fileszip/fileszip_test.go rename to archive/archive_test.go index 221b3e924471f..c10d103622fa7 100644 --- a/fileszip/fileszip_test.go +++ b/archive/archive_test.go @@ -1,4 +1,4 @@ -package fileszip_test +package archive_test import ( "archive/tar" @@ -15,8 +15,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/coder/coder/v2/fileszip" - "github.com/coder/coder/v2/fileszip/filesziptest" + "github.com/coder/coder/v2/archive" + "github.com/coder/coder/v2/archive/archivetest" "github.com/coder/coder/v2/testutil" ) @@ -28,17 +28,17 @@ func TestCreateTarFromZip(t *testing.T) { // Read a zip file we prepared earlier ctx := testutil.Context(t, testutil.WaitShort) - zipBytes := filesziptest.TestZipFileBytes() + zipBytes := archivetest.TestZipFileBytes() // Assert invariant - filesziptest.AssertSampleZipFile(t, zipBytes) + archivetest.AssertSampleZipFile(t, zipBytes) zr, err := zip.NewReader(bytes.NewReader(zipBytes), int64(len(zipBytes))) require.NoError(t, err, "failed to parse sample zip file") - tarBytes, err := fileszip.CreateTarFromZip(zr, 10240) + tarBytes, err := archive.CreateTarFromZip(zr, int64(len(zipBytes))) require.NoError(t, err, "failed to convert zip to tar") - filesziptest.AssertSampleTarFile(t, tarBytes) + archivetest.AssertSampleTarFile(t, tarBytes) tempDir := t.TempDir() tempFilePath := filepath.Join(tempDir, "test.tar") @@ -57,13 +57,13 @@ func TestCreateZipFromTar(t *testing.T) { } t.Run("OK", func(t *testing.T) { t.Parallel() - tarBytes := filesziptest.TestTarFileBytes() + tarBytes := archivetest.TestTarFileBytes() tr := tar.NewReader(bytes.NewReader(tarBytes)) - zipBytes, err := fileszip.CreateZipFromTar(tr, 10240) + zipBytes, err := archive.CreateZipFromTar(tr, int64(len(tarBytes))) require.NoError(t, err) - filesziptest.AssertSampleZipFile(t, zipBytes) + archivetest.AssertSampleZipFile(t, zipBytes) tempDir := t.TempDir() tempFilePath := filepath.Join(tempDir, "test.zip") @@ -95,7 +95,7 @@ func TestCreateZipFromTar(t *testing.T) { // When: we convert this to a zip tr := tar.NewReader(&tarBytes) - zipBytes, err := fileszip.CreateZipFromTar(tr, 10240) + zipBytes, err := archive.CreateZipFromTar(tr, int64(tarBytes.Len())) require.NoError(t, err) // Then: the resulting zip should contain a corresponding directory @@ -129,7 +129,7 @@ func assertExtractedFiles(t *testing.T, dir string, checkModePerm bool) { if checkModePerm { assert.Equal(t, fs.ModePerm&0o755, stat.Mode().Perm(), "expected mode 0755 on directory") } - assert.Equal(t, filesziptest.ArchiveRefTime(t).UTC(), stat.ModTime().UTC(), "unexpected modtime of %q", path) + assert.Equal(t, archivetest.ArchiveRefTime(t).UTC(), stat.ModTime().UTC(), "unexpected modtime of %q", path) case "/test/hello.txt": stat, err := os.Stat(path) assert.NoError(t, err, "failed to stat path %q", path) diff --git a/fileszip/filesziptest/filesziptest.go b/archive/archivetest/archivetest.go similarity index 89% rename from fileszip/filesziptest/filesziptest.go rename to archive/archivetest/archivetest.go index df4902459ef49..2daa6fad4ae9b 100644 --- a/fileszip/filesziptest/filesziptest.go +++ b/archive/archivetest/archivetest.go @@ -1,4 +1,4 @@ -package filesziptest +package archivetest import ( "archive/tar" @@ -19,14 +19,17 @@ var testTarFileBytes []byte //go:embed testdata/test.zip var testZipFileBytes []byte +// TestTarFileBytes returns the content of testdata/test.tar func TestTarFileBytes() []byte { return append([]byte{}, testTarFileBytes...) } +// TestZipFileBytes returns the content of testdata/test.zip func TestZipFileBytes() []byte { return append([]byte{}, testZipFileBytes...) } +// AssertSampleTarfile compares the content of tarBytes against testdata/test.tar. func AssertSampleTarFile(t *testing.T, tarBytes []byte) { t.Helper() @@ -68,6 +71,7 @@ func AssertSampleTarFile(t *testing.T, tarBytes []byte) { } } +// AssertSampleZipFile compares the content of zipBytes against testdata/test.zip. func AssertSampleZipFile(t *testing.T, zipBytes []byte) { t.Helper() diff --git a/fileszip/filesziptest/testdata/test.tar b/archive/archivetest/testdata/test.tar similarity index 100% rename from fileszip/filesziptest/testdata/test.tar rename to archive/archivetest/testdata/test.tar diff --git a/fileszip/filesziptest/testdata/test.zip b/archive/archivetest/testdata/test.zip similarity index 100% rename from fileszip/filesziptest/testdata/test.zip rename to archive/archivetest/testdata/test.zip diff --git a/cli/templatepull_test.go b/cli/templatepull_test.go index 9ba362b8d7773..e1c10f350b89b 100644 --- a/cli/templatepull_test.go +++ b/cli/templatepull_test.go @@ -13,10 +13,10 @@ import ( "github.com/google/uuid" "github.com/stretchr/testify/require" + "github.com/coder/coder/v2/archive" "github.com/coder/coder/v2/cli/clitest" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/rbac" - "github.com/coder/coder/v2/fileszip" "github.com/coder/coder/v2/provisioner/echo" "github.com/coder/coder/v2/provisionersdk" "github.com/coder/coder/v2/provisionersdk/proto" @@ -95,7 +95,7 @@ func TestTemplatePull_Stdout(t *testing.T) { // Verify .zip format tarReader := tar.NewReader(bytes.NewReader(expected)) - expectedZip, err := fileszip.CreateZipFromTar(tarReader, int64(len(expected))) + expectedZip, err := archive.CreateZipFromTar(tarReader, int64(len(expected))) require.NoError(t, err) inv, root = clitest.New(t, "templates", "pull", "--zip", template.Name) diff --git a/coderd/files.go b/coderd/files.go index 8e410fc5d1494..ce536c93cab81 100644 --- a/coderd/files.go +++ b/coderd/files.go @@ -16,12 +16,12 @@ import ( "github.com/google/uuid" "cdr.dev/slog" + "github.com/coder/coder/v2/archive" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/coderd/httpmw" "github.com/coder/coder/v2/codersdk" - "github.com/coder/coder/v2/fileszip" ) const ( @@ -76,7 +76,7 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) { return } - data, err = fileszip.CreateTarFromZip(zipReader, httpFileMaxBytes) + data, err = archive.CreateTarFromZip(zipReader, httpFileMaxBytes) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error processing .zip archive.", @@ -182,7 +182,7 @@ func (api *API) fileByID(rw http.ResponseWriter, r *http.Request) { rw.Header().Set("Content-Type", codersdk.ContentTypeZip) rw.WriteHeader(http.StatusOK) - err = fileszip.WriteZipArchive(rw, tar.NewReader(bytes.NewReader(file.Data)), httpFileMaxBytes) + err = archive.WriteZip(rw, tar.NewReader(bytes.NewReader(file.Data)), httpFileMaxBytes) if err != nil { api.Logger.Error(ctx, "invalid .zip archive", slog.F("file_id", fileID), slog.F("mimetype", file.Mimetype), slog.Error(err)) } diff --git a/coderd/files_test.go b/coderd/files_test.go index 36fdd58682376..f2dd788e3a6dd 100644 --- a/coderd/files_test.go +++ b/coderd/files_test.go @@ -10,10 +10,10 @@ import ( "github.com/google/uuid" "github.com/stretchr/testify/require" + "github.com/coder/coder/v2/archive" + "github.com/coder/coder/v2/archive/archivetest" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/codersdk" - "github.com/coder/coder/v2/fileszip" - "github.com/coder/coder/v2/fileszip/filesziptest" "github.com/coder/coder/v2/testutil" ) @@ -84,7 +84,7 @@ func TestDownload(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - tarball := filesziptest.TestTarFileBytes() + tarball := archivetest.TestTarFileBytes() // when resp, err := client.Upload(ctx, codersdk.ContentTypeTar, bytes.NewReader(tarball)) @@ -96,7 +96,7 @@ func TestDownload(t *testing.T) { require.Len(t, data, len(tarball)) require.Equal(t, codersdk.ContentTypeTar, contentType) require.Equal(t, tarball, data) - filesziptest.AssertSampleTarFile(t, data) + archivetest.AssertSampleTarFile(t, data) }) t.Run("InsertZip_DownloadTar", func(t *testing.T) { @@ -105,7 +105,7 @@ func TestDownload(t *testing.T) { _ = coderdtest.CreateFirstUser(t, client) // given - zipContent := filesziptest.TestZipFileBytes() + zipContent := archivetest.TestZipFileBytes() ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() @@ -121,7 +121,7 @@ func TestDownload(t *testing.T) { // Note: creating a zip from a tar will result in some loss of information // as zip files do not store UNIX user:group data. - filesziptest.AssertSampleTarFile(t, data) + archivetest.AssertSampleTarFile(t, data) }) t.Run("InsertTar_DownloadZip", func(t *testing.T) { @@ -130,10 +130,10 @@ func TestDownload(t *testing.T) { _ = coderdtest.CreateFirstUser(t, client) // given - tarball := filesziptest.TestTarFileBytes() + tarball := archivetest.TestTarFileBytes() tarReader := tar.NewReader(bytes.NewReader(tarball)) - expectedZip, err := fileszip.CreateZipFromTar(tarReader, 10240) + expectedZip, err := archive.CreateZipFromTar(tarReader, 10240) require.NoError(t, err) ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) @@ -148,6 +148,6 @@ func TestDownload(t *testing.T) { // then require.Equal(t, codersdk.ContentTypeZip, contentType) require.Equal(t, expectedZip, data) - filesziptest.AssertSampleZipFile(t, data) + archivetest.AssertSampleZipFile(t, data) }) } From 2c1d3d60f32820eb99040d1f3438034f80b39ce8 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 25 Oct 2024 14:47:30 +0100 Subject: [PATCH 4/4] export HTTPFileMaxBytes, reference in cli test --- cli/templatepull_test.go | 3 ++- coderd/files.go | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/cli/templatepull_test.go b/cli/templatepull_test.go index e1c10f350b89b..99f23d12923cd 100644 --- a/cli/templatepull_test.go +++ b/cli/templatepull_test.go @@ -15,6 +15,7 @@ import ( "github.com/coder/coder/v2/archive" "github.com/coder/coder/v2/cli/clitest" + "github.com/coder/coder/v2/coderd" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/provisioner/echo" @@ -95,7 +96,7 @@ func TestTemplatePull_Stdout(t *testing.T) { // Verify .zip format tarReader := tar.NewReader(bytes.NewReader(expected)) - expectedZip, err := archive.CreateZipFromTar(tarReader, int64(len(expected))) + expectedZip, err := archive.CreateZipFromTar(tarReader, coderd.HTTPFileMaxBytes) require.NoError(t, err) inv, root = clitest.New(t, "templates", "pull", "--zip", template.Name) diff --git a/coderd/files.go b/coderd/files.go index ce536c93cab81..bf1885da1eee9 100644 --- a/coderd/files.go +++ b/coderd/files.go @@ -28,7 +28,7 @@ const ( tarMimeType = "application/x-tar" zipMimeType = "application/zip" - httpFileMaxBytes = 10 * (10 << 20) + HTTPFileMaxBytes = 10 * (10 << 20) ) // @Summary Upload file @@ -56,7 +56,7 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) { return } - r.Body = http.MaxBytesReader(rw, r.Body, httpFileMaxBytes) + r.Body = http.MaxBytesReader(rw, r.Body, HTTPFileMaxBytes) data, err := io.ReadAll(r.Body) if err != nil { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ @@ -76,7 +76,7 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) { return } - data, err = archive.CreateTarFromZip(zipReader, httpFileMaxBytes) + data, err = archive.CreateTarFromZip(zipReader, HTTPFileMaxBytes) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error processing .zip archive.", @@ -182,7 +182,7 @@ func (api *API) fileByID(rw http.ResponseWriter, r *http.Request) { rw.Header().Set("Content-Type", codersdk.ContentTypeZip) rw.WriteHeader(http.StatusOK) - err = archive.WriteZip(rw, tar.NewReader(bytes.NewReader(file.Data)), httpFileMaxBytes) + err = archive.WriteZip(rw, tar.NewReader(bytes.NewReader(file.Data)), HTTPFileMaxBytes) if err != nil { api.Logger.Error(ctx, "invalid .zip archive", slog.F("file_id", fileID), slog.F("mimetype", file.Mimetype), slog.Error(err)) }