From 70739c32a912fb71e10da0ef7fc158a7ada23b03 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 1 Jun 2020 08:28:52 +0200 Subject: [PATCH] Handle expected errors in FileCreate & FileUpdate API (#11643) (#11718) as title needed for #11641 --- integrations/api_repo_file_create_test.go | 2 +- integrations/api_repo_file_update_test.go | 2 +- routers/api/v1/repo/file.go | 39 +++++++++++++++++++++-- templates/swagger/v1_json.tmpl | 18 +++++++++++ 4 files changed, 57 insertions(+), 4 deletions(-) diff --git a/integrations/api_repo_file_create_test.go b/integrations/api_repo_file_create_test.go index 3c8b50d5d..853224f09 100644 --- a/integrations/api_repo_file_create_test.go +++ b/integrations/api_repo_file_create_test.go @@ -189,7 +189,7 @@ func TestAPICreateFile(t *testing.T) { treePath = "README.md" url = fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s?token=%s", user2.Name, repo1.Name, treePath, token2) req = NewRequestWithJSON(t, "POST", url, &createFileOptions) - resp = session.MakeRequest(t, req, http.StatusInternalServerError) + resp = session.MakeRequest(t, req, http.StatusUnprocessableEntity) expectedAPIError := context.APIError{ Message: "repository file already exists [path: " + treePath + "]", URL: setting.API.SwaggerURL, diff --git a/integrations/api_repo_file_update_test.go b/integrations/api_repo_file_update_test.go index 369e48d64..69a946377 100644 --- a/integrations/api_repo_file_update_test.go +++ b/integrations/api_repo_file_update_test.go @@ -208,7 +208,7 @@ func TestAPIUpdateFile(t *testing.T) { updateFileOptions.SHA = "badsha" url = fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s?token=%s", user2.Name, repo1.Name, treePath, token2) req = NewRequestWithJSON(t, "PUT", url, &updateFileOptions) - resp = session.MakeRequest(t, req, http.StatusInternalServerError) + resp = session.MakeRequest(t, req, http.StatusUnprocessableEntity) expectedAPIError := context.APIError{ Message: "sha does not match [given: " + updateFileOptions.SHA + ", expected: " + correctSHA + "]", URL: setting.API.SwaggerURL, diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 02a7de9b5..73b8e31a4 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -7,6 +7,7 @@ package repo import ( "encoding/base64" + "fmt" "net/http" "time" @@ -198,6 +199,16 @@ func CreateFile(ctx *context.APIContext, apiOpts api.CreateFileOptions) { // responses: // "201": // "$ref": "#/responses/FileResponse" + // "403": + // "$ref": "#/responses/error" + // "404": + // "$ref": "#/responses/notFound" + // "422": + // "$ref": "#/responses/error" + + if ctx.Repo.Repository.IsEmpty { + ctx.Error(http.StatusUnprocessableEntity, "RepoIsEmpty", fmt.Errorf("repo is empty")) + } if apiOpts.BranchName == "" { apiOpts.BranchName = ctx.Repo.Repository.DefaultBranch @@ -235,7 +246,7 @@ func CreateFile(ctx *context.APIContext, apiOpts api.CreateFileOptions) { } if fileResponse, err := createOrUpdateFile(ctx, opts); err != nil { - ctx.Error(http.StatusInternalServerError, "CreateFile", err) + handleCreateOrUpdateFileError(ctx, err) } else { ctx.JSON(http.StatusCreated, fileResponse) } @@ -274,6 +285,16 @@ func UpdateFile(ctx *context.APIContext, apiOpts api.UpdateFileOptions) { // responses: // "200": // "$ref": "#/responses/FileResponse" + // "403": + // "$ref": "#/responses/error" + // "404": + // "$ref": "#/responses/notFound" + // "422": + // "$ref": "#/responses/error" + + if ctx.Repo.Repository.IsEmpty { + ctx.Error(http.StatusUnprocessableEntity, "RepoIsEmpty", fmt.Errorf("repo is empty")) + } if apiOpts.BranchName == "" { apiOpts.BranchName = ctx.Repo.Repository.DefaultBranch @@ -313,12 +334,26 @@ func UpdateFile(ctx *context.APIContext, apiOpts api.UpdateFileOptions) { } if fileResponse, err := createOrUpdateFile(ctx, opts); err != nil { - ctx.Error(http.StatusInternalServerError, "UpdateFile", err) + handleCreateOrUpdateFileError(ctx, err) } else { ctx.JSON(http.StatusOK, fileResponse) } } +func handleCreateOrUpdateFileError(ctx *context.APIContext, err error) { + if models.IsErrUserCannotCommit(err) || models.IsErrFilePathProtected(err) { + ctx.Error(http.StatusForbidden, "Access", err) + return + } + if models.IsErrBranchAlreadyExists(err) || models.IsErrFilenameInvalid(err) || models.IsErrSHADoesNotMatch(err) || + models.IsErrFilePathInvalid(err) || models.IsErrRepoFileAlreadyExists(err) { + ctx.Error(http.StatusUnprocessableEntity, "Invalid", err) + return + } + + ctx.Error(http.StatusInternalServerError, "UpdateFile", err) +} + // Called from both CreateFile or UpdateFile to handle both func createOrUpdateFile(ctx *context.APIContext, opts *repofiles.UpdateRepoFileOptions) (*api.FileResponse, error) { if !canWriteFiles(ctx.Repo) { diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 0cbe33bd2..2a8f0457b 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -2746,6 +2746,15 @@ "responses": { "200": { "$ref": "#/responses/FileResponse" + }, + "403": { + "$ref": "#/responses/error" + }, + "404": { + "$ref": "#/responses/notFound" + }, + "422": { + "$ref": "#/responses/error" } } }, @@ -2795,6 +2804,15 @@ "responses": { "201": { "$ref": "#/responses/FileResponse" + }, + "403": { + "$ref": "#/responses/error" + }, + "404": { + "$ref": "#/responses/notFound" + }, + "422": { + "$ref": "#/responses/error" } } },