Make composer registry support tar.gz and tar.bz2 and fix bugs (#35958)

Fix #35957

modules/packages/composer/metadata.go

@@ -4,8 +4,13 @@
 package composer
 
 import (
+	"archive/tar"
 	"archive/zip"
+	"compress/bzip2"
+	"compress/gzip"
+	"errors"
 	"io"
+	"io/fs"
 	"path"
 	"regexp"
 	"strings"
@@ -29,8 +34,10 @@ var (
 	ErrInvalidVersion = util.NewInvalidArgumentErrorf("package version is invalid")
 )
 
-// Package represents a Composer package
-type Package struct {
+// PackageInfo represents Composer package info
+type PackageInfo struct {
+	Filename string
+
 	Name     string
 	Version  string
 	Type     string
@@ -44,7 +51,7 @@ type Metadata struct {
 	Description string            `json:"description,omitempty"`
 	Readme      string            `json:"readme,omitempty"`
 	Keywords    []string          `json:"keywords,omitempty"`
-	Comments    Comments          `json:"_comments,omitempty"`
+	Comments    Comments          `json:"_comment,omitempty"`
 	Homepage    string            `json:"homepage,omitempty"`
 	License     Licenses          `json:"license,omitempty"`
 	Authors     []Author          `json:"authors,omitempty"`
@@ -75,7 +82,7 @@ func (l *Licenses) UnmarshalJSON(data []byte) error {
 		if err := json.Unmarshal(data, &values); err != nil {
 			return err
 		}
-		*l = Licenses(values)
+		*l = values
 	}
 	return nil
 }
@@ -97,7 +104,7 @@ func (c *Comments) UnmarshalJSON(data []byte) error {
 		if err := json.Unmarshal(data, &values); err != nil {
 			return err
 		}
-		*c = Comments(values)
+		*c = values
 	}
 	return nil
 }
@@ -111,39 +118,121 @@ type Author struct {
 
 var nameMatch = regexp.MustCompile(`\A[a-z0-9]([_\.-]?[a-z0-9]+)*/[a-z0-9](([_\.]?|-{0,2})[a-z0-9]+)*\z`)
 
-// ParsePackage parses the metadata of a Composer package file
-func ParsePackage(r io.ReaderAt, size int64) (*Package, error) {
-	archive, err := zip.NewReader(r, size)
+type ReadSeekAt interface {
+	io.Reader
+	io.ReaderAt
+	io.Seeker
+	Size() int64
+}
+
+func readPackageFileZip(r ReadSeekAt, filename string, limit int) ([]byte, error) {
+	archive, err := zip.NewReader(r, r.Size())
 	if err != nil {
 		return nil, err
 	}
 
 	for _, file := range archive.File {
-		if strings.Count(file.Name, "/") > 1 {
-			continue
-		}
-		if strings.HasSuffix(strings.ToLower(file.Name), "composer.json") {
+		filePath := path.Clean(file.Name)
+		if util.AsciiEqualFold(filePath, filename) {
 			f, err := archive.Open(file.Name)
 			if err != nil {
 				return nil, err
 			}
 			defer f.Close()
 
-			return ParseComposerFile(archive, path.Dir(file.Name), f)
+			return util.ReadWithLimit(f, limit)
+		}
+	}
+	return nil, fs.ErrNotExist
+}
+
+func readPackageFileTar(r io.Reader, filename string, limit int) ([]byte, error) {
+	tarReader := tar.NewReader(r)
+	for {
+		header, err := tarReader.Next()
+		if err == io.EOF {
+			break
+		} else if err != nil {
+			return nil, err
+		}
+
+		filePath := path.Clean(header.Name)
+		if util.AsciiEqualFold(filePath, filename) {
+			return util.ReadWithLimit(tarReader, limit)
 		}
 	}
-	return nil, ErrMissingComposerFile
+	return nil, fs.ErrNotExist
 }
 
-// ParseComposerFile parses a composer.json file to retrieve the metadata of a Composer package
-func ParseComposerFile(archive *zip.Reader, pathPrefix string, r io.Reader) (*Package, error) {
+const (
+	pkgExtZip    = ".zip"
+	pkgExtTarGz  = ".tar.gz"
+	pkgExtTarBz2 = ".tar.bz2"
+)
+
+func detectPackageExtName(r ReadSeekAt) (string, error) {
+	headBytes := make([]byte, 4)
+	_, err := r.ReadAt(headBytes, 0)
+	if err != nil {
+		return "", err
+	}
+	_, err = r.Seek(0, io.SeekStart)
+	if err != nil {
+		return "", err
+	}
+	switch {
+	case headBytes[0] == 'P' && headBytes[1] == 'K':
+		return pkgExtZip, nil
+	case string(headBytes[:3]) == "BZh":
+		return pkgExtTarBz2, nil
+	case headBytes[0] == 0x1f && headBytes[1] == 0x8b:
+		return pkgExtTarGz, nil
+	}
+	return "", util.NewInvalidArgumentErrorf("not a valid package file")
+}
+
+func readPackageFile(pkgExt string, r ReadSeekAt, filename string, limit int) ([]byte, error) {
+	_, err := r.Seek(0, io.SeekStart)
+	if err != nil {
+		return nil, err
+	}
+
+	switch pkgExt {
+	case pkgExtZip:
+		return readPackageFileZip(r, filename, limit)
+	case pkgExtTarBz2:
+		bzip2Reader := bzip2.NewReader(r)
+		return readPackageFileTar(bzip2Reader, filename, limit)
+	case pkgExtTarGz:
+		gzReader, err := gzip.NewReader(r)
+		if err != nil {
+			return nil, err
+		}
+		return readPackageFileTar(gzReader, filename, limit)
+	}
+	return nil, util.NewInvalidArgumentErrorf("not a valid package file")
+}
+
+// ParsePackage parses the metadata of a Composer package file
+func ParsePackage(r ReadSeekAt, optVersion ...string) (*PackageInfo, error) {
+	pkgExt, err := detectPackageExtName(r)
+	if err != nil {
+		return nil, err
+	}
+	dataComposerJSON, err := readPackageFile(pkgExt, r, "composer.json", 10*1024*1024)
+	if errors.Is(err, fs.ErrNotExist) {
+		return nil, ErrMissingComposerFile
+	} else if err != nil {
+		return nil, err
+	}
+
 	var cj struct {
 		Name    string `json:"name"`
 		Version string `json:"version"`
 		Type    string `json:"type"`
 		Metadata
 	}
-	if err := json.NewDecoder(r).Decode(&cj); err != nil {
+	if err := json.Unmarshal(dataComposerJSON, &cj); err != nil {
 		return nil, err
 	}
 
@@ -151,6 +240,9 @@ func ParseComposerFile(archive *zip.Reader, pathPrefix string, r io.Reader) (*Pa
 		return nil, ErrInvalidName
 	}
 
+	if cj.Version == "" {
+		cj.Version = util.OptionalArg(optVersion)
+	}
 	if cj.Version != "" {
 		if _, err := version.NewSemver(cj.Version); err != nil {
 			return nil, ErrInvalidVersion
@@ -168,17 +260,23 @@ func ParseComposerFile(archive *zip.Reader, pathPrefix string, r io.Reader) (*Pa
 	if cj.Readme == "" {
 		cj.Readme = "README.md"
 	}
-	f, err := archive.Open(path.Join(pathPrefix, cj.Readme))
-	if err == nil {
-		// 10kb limit for readme content
-		buf, _ := io.ReadAll(io.LimitReader(f, 10*1024))
-		cj.Readme = string(buf)
-		_ = f.Close()
-	} else {
+	dataReadmeMd, _ := readPackageFile(pkgExt, r, cj.Readme, 10*1024)
+
+	// FIXME: legacy problem, the "Readme" field is abused, it should always be the path to the readme file
+	if len(dataReadmeMd) == 0 {
 		cj.Readme = ""
+	} else {
+		cj.Readme = string(dataReadmeMd)
 	}
 
-	return &Package{
+	// FIXME: legacy format: strings.ToLower(fmt.Sprintf("%s.%s.zip", strings.ReplaceAll(cp.Name, "/", "-"), cp.Version)), doesn't read good
+	pkgFilename := strings.ReplaceAll(cj.Name, "/", "-")
+	if cj.Version != "" {
+		pkgFilename += "." + cj.Version
+	}
+	pkgFilename += pkgExt
+	return &PackageInfo{
+		Filename: pkgFilename,
 		Name:     cj.Name,
 		Version:  cj.Version,
 		Type:     cj.Type,

modules/packages/composer/metadata_test.go

@@ -4,14 +4,19 @@
 package composer
 
 import (
+	"archive/tar"
 	"archive/zip"
 	"bytes"
+	"compress/gzip"
+	"io"
 	"strings"
 	"testing"
 
 	"code.gitea.io/gitea/modules/json"
 
+	"github.com/dsnet/compress/bzip2"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 const (
@@ -26,8 +31,10 @@ const (
 	license     = "MIT"
 )
 
-const composerContent = `{
+func buildComposerContent(version string) string {
+	return `{
     "name": "` + name + `",
+		"version": "` + version + `",
     "description": "` + description + `",
     "type": "` + packageType + `",
     "license": "` + license + `",
@@ -44,8 +51,9 @@ const composerContent = `{
     "require": {
         "php": ">=7.2 || ^8.0"
     },
-    "_comments": "` + comments + `"
+    "_comment": "` + comments + `"
 }`
+}
 
 func TestLicenseUnmarshal(t *testing.T) {
 	var l Licenses
@@ -73,16 +81,34 @@ func TestParsePackage(t *testing.T) {
 		archive := zip.NewWriter(&buf)
 		for name, content := range files {
 			w, _ := archive.Create(name)
-			w.Write([]byte(content))
+			_, _ = w.Write([]byte(content))
+		}
+		_ = archive.Close()
+		return buf.Bytes()
+	}
+
+	createArchiveTar := func(comp func(io.Writer) io.WriteCloser, files map[string]string) []byte {
+		var buf bytes.Buffer
+		w := comp(&buf)
+		archive := tar.NewWriter(w)
+		for name, content := range files {
+			hdr := &tar.Header{
+				Name: name,
+				Mode: 0o600,
+				Size: int64(len(content)),
+			}
+			_ = archive.WriteHeader(hdr)
+			_, _ = archive.Write([]byte(content))
 		}
-		archive.Close()
+		_ = w.Close()
+		_ = archive.Close()
 		return buf.Bytes()
 	}
 
 	t.Run("MissingComposerFile", func(t *testing.T) {
 		data := createArchive(map[string]string{"dummy.txt": ""})
 
-		cp, err := ParsePackage(bytes.NewReader(data), int64(len(data)))
+		cp, err := ParsePackage(bytes.NewReader(data))
 		assert.Nil(t, cp)
 		assert.ErrorIs(t, err, ErrMissingComposerFile)
 	})
@@ -90,7 +116,7 @@ func TestParsePackage(t *testing.T) {
 	t.Run("MissingComposerFileInRoot", func(t *testing.T) {
 		data := createArchive(map[string]string{"sub/sub/composer.json": ""})
 
-		cp, err := ParsePackage(bytes.NewReader(data), int64(len(data)))
+		cp, err := ParsePackage(bytes.NewReader(data))
 		assert.Nil(t, cp)
 		assert.ErrorIs(t, err, ErrMissingComposerFile)
 	})
@@ -98,7 +124,7 @@ func TestParsePackage(t *testing.T) {
 	t.Run("InvalidComposerFile", func(t *testing.T) {
 		data := createArchive(map[string]string{"composer.json": ""})
 
-		cp, err := ParsePackage(bytes.NewReader(data), int64(len(data)))
+		cp, err := ParsePackage(bytes.NewReader(data))
 		assert.Nil(t, cp)
 		assert.Error(t, err)
 	})
@@ -106,7 +132,7 @@ func TestParsePackage(t *testing.T) {
 	t.Run("InvalidPackageName", func(t *testing.T) {
 		data := createArchive(map[string]string{"composer.json": "{}"})
 
-		cp, err := ParsePackage(bytes.NewReader(data), int64(len(data)))
+		cp, err := ParsePackage(bytes.NewReader(data))
 		assert.Nil(t, cp)
 		assert.ErrorIs(t, err, ErrInvalidName)
 	})
@@ -114,7 +140,7 @@ func TestParsePackage(t *testing.T) {
 	t.Run("InvalidPackageVersion", func(t *testing.T) {
 		data := createArchive(map[string]string{"composer.json": `{"name": "gitea/composer-package", "version": "1.a.3"}`})
 
-		cp, err := ParsePackage(bytes.NewReader(data), int64(len(data)))
+		cp, err := ParsePackage(bytes.NewReader(data))
 		assert.Nil(t, cp)
 		assert.ErrorIs(t, err, ErrInvalidVersion)
 	})
@@ -122,22 +148,21 @@ func TestParsePackage(t *testing.T) {
 	t.Run("InvalidReadmePath", func(t *testing.T) {
 		data := createArchive(map[string]string{"composer.json": `{"name": "gitea/composer-package", "readme": "sub/README.md"}`})
 
-		cp, err := ParsePackage(bytes.NewReader(data), int64(len(data)))
+		cp, err := ParsePackage(bytes.NewReader(data))
 		assert.NoError(t, err)
 		assert.NotNil(t, cp)
 
 		assert.Empty(t, cp.Metadata.Readme)
 	})
 
-	t.Run("Valid", func(t *testing.T) {
-		data := createArchive(map[string]string{"composer.json": composerContent, "README.md": readme})
-
-		cp, err := ParsePackage(bytes.NewReader(data), int64(len(data)))
-		assert.NoError(t, err)
+	assertValidPackage := func(t *testing.T, data []byte, version, filename string) {
+		cp, err := ParsePackage(bytes.NewReader(data))
+		require.NoError(t, err)
 		assert.NotNil(t, cp)
 
+		assert.Equal(t, filename, cp.Filename)
 		assert.Equal(t, name, cp.Name)
-		assert.Empty(t, cp.Version)
+		assert.Equal(t, version, cp.Version)
 		assert.Equal(t, description, cp.Metadata.Description)
 		assert.Equal(t, readme, cp.Metadata.Readme)
 		assert.Len(t, cp.Metadata.Comments, 1)
@@ -149,5 +174,25 @@ func TestParsePackage(t *testing.T) {
 		assert.Equal(t, packageType, cp.Type)
 		assert.Len(t, cp.Metadata.License, 1)
 		assert.Equal(t, license, cp.Metadata.License[0])
+	}
+
+	t.Run("ValidZip", func(t *testing.T) {
+		data := createArchive(map[string]string{"composer.json": buildComposerContent(""), "README.md": readme})
+		assertValidPackage(t, data, "", "gitea-composer-package.zip")
+	})
+
+	t.Run("ValidTarBz2", func(t *testing.T) {
+		data := createArchiveTar(func(w io.Writer) io.WriteCloser {
+			bz2Writer, _ := bzip2.NewWriter(w, nil)
+			return bz2Writer
+		}, map[string]string{"composer.json": buildComposerContent("1.0"), "README.md": readme})
+		assertValidPackage(t, data, "1.0", "gitea-composer-package.1.0.tar.bz2")
+	})
+
+	t.Run("ValidTarGz", func(t *testing.T) {
+		data := createArchiveTar(func(w io.Writer) io.WriteCloser {
+			return gzip.NewWriter(w)
+		}, map[string]string{"composer.json": buildComposerContent(""), "README.md": readme})
+		assertValidPackage(t, data, "", "gitea-composer-package.tar.gz")
 	})
 }

routers/api/packages/composer/composer.go

@@ -5,12 +5,10 @@ package composer
 
 import (
 	"errors"
-	"fmt"
 	"io"
 	"net/http"
 	"net/url"
 	"strconv"
-	"strings"
 
 	"code.gitea.io/gitea/models/db"
 	packages_model "code.gitea.io/gitea/models/packages"
@@ -23,8 +21,6 @@ import (
 	"code.gitea.io/gitea/services/context"
 	"code.gitea.io/gitea/services/convert"
 	packages_service "code.gitea.io/gitea/services/packages"
-
-	"github.com/hashicorp/go-version"
 )
 
 func apiError(ctx *context.Context, status int, obj any) {
@@ -193,7 +189,7 @@ func UploadPackage(ctx *context.Context) {
 	}
 	defer buf.Close()
 
-	cp, err := composer_module.ParsePackage(buf, buf.Size())
+	cp, err := composer_module.ParsePackage(buf, ctx.FormTrim("version"))
 	if err != nil {
 		if errors.Is(err, util.ErrInvalidArgument) {
 			apiError(ctx, http.StatusBadRequest, err)
@@ -209,12 +205,9 @@ func UploadPackage(ctx *context.Context) {
 	}
 
 	if cp.Version == "" {
-		v, err := version.NewVersion(ctx.FormTrim("version"))
-		if err != nil {
-			apiError(ctx, http.StatusBadRequest, composer_module.ErrInvalidVersion)
-			return
-		}
-		cp.Version = v.String()
+		// the version should be either set in the "composer.json", or as a query parameter "?version=xxx"
+		apiError(ctx, http.StatusBadRequest, composer_module.ErrInvalidVersion)
+		return
 	}
 
 	_, _, err = packages_service.CreatePackageAndAddFile(
@@ -235,7 +228,7 @@ func UploadPackage(ctx *context.Context) {
 		},
 		&packages_service.PackageFileCreationInfo{
 			PackageFileInfo: packages_service.PackageFileInfo{
-				Filename: strings.ToLower(fmt.Sprintf("%s.%s.zip", strings.ReplaceAll(cp.Name, "/", "-"), cp.Version)),
+				Filename: cp.Filename,
 			},
 			Creator: ctx.Doer,
 			Data:    buf,