feat: make upload URL compatible with GitHub API (#9285)

Adds new a function, `AcceptsGithubResponse`, to the API router context struct to check if the requests accepts a Github response. Although Forgejo API will never be compatible with the Github API, historically Forgejo's API has been designed to follow that of Github closely and we know that a lot of tooling that uses the Github API can be used against the Forgejo API with little to no problem.

As a meet in the middle solution, this function can be used to respond with a more appropriate response that follows the Github API. This allows Forgejo to avoid breaking compatibility with existing users of the API and allows the API to be oh so slightly more compatible with that of Github for API clients that expect a Github response.

Because the `upload_url` field was added purely to match the Github API (forgejo/forgejo#580), it is fair to actually make it compatible with how the Github API intended it to be and that is by adding `{?name,label}` which is used by Github's Oktokit.

Only add `{?name,label}` when Forgejo knows the request accepts a Github response. This avoids breaking the API compatibility with non-Github API  clients.

Resolves Codeberg/Community#2132

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9285
Reviewed-by: 0ko <0ko@noreply.codeberg.org>
Reviewed-by: oliverpool <oliverpool@noreply.codeberg.org>
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-committed-by: Gusted <postmaster@gusted.xyz>

models/repo/release.go

@@ -180,7 +180,11 @@ func (r *Release) HTMLURL() string {
 }
 
 // APIUploadURL the api url to upload assets to a release. release must have attributes loaded
-func (r *Release) APIUploadURL() string {
+// If `githubFormat` is true, then `{?name,label}` is added to match the Github API.
+func (r *Release) APIUploadURL(githubFormat bool) string {
+	if githubFormat {
+		return r.APIURL() + "/assets{?name,label}"
+	}
 	return r.APIURL() + "/assets"
 }
 

routers/api/v1/repo/issue_label.go

@@ -215,7 +215,7 @@ func DeleteIssueLabel(ctx *context.APIContext) {
 		return
 	}
 
-	if ctx.Req.Header.Get("Accept") == "application/vnd.github+json" {
+	if ctx.AcceptsGithubResponse() {
 		ctx.JSON(http.StatusOK, convert.ToLabelList([]*issues_model.Label{label}, ctx.Repo.Repository, ctx.Repo.Owner))
 	} else {
 		ctx.Status(http.StatusNoContent)

routers/api/v1/repo/release.go

@@ -67,7 +67,7 @@ func GetRelease(ctx *context.APIContext) {
 		ctx.Error(http.StatusInternalServerError, "LoadAttributes", err)
 		return
 	}
-	ctx.JSON(http.StatusOK, convert.ToAPIRelease(ctx, ctx.Repo.Repository, release))
+	ctx.JSON(http.StatusOK, convert.ToAPIRelease(ctx, ctx.Repo.Repository, release, ctx.AcceptsGithubResponse()))
 }
 
 // GetLatestRelease gets the most recent non-prerelease, non-draft release of a repository, sorted by created_at
@@ -108,7 +108,7 @@ func GetLatestRelease(ctx *context.APIContext) {
 		ctx.Error(http.StatusInternalServerError, "LoadAttributes", err)
 		return
 	}
-	ctx.JSON(http.StatusOK, convert.ToAPIRelease(ctx, ctx.Repo.Repository, release))
+	ctx.JSON(http.StatusOK, convert.ToAPIRelease(ctx, ctx.Repo.Repository, release, ctx.AcceptsGithubResponse()))
 }
 
 // ListReleases list a repository's releases
@@ -177,7 +177,7 @@ func ListReleases(ctx *context.APIContext) {
 			ctx.Error(http.StatusInternalServerError, "LoadAttributes", err)
 			return
 		}
-		rels[i] = convert.ToAPIRelease(ctx, ctx.Repo.Repository, release)
+		rels[i] = convert.ToAPIRelease(ctx, ctx.Repo.Repository, release, ctx.AcceptsGithubResponse())
 	}
 
 	filteredCount, err := db.Count[repo_model.Release](ctx, opts)
@@ -288,7 +288,7 @@ func CreateRelease(ctx *context.APIContext) {
 			return
 		}
 	}
-	ctx.JSON(http.StatusCreated, convert.ToAPIRelease(ctx, ctx.Repo.Repository, rel))
+	ctx.JSON(http.StatusCreated, convert.ToAPIRelease(ctx, ctx.Repo.Repository, rel, ctx.AcceptsGithubResponse()))
 }
 
 // EditRelease edit a release
