From a26845156bdfbcbce63b11873714ab7697b56ce4 Mon Sep 17 00:00:00 2001 From: Wolfgang Reithmeier Date: Sun, 6 Apr 2025 21:29:16 +0200 Subject: [PATCH 1/5] Fixes: Swift Package Manager Publishing using commandline throws error #33990 --- routers/api/packages/swift/swift.go | 30 ++++-- tests/integration/api_packages_swift_test.go | 97 +++++++++++++++++++- 2 files changed, 116 insertions(+), 11 deletions(-) diff --git a/routers/api/packages/swift/swift.go b/routers/api/packages/swift/swift.go index 4d7fb8b1a6992..a5e8b858faba2 100644 --- a/routers/api/packages/swift/swift.go +++ b/routers/api/packages/swift/swift.go @@ -304,10 +304,18 @@ func UploadPackageFile(ctx *context.Context) { packageVersion := v.Core().String() - file, _, err := ctx.Req.FormFile("source-archive") + var file io.ReadCloser + multipartFile, _, err := ctx.Req.FormFile("source-archive") if err != nil { - apiError(ctx, http.StatusBadRequest, err) - return + content := ctx.Req.FormValue("source-archive") + if content != "" { + file = io.NopCloser(strings.NewReader(content)) + } else { + apiError(ctx, http.StatusBadRequest, err) + return + } + } else { + file = multipartFile } defer file.Close() @@ -318,10 +326,18 @@ func UploadPackageFile(ctx *context.Context) { } defer buf.Close() - var mr io.Reader - metadata := ctx.Req.FormValue("metadata") - if metadata != "" { - mr = strings.NewReader(metadata) + var mr io.ReadCloser + metadataFile, _, err := ctx.Req.FormFile("metadata") + if err != nil { + metadata := ctx.Req.FormValue("metadata") + if metadata != "" { + mr = io.NopCloser(strings.NewReader(metadata)) + } + } else { + mr = metadataFile + } + if mr != nil { + defer mr.Close() } pck, err := swift_module.ParsePackage(buf, buf.Size(), mr) diff --git a/tests/integration/api_packages_swift_test.go b/tests/integration/api_packages_swift_test.go index c0e0dccfab11b..8c75c904f6da5 100644 --- a/tests/integration/api_packages_swift_test.go +++ b/tests/integration/api_packages_swift_test.go @@ -34,6 +34,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 +184,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, + "Package@swift-5.6.swift": 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) + assert.Len(t, pvs, 2) + + pd, err := packages.GetPackageDescriptor(db.DefaultContext, pvs[1]) + 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, pvs[1].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 +300,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 +310,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) From 7af142a8b3b1387246dfbc7bb026c08c4d77927c Mon Sep 17 00:00:00 2001 From: Wolfgang Reithmeier Date: Mon, 7 Apr 2025 22:46:53 +0200 Subject: [PATCH 2/5] Improve error handling for package file uploads in Swift API (#33990) --- routers/api/packages/swift/swift.go | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/routers/api/packages/swift/swift.go b/routers/api/packages/swift/swift.go index a5e8b858faba2..f0eb59ee1de88 100644 --- a/routers/api/packages/swift/swift.go +++ b/routers/api/packages/swift/swift.go @@ -306,16 +306,20 @@ func UploadPackageFile(ctx *context.Context) { var file io.ReadCloser multipartFile, _, err := ctx.Req.FormFile("source-archive") - if err != nil { + if err != nil && !errors.Is(err, http.ErrMissingFile) { + apiError(ctx, http.StatusBadRequest, err) + return + } + + if multipartFile != nil { + file = multipartFile + } else { content := ctx.Req.FormValue("source-archive") - if content != "" { - file = io.NopCloser(strings.NewReader(content)) - } else { - apiError(ctx, http.StatusBadRequest, err) + if content == "" { + apiError(ctx, http.StatusBadRequest, "source-archive is required either as file or form value") return } - } else { - file = multipartFile + file = io.NopCloser(strings.NewReader(content)) } defer file.Close() From f8b66c5bfab37a0c9ca5c55b3d374e5e7dfbcc4f Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 18 Apr 2025 18:59:43 +0800 Subject: [PATCH 3/5] fix --- models/packages/package_version.go | 4 +-- routers/api/packages/swift/swift.go | 50 +++++++++++++++-------------- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/models/packages/package_version.go b/models/packages/package_version.go index 278e8e3a86b0e..bb7fd895f81da 100644 --- a/models/packages/package_version.go +++ b/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 diff --git a/routers/api/packages/swift/swift.go b/routers/api/packages/swift/swift.go index f0eb59ee1de88..316b5c4941493 100644 --- a/routers/api/packages/swift/swift.go +++ b/routers/api/packages/swift/swift.go @@ -290,7 +290,26 @@ 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) { + var file io.ReadCloser + multipartFile, _, err := ctx.Req.FormFile(formKey) // it calls ParseMultipartForm automatically + if err != nil && !errors.Is(err, http.ErrMissingFile) { + return nil, err + } + if multipartFile != nil { + return multipartFile, nil + } + + _ = ctx.Req.ParseForm() // although ParseForm should have been called by FormFile->ParseMultipartForm, it's safe to call it again + if !ctx.Req.Form.Has(formKey) { + return nil, nil + } + file = io.NopCloser(strings.NewReader(ctx.Req.FormValue(formKey))) + return file, 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,23 +323,11 @@ func UploadPackageFile(ctx *context.Context) { packageVersion := v.Core().String() - var file io.ReadCloser - multipartFile, _, err := ctx.Req.FormFile("source-archive") - if err != nil && !errors.Is(err, http.ErrMissingFile) { - 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 } - - if multipartFile != nil { - file = multipartFile - } else { - content := ctx.Req.FormValue("source-archive") - if content == "" { - apiError(ctx, http.StatusBadRequest, "source-archive is required either as file or form value") - return - } - file = io.NopCloser(strings.NewReader(content)) - } defer file.Close() buf, err := packages_module.CreateHashedBufferFromReader(file) @@ -330,15 +337,10 @@ func UploadPackageFile(ctx *context.Context) { } defer buf.Close() - var mr io.ReadCloser - metadataFile, _, err := ctx.Req.FormFile("metadata") + mr, err := formFileOptionalReadCloser(ctx, "metadata") if err != nil { - metadata := ctx.Req.FormValue("metadata") - if metadata != "" { - mr = io.NopCloser(strings.NewReader(metadata)) - } - } else { - mr = metadataFile + apiError(ctx, http.StatusBadRequest, "unable to read metadata file") + return } if mr != nil { defer mr.Close() From c97d8eddc32361e5ad8ee0e92af0734aae5bae34 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 18 Apr 2025 19:03:24 +0800 Subject: [PATCH 4/5] fix --- routers/api/packages/swift/swift.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/routers/api/packages/swift/swift.go b/routers/api/packages/swift/swift.go index 316b5c4941493..9c909d49186f0 100644 --- a/routers/api/packages/swift/swift.go +++ b/routers/api/packages/swift/swift.go @@ -292,8 +292,7 @@ func DownloadManifest(ctx *context.Context) { // formFileOptionalReadCloser returns (nil, nil) if the formKey is not present. func formFileOptionalReadCloser(ctx *context.Context, formKey string) (io.ReadCloser, error) { - var file io.ReadCloser - multipartFile, _, err := ctx.Req.FormFile(formKey) // it calls ParseMultipartForm automatically + multipartFile, _, err := ctx.Req.FormFile(formKey) if err != nil && !errors.Is(err, http.ErrMissingFile) { return nil, err } @@ -301,12 +300,11 @@ func formFileOptionalReadCloser(ctx *context.Context, formKey string) (io.ReadCl return multipartFile, nil } - _ = ctx.Req.ParseForm() // although ParseForm should have been called by FormFile->ParseMultipartForm, it's safe to call it again - if !ctx.Req.Form.Has(formKey) { + content := ctx.Req.FormValue(formKey) + if content == "" { return nil, nil } - file = io.NopCloser(strings.NewReader(ctx.Req.FormValue(formKey))) - return file, 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 From be2725d293fad99772204b44ce5ea4b9ce1dd69a Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 18 Apr 2025 19:22:41 +0800 Subject: [PATCH 5/5] fix test --- tests/integration/api_packages_swift_test.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/integration/api_packages_swift_test.go b/tests/integration/api_packages_swift_test.go index 8c75c904f6da5..b29e8459ffb2b 100644 --- a/tests/integration/api_packages_swift_test.go +++ b/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) { @@ -239,9 +240,9 @@ func TestPackageSwift(t *testing.T) { pvs, err := packages.GetVersionsByPackageType(db.DefaultContext, user.ID, packages.TypeSwift) assert.NoError(t, err) - assert.Len(t, pvs, 2) - - pd, err := packages.GetPackageDescriptor(db.DefaultContext, pvs[1]) + 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) @@ -255,7 +256,7 @@ func TestPackageSwift(t *testing.T) { assert.Len(t, pd.VersionProperties, 1) assert.Equal(t, packageRepositoryURL, pd.VersionProperties.GetByName(swift_module.PropertyRepositoryURL)) - pfs, err := packages.GetFilesByVersionID(db.DefaultContext, pvs[1].ID) + 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)