Swift files can be passed either as file or as form value (#34068) (#34236)

Backport #34068 by wgr1984

Fix #33990

Co-authored-by: Wolfgang Reithmeier <[email protected]>
Co-authored-by: wxiaoguang <[email protected]>

Author: 3 people

models/packages/package_version.go

@@ -279,9 +279,7 @@ func (opts *PackageSearchOptions) configureOrderBy(e db.Engine) {
 	default:
 		e.Desc("package_version.created_unix")
 	}
-
-	// Sort by id for stable order with duplicates in the other field
-	e.Asc("package_version.id")
+	e.Desc("package_version.id") // Sort by id for stable order with duplicates in the other field
 }
 
 // SearchVersions gets all versions of packages matching the search options

routers/api/packages/swift/swift.go

@@ -290,7 +290,24 @@ func DownloadManifest(ctx *context.Context) {
 	})
 }
 
-// https://github.com/swiftlang/swift-package-manager/blob/main/Documentation/PackageRegistry/Registry.md#endpoint-6
+// formFileOptionalReadCloser returns (nil, nil) if the formKey is not present.
+func formFileOptionalReadCloser(ctx *context.Context, formKey string) (io.ReadCloser, error) {
+	multipartFile, _, err := ctx.Req.FormFile(formKey)
+	if err != nil && !errors.Is(err, http.ErrMissingFile) {
+		return nil, err
+	}
+	if multipartFile != nil {
+		return multipartFile, nil
+	}
+
+	content := ctx.Req.FormValue(formKey)
+	if content == "" {
+		return nil, nil
+	}
+	return io.NopCloser(strings.NewReader(ctx.Req.FormValue(formKey))), nil
+}
+
+// UploadPackageFile refers to https://github.com/swiftlang/swift-package-manager/blob/main/Documentation/PackageRegistry/Registry.md#endpoint-6
 func UploadPackageFile(ctx *context.Context) {
 	packageScope := ctx.PathParam("scope")
 	packageName := ctx.PathParam("name")
@@ -304,9 +321,9 @@ func UploadPackageFile(ctx *context.Context) {
 
 	packageVersion := v.Core().String()
 
-	file, _, err := ctx.Req.FormFile("source-archive")
-	if err != nil {
-		apiError(ctx, http.StatusBadRequest, err)
+	file, err := formFileOptionalReadCloser(ctx, "source-archive")
+	if file == nil || err != nil {
+		apiError(ctx, http.StatusBadRequest, "unable to read source-archive file")
 		return
 	}
 	defer file.Close()
@@ -318,10 +335,13 @@ func UploadPackageFile(ctx *context.Context) {
 	}
 	defer buf.Close()
 
-	var mr io.Reader
-	metadata := ctx.Req.FormValue("metadata")
-	if metadata != "" {
-		mr = strings.NewReader(metadata)
+	mr, err := formFileOptionalReadCloser(ctx, "metadata")
+	if err != nil {
+		apiError(ctx, http.StatusBadRequest, "unable to read metadata file")
+		return
+	}
+	if mr != nil {
+		defer mr.Close()
 	}
 
 	pck, err := swift_module.ParsePackage(buf, buf.Size(), mr)

tests/integration/api_packages_swift_test.go

@@ -23,6 +23,7 @@ import (
 	"code.gitea.io/gitea/tests"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestPackageSwift(t *testing.T) {
@@ -34,6 +35,7 @@ func TestPackageSwift(t *testing.T) {
 	packageName := "test_package"
 	packageID := packageScope + "." + packageName
 	packageVersion := "1.0.3"
+	packageVersion2 := "1.0.4"
 	packageAuthor := "KN4CK3R"
 	packageDescription := "Gitea Test Package"
 	packageRepositoryURL := "https://gitea.io/gitea/gitea"
@@ -183,6 +185,94 @@ func TestPackageSwift(t *testing.T) {
 		)
 	})
 
+	t.Run("UploadMultipart", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+
+		uploadPackage := func(t *testing.T, url string, expectedStatus int, sr io.Reader, metadata string) {
+			var body bytes.Buffer
+			mpw := multipart.NewWriter(&body)
+
+			// Read the source archive content
+			sourceContent, err := io.ReadAll(sr)
+			assert.NoError(t, err)
+			mpw.WriteField("source-archive", string(sourceContent))
+
+			if metadata != "" {
+				mpw.WriteField("metadata", metadata)
+			}
+
+			mpw.Close()
+
+			req := NewRequestWithBody(t, "PUT", url, &body).
+				SetHeader("Content-Type", mpw.FormDataContentType()).
+				SetHeader("Accept", swift_router.AcceptJSON).
+				AddBasicAuth(user.Name)
+			MakeRequest(t, req, expectedStatus)
+		}
+
+		createArchive := func(files map[string]string) *bytes.Buffer {
+			var buf bytes.Buffer
+			zw := zip.NewWriter(&buf)
+			for filename, content := range files {
+				w, _ := zw.Create(filename)
+				w.Write([]byte(content))
+			}
+			zw.Close()
+			return &buf
+		}
+
+		uploadURL := fmt.Sprintf("%s/%s/%s/%s", url, packageScope, packageName, packageVersion2)
+
+		req := NewRequestWithBody(t, "PUT", uploadURL, bytes.NewReader([]byte{}))
+		MakeRequest(t, req, http.StatusUnauthorized)
+
+		// Test with metadata as form field
+		uploadPackage(
+			t,
+			uploadURL,
+			http.StatusCreated,
+			createArchive(map[string]string{
+				"Package.swift":           contentManifest1,
+				"[email protected]": contentManifest2,
+			}),
+			`{"name":"`+packageName+`","version":"`+packageVersion2+`","description":"`+packageDescription+`","codeRepository":"`+packageRepositoryURL+`","author":{"givenName":"`+packageAuthor+`"},"repositoryURLs":["`+packageRepositoryURL+`"]}`,
+		)
+
+		pvs, err := packages.GetVersionsByPackageType(db.DefaultContext, user.ID, packages.TypeSwift)
+		assert.NoError(t, err)
+		require.Len(t, pvs, 2) // ATTENTION: many subtests are unable to run separately, they depend on the results of previous tests
+		thisPackageVersion := pvs[0]
+		pd, err := packages.GetPackageDescriptor(db.DefaultContext, thisPackageVersion)
+		assert.NoError(t, err)
+		assert.NotNil(t, pd.SemVer)
+		assert.Equal(t, packageID, pd.Package.Name)
+		assert.Equal(t, packageVersion2, pd.Version.Version)
+		assert.IsType(t, &swift_module.Metadata{}, pd.Metadata)
+		metadata := pd.Metadata.(*swift_module.Metadata)
+		assert.Equal(t, packageDescription, metadata.Description)
+		assert.Len(t, metadata.Manifests, 2)
+		assert.Equal(t, contentManifest1, metadata.Manifests[""].Content)
+		assert.Equal(t, contentManifest2, metadata.Manifests["5.6"].Content)
+		assert.Len(t, pd.VersionProperties, 1)
+		assert.Equal(t, packageRepositoryURL, pd.VersionProperties.GetByName(swift_module.PropertyRepositoryURL))
+
+		pfs, err := packages.GetFilesByVersionID(db.DefaultContext, thisPackageVersion.ID)
+		assert.NoError(t, err)
+		assert.Len(t, pfs, 1)
+		assert.Equal(t, fmt.Sprintf("%s-%s.zip", packageName, packageVersion2), pfs[0].Name)
+		assert.True(t, pfs[0].IsLead)
+
+		uploadPackage(
+			t,
+			uploadURL,
+			http.StatusConflict,
+			createArchive(map[string]string{
+				"Package.swift": contentManifest1,
+			}),
+			"",
+		)
+	})
+
 	t.Run("Download", func(t *testing.T) {
 		defer tests.PrintCurrentTest(t)()
 
@@ -211,7 +301,7 @@ func TestPackageSwift(t *testing.T) {
 			SetHeader("Accept", swift_router.AcceptJSON)
 		resp := MakeRequest(t, req, http.StatusOK)
 
-		versionURL := setting.AppURL + url[1:] + fmt.Sprintf("/%s/%s/%s", packageScope, packageName, packageVersion)
+		versionURL := setting.AppURL + url[1:] + fmt.Sprintf("/%s/%s/%s", packageScope, packageName, packageVersion2)
 
 		assert.Equal(t, "1", resp.Header().Get("Content-Version"))
 		assert.Equal(t, fmt.Sprintf(`<%s>; rel="latest-version"`, versionURL), resp.Header().Get("Link"))
@@ -221,9 +311,9 @@ func TestPackageSwift(t *testing.T) {
 		var result *swift_router.EnumeratePackageVersionsResponse
 		DecodeJSON(t, resp, &result)
 
-		assert.Len(t, result.Releases, 1)
-		assert.Contains(t, result.Releases, packageVersion)
-		assert.Equal(t, versionURL, result.Releases[packageVersion].URL)
+		assert.Len(t, result.Releases, 2)
+		assert.Contains(t, result.Releases, packageVersion2)
+		assert.Equal(t, versionURL, result.Releases[packageVersion2].URL)
 
 		req = NewRequest(t, "GET", fmt.Sprintf("%s/%s/%s.json", url, packageScope, packageName)).
 			AddBasicAuth(user.Name)