From 6aa30af724b73c698b87c24fd5bb7d71b3c37abe Mon Sep 17 00:00:00 2001
From: KN4CK3R <admin@oldschoolhack.me>
Date: Mon, 24 Jul 2023 16:19:44 +0200
Subject: [PATCH] Fix handling of Debian files with trailing slash (#26087)

Fixes #26022

- Fix handling of files with trailing slash
- Fix handling of duplicate package file errors
- Added test for both
---
 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 f7270e0ae0..a6da1a11a8 100644
--- a/routers/api/packages/debian/debian.go
+++ b/routers/api/packages/debian/debian.go
@@ -187,7 +187,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) {