@@ -375,7 +375,7 @@ func EditRelease(ctx *context.APIContext) {
 		ctx.Error(http.StatusInternalServerError, "LoadAttributes", err)
 		return
 	}
-	ctx.JSON(http.StatusOK, convert.ToAPIRelease(ctx, ctx.Repo.Repository, rel))
+	ctx.JSON(http.StatusOK, convert.ToAPIRelease(ctx, ctx.Repo.Repository, rel, ctx.AcceptsGithubResponse()))
 }
 
 // DeleteRelease delete a release from a repository

routers/api/v1/repo/release_attachment.go

@@ -147,7 +147,7 @@ func ListReleaseAttachments(ctx *context.APIContext) {
 		ctx.Error(http.StatusInternalServerError, "LoadAttributes", err)
 		return
 	}
-	ctx.JSON(http.StatusOK, convert.ToAPIRelease(ctx, ctx.Repo.Repository, release).Attachments)
+	ctx.JSON(http.StatusOK, convert.ToAPIRelease(ctx, ctx.Repo.Repository, release, false).Attachments)
 }
 
 // CreateReleaseAttachment creates an attachment and saves the given file

routers/api/v1/repo/release_tags.go

@@ -63,7 +63,7 @@ func GetReleaseByTag(ctx *context.APIContext) {
 		ctx.Error(http.StatusInternalServerError, "LoadAttributes", err)
 		return
 	}
-	ctx.JSON(http.StatusOK, convert.ToAPIRelease(ctx, ctx.Repo.Repository, release))
+	ctx.JSON(http.StatusOK, convert.ToAPIRelease(ctx, ctx.Repo.Repository, release, ctx.AcceptsGithubResponse()))
 }
 
 // DeleteReleaseByTag delete a release from a repository by tag name

services/actions/notifier_helper.go

