From a424f6d4f8224ca7d5db1a27e1db46a5dc47f30b Mon Sep 17 00:00:00 2001 From: Giteabot Date: Mon, 24 Jul 2023 10:56:13 -0400 Subject: [PATCH] Fix handling of Debian files with trailing slash (#26087) (#26098) Backport #26087 by @KN4CK3R Fixes #26022 - Fix handling of files with trailing slash - Fix handling of duplicate package file errors - Added test for both Co-authored-by: KN4CK3R --- modules/packages/debian/metadata.go | 4 +- modules/packages/debian/metadata_test.go | 104 ++++++++++-------- routers/api/packages/debian/debian.go | 2 +- tests/integration/api_packages_debian_test.go | 4 + 4 files changed, 67 insertions(+), 47 deletions(-) diff --git a/modules/packages/debian/metadata.go b/modules/packages/debian/metadata.go index dee524c8ff..bb77f7524b 100644 --- a/modules/packages/debian/metadata.go +++ b/modules/packages/debian/metadata.go @@ -80,7 +80,9 @@ func ParsePackage(r io.Reader) (*Package, error) { if strings.HasPrefix(hd.Name, controlTar) { var inner io.Reader - switch hd.Name[len(controlTar):] { + // https://man7.org/linux/man-pages/man5/deb-split.5.html#FORMAT + // The file names might contain a trailing slash (since dpkg 1.15.6). + switch strings.TrimSuffix(hd.Name[len(controlTar):], "/") { case "": inner = arr case ".gz": diff --git a/modules/packages/debian/metadata_test.go b/modules/packages/debian/metadata_test.go index 69fd51ea79..26c2a6fc68 100644 --- a/modules/packages/debian/metadata_test.go +++ b/modules/packages/debian/metadata_test.go @@ -69,59 +69,73 @@ func TestParsePackage(t *testing.T) { tw.Write([]byte("Package: gitea\nVersion: 1.0.0\nArchitecture: amd64\n")) tw.Close() - t.Run("None", func(t *testing.T) { - data := createArchive(map[string][]byte{"control.tar": buf.Bytes()}) + cases := []struct { + Extension string + WriterFactory func(io.Writer) io.WriteCloser + }{ + { + Extension: "", + WriterFactory: func(w io.Writer) io.WriteCloser { + return nopCloser{w} + }, + }, + { + Extension: ".gz", + WriterFactory: func(w io.Writer) io.WriteCloser { + return gzip.NewWriter(w) + }, + }, + { + Extension: ".xz", + WriterFactory: func(w io.Writer) io.WriteCloser { + xw, _ := xz.NewWriter(w) + return xw + }, + }, + { + Extension: ".zst", + WriterFactory: func(w io.Writer) io.WriteCloser { + zw, _ := zstd.NewWriter(w) + return zw + }, + }, + } - p, err := ParsePackage(data) - assert.NotNil(t, p) - assert.NoError(t, err) - assert.Equal(t, "gitea", p.Name) - }) + for _, c := range cases { + t.Run(c.Extension, func(t *testing.T) { + var cbuf bytes.Buffer + w := c.WriterFactory(&cbuf) + w.Write(buf.Bytes()) + w.Close() - t.Run("gz", func(t *testing.T) { - var zbuf bytes.Buffer - zw := gzip.NewWriter(&zbuf) - zw.Write(buf.Bytes()) - zw.Close() + data := createArchive(map[string][]byte{"control.tar" + c.Extension: cbuf.Bytes()}) - data := createArchive(map[string][]byte{"control.tar.gz": zbuf.Bytes()}) + p, err := ParsePackage(data) + assert.NotNil(t, p) + assert.NoError(t, err) + assert.Equal(t, "gitea", p.Name) - p, err := ParsePackage(data) - assert.NotNil(t, p) - assert.NoError(t, err) - assert.Equal(t, "gitea", p.Name) - }) + t.Run("TrailingSlash", func(t *testing.T) { + data := createArchive(map[string][]byte{"control.tar" + c.Extension + "/": cbuf.Bytes()}) - t.Run("xz", func(t *testing.T) { - var xbuf bytes.Buffer - xw, _ := xz.NewWriter(&xbuf) - xw.Write(buf.Bytes()) - xw.Close() - - data := createArchive(map[string][]byte{"control.tar.xz": xbuf.Bytes()}) - - p, err := ParsePackage(data) - assert.NotNil(t, p) - assert.NoError(t, err) - assert.Equal(t, "gitea", p.Name) - }) - - t.Run("zst", func(t *testing.T) { - var zbuf bytes.Buffer - zw, _ := zstd.NewWriter(&zbuf) - zw.Write(buf.Bytes()) - zw.Close() - - data := createArchive(map[string][]byte{"control.tar.zst": zbuf.Bytes()}) - - p, err := ParsePackage(data) - assert.NotNil(t, p) - assert.NoError(t, err) - assert.Equal(t, "gitea", p.Name) - }) + p, err := ParsePackage(data) + assert.NotNil(t, p) + assert.NoError(t, err) + assert.Equal(t, "gitea", p.Name) + }) + }) + } }) } +type nopCloser struct { + io.Writer +} + +func (nopCloser) Close() error { + return nil +} + func TestParseControlFile(t *testing.T) { buildContent := func(name, version, architecture string) *bytes.Buffer { var buf bytes.Buffer diff --git a/routers/api/packages/debian/debian.go b/routers/api/packages/debian/debian.go index 1b3d76ba25..dd0db019d4 100644 --- a/routers/api/packages/debian/debian.go +++ b/routers/api/packages/debian/debian.go @@ -195,7 +195,7 @@ func UploadPackageFile(ctx *context.Context) { ) if err != nil { switch err { - case packages_model.ErrDuplicatePackageVersion: + case packages_model.ErrDuplicatePackageVersion, packages_model.ErrDuplicatePackageFile: apiError(ctx, http.StatusBadRequest, err) case packages_service.ErrQuotaTotalCount, packages_service.ErrQuotaTypeSize, packages_service.ErrQuotaTotalSize: apiError(ctx, http.StatusForbidden, err) diff --git a/tests/integration/api_packages_debian_test.go b/tests/integration/api_packages_debian_test.go index 3e25acd8cf..2d92b93159 100644 --- a/tests/integration/api_packages_debian_test.go +++ b/tests/integration/api_packages_debian_test.go @@ -144,6 +144,10 @@ func TestPackageDebian(t *testing.T) { } return seen }) + + req = NewRequestWithBody(t, "PUT", uploadURL, createArchive(packageName, packageVersion, architecture)) + AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, http.StatusBadRequest) }) t.Run("Download", func(t *testing.T) {