aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLunny Xiao2021-04-11 10:57:44 +0800
committerGitHub2021-04-11 04:57:44 +0200
commit87074ec860e4ea49b707c8ada7137456f3531758 (patch)
tree7e0dbb5fb8d3a0ee656e233f0e8f7f9c3160018e
parent1fe5fe419ec98934bf0447e50fe8c28adeabaee5 (diff)
Fix delete nonexist oauth application 500 and prevent deadlock (#15384) (#15396)
* 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 <art27@cantab.net> Co-authored-by: Andrew Thornton <art27@cantab.net> Co-authored-by: 6543 <6543@obermui.de>
-rw-r--r--integrations/api_oauth2_apps_test.go4
-rw-r--r--models/migrate.go2
-rw-r--r--models/oauth2_application.go3
-rw-r--r--routers/api/v1/user/app.go6
4 files changed, 13 insertions, 2 deletions
diff --git a/integrations/api_oauth2_apps_test.go b/integrations/api_oauth2_apps_test.go
index 998043a6f..0ba56b6c9 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 9e44f32e5..28b670747 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 1b544e4e9..679fdb18f 100644
--- a/models/oauth2_application.go
+++ b/models/oauth2_application.go
@@ -235,7 +235,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
@@ -261,6 +261,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 33b27d60e..b88ed6fbd 100644
--- a/routers/api/v1/user/app.go
+++ b/routers/api/v1/user/app.go
@@ -274,7 +274,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
}