@@ -432,7 +432,7 @@ func notifyRelease(ctx context.Context, doer *user_model.User, rel *repo_model.R
 		WithRef(git.RefNameFromTag(rel.TagName).String()).
 		WithPayload(&api.ReleasePayload{
 			Action:     action,
-			Release:    convert.ToAPIRelease(ctx, rel.Repo, rel),
+			Release:    convert.ToAPIRelease(ctx, rel.Repo, rel, false),
 			Repository: convert.ToRepo(ctx, rel.Repo, permission),
 			Sender:     convert.ToUser(ctx, doer, nil),
 		}).

services/context/api.go

@@ -474,3 +474,10 @@ func (ctx *APIContext) IsUserRepoWriter(unitTypes []unit.Type) bool {
 
 	return false
 }
+
+// Returns true when the requests indicates that it accepts a Github response.
+// This should be used to return information in the way that the Github API
+// specifies it. Avoids breaking compatability with non-Github API clients.
+func (ctx *APIContext) AcceptsGithubResponse() bool {
+	return ctx.Req.Header.Get("Accept") == "application/vnd.github+json"
+}

services/context/api_test.go

@@ -4,6 +4,7 @@
 package context
 
 import (
+	"net/http/httptest"
 	"net/url"
 	"strconv"
 	"testing"
@@ -50,3 +51,26 @@ func TestGenAPILinks(t *testing.T) {
 		assert.Equal(t, links, response)
 	}
 }
+
+func TestAcceptsGithubResponse(t *testing.T) {
+	t.Run("Normal", func(t *testing.T) {
+		req := httptest.NewRequest("GET", "/", nil)
+		resp := httptest.NewRecorder()
+		base, baseCleanUp := NewBaseContext(resp, req)
+		t.Cleanup(baseCleanUp)
+		ctx := &APIContext{Base: base}
+
+		assert.False(t, ctx.AcceptsGithubResponse())
+	})
+
+	t.Run("Accepts Github", func(t *testing.T) {
+		req := httptest.NewRequest("GET", "/", nil)
+		req.Header.Add("Accept", "application/vnd.github+json")
+		resp := httptest.NewRecorder()
+		base, baseCleanUp := NewBaseContext(resp, req)
+		t.Cleanup(baseCleanUp)
+		ctx := &APIContext{Base: base}
+
+		assert.True(t, ctx.AcceptsGithubResponse())
+	})
+}

services/convert/release.go

@@ -11,7 +11,7 @@ import (
 )
 
 // ToAPIRelease convert a repo_model.Release to api.Release
-func ToAPIRelease(ctx context.Context, repo *repo_model.Repository, r *repo_model.Release) *api.Release {
+func ToAPIRelease(ctx context.Context, repo *repo_model.Repository, r *repo_model.Release, githubFormat bool) *api.Release {
 	return &api.Release{
 		ID:                   r.ID,
 		TagName:              r.TagName,
@@ -23,7 +23,7 @@ func ToAPIRelease(ctx context.Context, repo *repo_model.Repository, r *repo_mode
 		TarURL:               r.TarURL(),
 		ZipURL:               r.ZipURL(),
 		HideArchiveLinks:     r.HideArchiveLinks,
-		UploadURL:            r.APIUploadURL(),
+		UploadURL:            r.APIUploadURL(githubFormat),
 		IsDraft:              r.IsDraft,
 		IsPrerelease:         r.IsPrerelease,
 		CreatedAt:            r.CreatedUnix.AsTime(),

services/convert/release_test.go

@@ -21,9 +21,19 @@ func TestRelease_ToRelease(t *testing.T) {
 	release1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Release{ID: 1})
 	release1.LoadAttributes(db.DefaultContext)
 
-	apiRelease := ToAPIRelease(db.DefaultContext, repo1, release1)
+	t.Run("Normal", func(t *testing.T) {
-	assert.NotNil(t, apiRelease)
+		apiRelease := ToAPIRelease(t.Context(), repo1, release1, false)
-	assert.EqualValues(t, 1, apiRelease.ID)
+		assert.NotNil(t, apiRelease)
-	assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/releases/1", apiRelease.URL)
+		assert.EqualValues(t, 1, apiRelease.ID)
-	assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/releases/1/assets", apiRelease.UploadURL)
+		assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/releases/1", apiRelease.URL)
+		assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/releases/1/assets", apiRelease.UploadURL)
+	})
+
+	t.Run("Github format", func(t *testing.T) {
+		apiRelease := ToAPIRelease(t.Context(), repo1, release1, true)
+		assert.NotNil(t, apiRelease)
+		assert.EqualValues(t, 1, apiRelease.ID)
+		assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/releases/1", apiRelease.URL)
+		assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/releases/1/assets{?name,label}", apiRelease.UploadURL)
+	})
 }

services/webhook/notifier.go

@@ -824,7 +824,7 @@ func sendReleaseHook(ctx context.Context, doer *user_model.User, rel *repo_model
 	permission, _ := access_model.GetUserRepoPermission(ctx, rel.Repo, doer)
 	if err := PrepareWebhooks(ctx, EventSource{Repository: rel.Repo}, webhook_module.HookEventRelease, &api.ReleasePayload{
 		Action:     action,
-		Release:    convert.ToAPIRelease(ctx, rel.Repo, rel),
+		Release:    convert.ToAPIRelease(ctx, rel.Repo, rel, false),
 		Repository: convert.ToRepo(ctx, rel.Repo, permission),
 		Sender:     convert.ToUser(ctx, doer, nil),
 	}); err != nil {

tests/integration/api_releases_test.go

@@ -480,3 +480,20 @@ func TestAPIMissingAssetRelease(t *testing.T) {
 		AddTokenAuth(token)
 	MakeRequest(t, req, http.StatusBadRequest)
 }
+
+func TestAPIReleaseGithubFormat(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
+	user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+	token := getUserToken(t, user2.LowerName, auth_model.AccessTokenScopeReadRepository)
+
+	req := NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/%s/%s/releases/1", user2.Name, repo.Name)).AddTokenAuth(token)
+	req.Header.Add("Accept", "application/vnd.github+json")
+	resp := MakeRequest(t, req, http.StatusOK)
+
+	var apiRelease *api.Release
+	DecodeJSON(t, resp, &apiRelease)
+
+	assert.True(t, strings.HasSuffix(apiRelease.UploadURL, "/api/v1/repos/user2/repo1/releases/1/assets{?name,label}"), apiRelease.UploadURL)
+}