From cbb563b080ad54cbd07a9d11b93f087be118240d Mon Sep 17 00:00:00 2001 From: leo <80651389+civilwarinfrance@users.noreply.github.com> Date: Fri, 30 Oct 2026 12:48:56 -0700 Subject: [PATCH] fix(api): edit and delete repository labels by name (#39974) Repo and org label endpoints accepted a label name for GET but only a numeric ID for PATCH and DELETE, which breaks label-sync style tools. - PATCH and DELETE on `/repos/{owner}/{repo}/labels/{id}` and `/orgs/{org}/labels/{id}` accept a label name when the value is not numeric, numeric values stay IDs - A name shared by several labels returns 400 instead of acting on an arbitrary one - Deleting a missing label returns 404 instead of 204 - The swagger `id` parameter of these endpoints is now a string Fixes: https://github.com/go-gitea/gitea/issues/34937 Co-authored-by: Hello-Liar <80651389+Hello-Liar@users.noreply.github.com> Co-authored-by: silverwind --- models/issues/label.go | 28 +++++----- models/issues/label_test.go | 3 + routers/api/v1/org/label.go | 59 +++++++++----------- routers/api/v1/repo/label.go | 45 +++++++-------- templates/swagger/v1-openapi3.generated.json | 30 ++++------ templates/swagger/v1-swagger.generated.json | 30 ++++------ tests/integration/api_issue_label_test.go | 12 +++- 7 files changed, 100 insertions(+), 107 deletions(-) diff --git a/models/issues/label.go b/models/issues/label.go index 0ee19f81689..2925ab19b2e 100644 --- a/models/issues/label.go +++ b/models/issues/label.go @@ -331,14 +331,23 @@ func GetLabelInRepoByName(ctx context.Context, repoID int64, labelName string) ( if len(labelName) == 0 || repoID <= 0 { return nil, ErrRepoLabelNotExist{0, repoID} } + return getUniqueLabel(ctx, builder.Eq{"name": labelName, "repo_id": repoID}, ErrRepoLabelNotExist{0, repoID}) +} - l, exist, err := db.Get[Label](ctx, builder.Eq{"name": labelName, "repo_id": repoID}) - if err != nil { +// getUniqueLabel rejects ambiguous matches because label names are not unique +func getUniqueLabel(ctx context.Context, cond builder.Cond, errNotExist error) (*Label, error) { + labels := make([]*Label, 0, 2) + if err := db.GetEngine(ctx).Where(cond).Limit(2).Find(&labels); err != nil { return nil, err - } else if !exist { - return nil, ErrRepoLabelNotExist{0, repoID} } - return l, nil + switch len(labels) { + case 0: + return nil, errNotExist + case 1: + return labels[0], nil + default: + return nil, util.NewInvalidArgumentErrorf("label name is ambiguous, use the label id") + } } // GetLabelInRepoOrOrgByID returns the label with labelID scoped to the repo, falling back to the @@ -454,14 +463,7 @@ func GetLabelInOrgByName(ctx context.Context, orgID int64, labelName string) (*L if len(labelName) == 0 || orgID <= 0 { return nil, ErrOrgLabelNotExist{0, orgID} } - - l, exist, err := db.Get[Label](ctx, builder.Eq{"name": labelName, "org_id": orgID}) - if err != nil { - return nil, err - } else if !exist { - return nil, ErrOrgLabelNotExist{0, orgID} - } - return l, nil + return getUniqueLabel(ctx, builder.Eq{"name": labelName, "org_id": orgID}, ErrOrgLabelNotExist{0, orgID}) } // GetLabelInOrgByID returns a label by ID in given organization. diff --git a/models/issues/label_test.go b/models/issues/label_test.go index e22693ec674..7eee7f2aad0 100644 --- a/models/issues/label_test.go +++ b/models/issues/label_test.go @@ -264,6 +264,9 @@ func TestNewLabels(t *testing.T) { for _, label := range labels { unittest.AssertExistsAndLoadBean(t, label, unittest.Cond("id = ?", label.ID)) } + assert.NoError(t, issues_model.NewLabel(t.Context(), &issues_model.Label{RepoID: 2, Name: "labelName2", Color: "#000000"})) + _, err := issues_model.GetLabelInRepoByName(t.Context(), 2, "labelName2") + assert.ErrorIs(t, err, util.ErrInvalidArgument) unittest.CheckConsistencyFor(t, &issues_model.Label{}, &repo_model.Repository{}) } diff --git a/routers/api/v1/org/label.go b/routers/api/v1/org/label.go index a2e3629bab0..77574e41d37 100644 --- a/routers/api/v1/org/label.go +++ b/routers/api/v1/org/label.go @@ -108,7 +108,15 @@ func CreateLabel(ctx *context.APIContext) { ctx.JSON(http.StatusCreated, convert.ToLabel(label, nil, ctx.Org.Organization.AsUser())) } -// GetLabel get label by organization and label id +func getOrgLabelByIDOrName(ctx *context.APIContext) (*issues_model.Label, error) { + idOrName := ctx.PathParam("id") + if labelID, err := strconv.ParseInt(idOrName, 10, 64); err == nil { + return issues_model.GetLabelInOrgByID(ctx, ctx.Org.Organization.ID, labelID) + } + return issues_model.GetLabelInOrgByName(ctx, ctx.Org.Organization.ID, idOrName) +} + +// GetLabel get label by organization and label id or name func GetLabel(ctx *context.APIContext) { // swagger:operation GET /orgs/{org}/labels/{id} organization orgGetLabel // --- @@ -123,9 +131,8 @@ func GetLabel(ctx *context.APIContext) { // required: true // - name: id // in: path - // description: id of the label to get - // type: integer - // format: int64 + // description: id of the label to get, or its name when not numeric + // type: string // required: true // responses: // "200": @@ -133,26 +140,13 @@ func GetLabel(ctx *context.APIContext) { // default: // "$ref": "#/responses/error" - var ( - label *issues_model.Label - err error - ) - strID := ctx.PathParam("id") - if intID, err2 := strconv.ParseInt(strID, 10, 64); err2 != nil { - label, err = issues_model.GetLabelInOrgByName(ctx, ctx.Org.Organization.ID, strID) - } else { - label, err = issues_model.GetLabelInOrgByID(ctx, ctx.Org.Organization.ID, intID) - } + l, err := getOrgLabelByIDOrName(ctx) if err != nil { - if issues_model.IsErrOrgLabelNotExist(err) { - ctx.APIErrorNotFound() - } else { - ctx.APIErrorInternal(err) - } + ctx.APIErrorAuto(err) return } - ctx.JSON(http.StatusOK, convert.ToLabel(label, nil, ctx.Org.Organization.AsUser())) + ctx.JSON(http.StatusOK, convert.ToLabel(l, nil, ctx.Org.Organization.AsUser())) } // EditLabel modify a label for an Organization @@ -172,9 +166,8 @@ func EditLabel(ctx *context.APIContext) { // required: true // - name: id // in: path - // description: id of the label to edit - // type: integer - // format: int64 + // description: id of the label to edit, or its name when not numeric + // type: string // required: true // - name: body // in: body @@ -186,13 +179,9 @@ func EditLabel(ctx *context.APIContext) { // default: // "$ref": "#/responses/error" form := web.GetForm[*api.EditLabelOption](ctx) - l, err := issues_model.GetLabelInOrgByID(ctx, ctx.Org.Organization.ID, ctx.PathParamInt64("id")) + l, err := getOrgLabelByIDOrName(ctx) if err != nil { - if issues_model.IsErrOrgLabelNotExist(err) { - ctx.APIErrorNotFound() - } else { - ctx.APIErrorInternal(err) - } + ctx.APIErrorAuto(err) return } @@ -235,9 +224,8 @@ func DeleteLabel(ctx *context.APIContext) { // required: true // - name: id // in: path - // description: id of the label to delete - // type: integer - // format: int64 + // description: id of the label to delete, or its name when not numeric + // type: string // required: true // responses: // "204": @@ -245,7 +233,12 @@ func DeleteLabel(ctx *context.APIContext) { // default: // "$ref": "#/responses/error" - if err := issues_model.DeleteOrgLabel(ctx, ctx.Org.Organization.ID, ctx.PathParamInt64("id")); err != nil { + l, err := getOrgLabelByIDOrName(ctx) + if err != nil { + ctx.APIErrorAuto(err) + return + } + if err := issues_model.DeleteOrgLabel(ctx, ctx.Org.Organization.ID, l.ID); err != nil { ctx.APIErrorInternal(err) return } diff --git a/routers/api/v1/repo/label.go b/routers/api/v1/repo/label.go index 83252445b0e..ec29cf326bf 100644 --- a/routers/api/v1/repo/label.go +++ b/routers/api/v1/repo/label.go @@ -65,7 +65,15 @@ func ListLabels(ctx *context.APIContext) { ctx.JSON(http.StatusOK, convert.ToLabelList(labels, ctx.Repo.Repository, nil)) } -// GetLabel get label by repository and label id +func getRepoLabelByIDOrName(ctx *context.APIContext) (*issues_model.Label, error) { + idOrName := ctx.PathParam("id") + if labelID, err := strconv.ParseInt(idOrName, 10, 64); err == nil { + return issues_model.GetLabelInRepoByID(ctx, ctx.Repo.Repository.ID, labelID) + } + return issues_model.GetLabelInRepoByName(ctx, ctx.Repo.Repository.ID, idOrName) +} + +// GetLabel get label by repository and label id or name func GetLabel(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/labels/{id} issue issueGetLabel // --- @@ -85,9 +93,8 @@ func GetLabel(ctx *context.APIContext) { // required: true // - name: id // in: path - // description: id of the label to get - // type: integer - // format: int64 + // description: id of the label to get, or its name when not numeric + // type: string // required: true // responses: // "200": @@ -95,16 +102,7 @@ func GetLabel(ctx *context.APIContext) { // default: // "$ref": "#/responses/error" - var ( - l *issues_model.Label - err error - ) - strID := ctx.PathParam("id") - if intID, err2 := strconv.ParseInt(strID, 10, 64); err2 != nil { - l, err = issues_model.GetLabelInRepoByName(ctx, ctx.Repo.Repository.ID, strID) - } else { - l, err = issues_model.GetLabelInRepoByID(ctx, ctx.Repo.Repository.ID, intID) - } + l, err := getRepoLabelByIDOrName(ctx) if err != nil { ctx.APIErrorAuto(err) return @@ -189,9 +187,8 @@ func EditLabel(ctx *context.APIContext) { // required: true // - name: id // in: path - // description: id of the label to edit - // type: integer - // format: int64 + // description: id of the label to edit, or its name when not numeric + // type: string // required: true // - name: body // in: body @@ -204,7 +201,7 @@ func EditLabel(ctx *context.APIContext) { // "$ref": "#/responses/error" form := web.GetForm[*api.EditLabelOption](ctx) - l, err := issues_model.GetLabelInRepoByID(ctx, ctx.Repo.Repository.ID, ctx.PathParamInt64("id")) + l, err := getRepoLabelByIDOrName(ctx) if err != nil { ctx.APIErrorAuto(err) return @@ -254,9 +251,8 @@ func DeleteLabel(ctx *context.APIContext) { // required: true // - name: id // in: path - // description: id of the label to delete - // type: integer - // format: int64 + // description: id of the label to delete, or its name when not numeric + // type: string // required: true // responses: // "204": @@ -264,7 +260,12 @@ func DeleteLabel(ctx *context.APIContext) { // default: // "$ref": "#/responses/error" - if err := issues_model.DeleteRepoLabel(ctx, ctx.Repo.Repository.ID, ctx.PathParamInt64("id")); err != nil { + l, err := getRepoLabelByIDOrName(ctx) + if err != nil { + ctx.APIErrorAuto(err) + return + } + if err := issues_model.DeleteRepoLabel(ctx, ctx.Repo.Repository.ID, l.ID); err != nil { ctx.APIErrorInternal(err) return } diff --git a/templates/swagger/v1-openapi3.generated.json b/templates/swagger/v1-openapi3.generated.json index 7454d108c5c..ee1230ce0a6 100644 --- a/templates/swagger/v1-openapi3.generated.json +++ b/templates/swagger/v1-openapi3.generated.json @@ -14503,13 +14503,12 @@ } }, { - "description": "id of the label to delete", + "description": "id of the label to delete, or its name when not numeric", "in": "path", "name": "id", "required": true, "schema": { - "format": "int64", - "type": "integer" + "type": "string" } } ], @@ -14539,13 +14538,12 @@ } }, { - "description": "id of the label to get", + "description": "id of the label to get, or its name when not numeric", "in": "path", "name": "id", "required": true, "schema": { - "format": "int64", - "type": "integer" + "type": "string" } } ], @@ -14575,13 +14573,12 @@ } }, { - "description": "id of the label to edit", + "description": "id of the label to edit, or its name when not numeric", "in": "path", "name": "id", "required": true, "schema": { - "format": "int64", - "type": "integer" + "type": "string" } } ], @@ -26676,13 +26673,12 @@ } }, { - "description": "id of the label to delete", + "description": "id of the label to delete, or its name when not numeric", "in": "path", "name": "id", "required": true, "schema": { - "format": "int64", - "type": "integer" + "type": "string" } } ], @@ -26721,13 +26717,12 @@ } }, { - "description": "id of the label to get", + "description": "id of the label to get, or its name when not numeric", "in": "path", "name": "id", "required": true, "schema": { - "format": "int64", - "type": "integer" + "type": "string" } } ], @@ -26766,13 +26761,12 @@ } }, { - "description": "id of the label to edit", + "description": "id of the label to edit, or its name when not numeric", "in": "path", "name": "id", "required": true, "schema": { - "format": "int64", - "type": "integer" + "type": "string" } } ], diff --git a/templates/swagger/v1-swagger.generated.json b/templates/swagger/v1-swagger.generated.json index c30635a6921..5a97a354502 100644 --- a/templates/swagger/v1-swagger.generated.json +++ b/templates/swagger/v1-swagger.generated.json @@ -3095,9 +3095,8 @@ "required": true }, { - "type": "integer", - "format": "int64", - "description": "id of the label to get", + "type": "string", + "description": "id of the label to get, or its name when not numeric", "name": "id", "in": "path", "required": true @@ -3127,9 +3126,8 @@ "required": true }, { - "type": "integer", - "format": "int64", - "description": "id of the label to delete", + "type": "string", + "description": "id of the label to delete, or its name when not numeric", "name": "id", "in": "path", "required": true @@ -3165,9 +3163,8 @@ "required": true }, { - "type": "integer", - "format": "int64", - "description": "id of the label to edit", + "type": "string", + "description": "id of the label to edit, or its name when not numeric", "name": "id", "in": "path", "required": true @@ -14239,9 +14236,8 @@ "required": true }, { - "type": "integer", - "format": "int64", - "description": "id of the label to get", + "type": "string", + "description": "id of the label to get, or its name when not numeric", "name": "id", "in": "path", "required": true @@ -14278,9 +14274,8 @@ "required": true }, { - "type": "integer", - "format": "int64", - "description": "id of the label to delete", + "type": "string", + "description": "id of the label to delete, or its name when not numeric", "name": "id", "in": "path", "required": true @@ -14323,9 +14318,8 @@ "required": true }, { - "type": "integer", - "format": "int64", - "description": "id of the label to edit", + "type": "string", + "description": "id of the label to edit, or its name when not numeric", "name": "id", "in": "path", "required": true diff --git a/tests/integration/api_issue_label_test.go b/tests/integration/api_issue_label_test.go index 1ced709204f..01e32ead015 100644 --- a/tests/integration/api_issue_label_test.go +++ b/tests/integration/api_issue_label_test.go @@ -6,6 +6,7 @@ package integration import ( "fmt" "net/http" + "net/url" "strings" "testing" @@ -116,7 +117,7 @@ func TestAPIModifyLabels(t *testing.T) { assert.Equal(t, strings.TrimLeft(dbLabel.Color, "#"), apiLabel.Color) // EditLabel - newName := "LabelNewName" + newName := "scope/new name" newColor := "09876a" newColorWrong := "09g76a" req = NewRequestWithJSON(t, "PATCH", singleURLStr, &api.EditLabelOption{ @@ -130,11 +131,16 @@ func TestAPIModifyLabels(t *testing.T) { Color: &newColorWrong, }).AddTokenAuth(token) MakeRequest(t, req, http.StatusUnprocessableEntity) + nameURLStr := urlStr + "/" + url.PathEscape(newName) + req = NewRequestWithJSON(t, "PATCH", nameURLStr, &api.EditLabelOption{}).AddTokenAuth(token) + resp = MakeRequest(t, req, http.StatusOK) + assert.Equal(t, dbLabel.ID, DecodeJSON(t, resp, &api.Label{}).ID) // DeleteLabel - req = NewRequest(t, "DELETE", singleURLStr). - AddTokenAuth(token) + req = NewRequest(t, "DELETE", nameURLStr).AddTokenAuth(token) MakeRequest(t, req, http.StatusNoContent) + req = NewRequest(t, "DELETE", singleURLStr).AddTokenAuth(token) + MakeRequest(t, req, http.StatusNotFound) }) } }