From 015d11d26dd596703b40c0cc4121026282b9bbd3 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 11 Apr 2021 10:57:23 +0800 Subject: [PATCH] Fix delete nonexist oauth application 500 and prevent deadlock (#15384) (#15397) * Fix delete nonexist oauth application 500 * Fix test * Close the session * Fix more missed sess.Close * Remove unnecessary blank line Signed-off-by: Andrew Thornton Co-authored-by: Andrew Thornton Co-authored-by: 6543 <6543@obermui.de> --- integrations/api_oauth2_apps_test.go | 4 ++++ models/migrate.go | 2 ++ models/oauth2_application.go | 3 ++- routers/api/v1/user/app.go | 6 +++++- 4 files changed, 13 insertions(+), 2 deletions(-) diff --git a/integrations/api_oauth2_apps_test.go b/integrations/api_oauth2_apps_test.go index 998043a6fb..0ba56b6c9f 100644 --- a/integrations/api_oauth2_apps_test.go +++ b/integrations/api_oauth2_apps_test.go @@ -92,6 +92,10 @@ func testAPIDeleteOAuth2Application(t *testing.T) { session.MakeRequest(t, req, http.StatusNoContent) models.AssertNotExistsBean(t, &models.OAuth2Application{UID: oldApp.UID, Name: oldApp.Name}) + + // Delete again will return not found + req = NewRequest(t, "DELETE", urlStr) + session.MakeRequest(t, req, http.StatusNotFound) } func testAPIGetOAuth2Application(t *testing.T) { diff --git a/models/migrate.go b/models/migrate.go index 2715c5bd9b..0b9d7da1ff 100644 --- a/models/migrate.go +++ b/models/migrate.go @@ -39,6 +39,7 @@ func InsertMilestones(ms ...*Milestone) (err error) { // InsertIssues insert issues to database func InsertIssues(issues ...*Issue) error { sess := x.NewSession() + defer sess.Close() if err := sess.Begin(); err != nil { return err } @@ -194,6 +195,7 @@ func InsertPullRequests(prs ...*PullRequest) error { // InsertReleases migrates release func InsertReleases(rels ...*Release) error { sess := x.NewSession() + defer sess.Close() if err := sess.Begin(); err != nil { return err } diff --git a/models/oauth2_application.go b/models/oauth2_application.go index 1c7937685c..e2f8753517 100644 --- a/models/oauth2_application.go +++ b/models/oauth2_application.go @@ -233,7 +233,7 @@ func deleteOAuth2Application(sess *xorm.Session, id, userid int64) error { if deleted, err := sess.Delete(&OAuth2Application{ID: id, UID: userid}); err != nil { return err } else if deleted == 0 { - return fmt.Errorf("cannot find oauth2 application") + return ErrOAuthApplicationNotFound{ID: id} } codes := make([]*OAuth2AuthorizationCode, 0) // delete correlating auth codes @@ -259,6 +259,7 @@ func deleteOAuth2Application(sess *xorm.Session, id, userid int64) error { // DeleteOAuth2Application deletes the application with the given id and the grants and auth codes related to it. It checks if the userid was the creator of the app. func DeleteOAuth2Application(id, userid int64) error { sess := x.NewSession() + defer sess.Close() if err := sess.Begin(); err != nil { return err } diff --git a/routers/api/v1/user/app.go b/routers/api/v1/user/app.go index d02b8cea21..e5cea556a3 100644 --- a/routers/api/v1/user/app.go +++ b/routers/api/v1/user/app.go @@ -268,7 +268,11 @@ func DeleteOauth2Application(ctx *context.APIContext) { // "$ref": "#/responses/empty" appID := ctx.ParamsInt64(":id") if err := models.DeleteOAuth2Application(appID, ctx.User.ID); err != nil { - ctx.Error(http.StatusInternalServerError, "DeleteOauth2ApplicationByID", err) + if models.IsErrOAuthApplicationNotFound(err) { + ctx.NotFound() + } else { + ctx.Error(http.StatusInternalServerError, "DeleteOauth2ApplicationByID", err) + } return }