mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-07-25 19:07:59 +00:00
This PR adds a new linter to the codebase and addresses all the problems that it identified (including a small number of false positives). The lint-single-response Go analyzer attempts to prevent a common problem in Forgejo where it is possible for a web handler to provide a response to a request, and then continue code execution unintentionally. For example:
```go
err := json.Unmarshal(data, &claims)
if err != nil {
ctx.Error(http.StatusInternalServerError, "Error in unmarshal", err)
// Oops, I forgot to `return` here...
}
// ... more work occurs ...
ctx.JSON(http.StatusOK, resp)
```
In order to detect these cases, lint-single-response contains a list of functions that deliver a web response. When any of those functions are used within a function, the control flow must not perform any work after the function is invoked -- it can only return and exit the function.
### Tests for Go changes
- I added test coverage for Go changes...
- [x] in their respective `*_test.go` for unit tests.
- [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I ran...
- [x] `make pr-go` before pushing
### Documentation
- [x] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- Documentation on the new linter is included inline, in `build/lint-single-response/README.md`.
- [ ] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [ ] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [x] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13087
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
149 lines
3.9 KiB
Go
149 lines
3.9 KiB
Go
// Copyright 2018 The Gitea Authors. All rights reserved.
|
|
// SPDX-License-Identifier: MIT
|
|
|
|
package repo
|
|
|
|
import (
|
|
"net/http"
|
|
|
|
issues_model "forgejo.org/models/issues"
|
|
access_model "forgejo.org/models/perm/access"
|
|
"forgejo.org/modules/setting"
|
|
"forgejo.org/services/context"
|
|
)
|
|
|
|
// AddDependency adds new dependencies
|
|
func AddDependency(ctx *context.Context) {
|
|
issueIndex := ctx.ParamsInt64("index")
|
|
issue, err := issues_model.GetIssueByIndex(ctx, ctx.Repo.Repository.ID, issueIndex)
|
|
if err != nil {
|
|
ctx.ServerError("GetIssueByIndex", err)
|
|
return
|
|
}
|
|
|
|
// Check if the Repo is allowed to have dependencies
|
|
if !ctx.Repo.CanCreateIssueDependencies(ctx, ctx.Doer, issue.IsPull) {
|
|
ctx.Error(http.StatusForbidden, "CanCreateIssueDependencies")
|
|
return
|
|
}
|
|
|
|
depID := ctx.FormInt64("newDependency")
|
|
|
|
if err = issue.LoadRepo(ctx); err != nil {
|
|
ctx.ServerError("LoadRepo", err)
|
|
return
|
|
}
|
|
|
|
// Dependency
|
|
dep, err := issues_model.GetIssueByID(ctx, depID)
|
|
if err != nil {
|
|
ctx.Flash.Error(ctx.Tr("repo.issues.dependency.add_error_dep_issue_not_exist"))
|
|
ctx.Redirect(issue.Link())
|
|
return
|
|
}
|
|
|
|
// Check if both issues are in the same repo if cross repository dependencies is not enabled
|
|
if issue.RepoID != dep.RepoID {
|
|
if !setting.Service.AllowCrossRepositoryDependencies {
|
|
ctx.Flash.Error(ctx.Tr("repo.issues.dependency.add_error_dep_not_same_repo"))
|
|
ctx.Redirect(issue.Link())
|
|
return
|
|
}
|
|
if err := dep.LoadRepo(ctx); err != nil {
|
|
ctx.ServerError("loadRepo", err)
|
|
return
|
|
}
|
|
// Can ctx.Doer read issues in the dep repo?
|
|
depRepoPerm, err := access_model.GetUserRepoPermission(ctx, dep.Repo, ctx.Doer)
|
|
if err != nil {
|
|
ctx.ServerError("GetUserRepoPermission", err)
|
|
return
|
|
}
|
|
if !depRepoPerm.CanReadIssuesOrPulls(dep.IsPull) {
|
|
// you can't see this dependency
|
|
ctx.Redirect(issue.Link())
|
|
return
|
|
}
|
|
}
|
|
|
|
// Check if issue and dependency is the same
|
|
if dep.ID == issue.ID {
|
|
ctx.Flash.Error(ctx.Tr("repo.issues.dependency.add_error_same_issue"))
|
|
ctx.Redirect(issue.Link())
|
|
return
|
|
}
|
|
|
|
err = issues_model.CreateIssueDependency(ctx, ctx.Doer, issue, dep)
|
|
if err != nil {
|
|
if issues_model.IsErrDependencyExists(err) {
|
|
ctx.Flash.Error(ctx.Tr("repo.issues.dependency.add_error_dep_exists"))
|
|
ctx.Redirect(issue.Link())
|
|
return
|
|
} else if issues_model.IsErrCircularDependency(err) {
|
|
ctx.Flash.Error(ctx.Tr("repo.issues.dependency.add_error_cannot_create_circular"))
|
|
ctx.Redirect(issue.Link())
|
|
return
|
|
}
|
|
ctx.ServerError("CreateOrUpdateIssueDependency", err)
|
|
return
|
|
}
|
|
|
|
ctx.Redirect(issue.Link())
|
|
}
|
|
|
|
// RemoveDependency removes the dependency
|
|
func RemoveDependency(ctx *context.Context) {
|
|
issueIndex := ctx.ParamsInt64("index")
|
|
issue, err := issues_model.GetIssueByIndex(ctx, ctx.Repo.Repository.ID, issueIndex)
|
|
if err != nil {
|
|
ctx.ServerError("GetIssueByIndex", err)
|
|
return
|
|
}
|
|
|
|
// Check if the Repo is allowed to have dependencies
|
|
if !ctx.Repo.CanCreateIssueDependencies(ctx, ctx.Doer, issue.IsPull) {
|
|
ctx.Error(http.StatusForbidden, "CanCreateIssueDependencies")
|
|
return
|
|
}
|
|
|
|
depID := ctx.FormInt64("removeDependencyID")
|
|
|
|
if err = issue.LoadRepo(ctx); err != nil {
|
|
ctx.ServerError("LoadRepo", err)
|
|
return
|
|
}
|
|
|
|
// Dependency Type
|
|
depTypeStr := ctx.Req.PostFormValue("dependencyType")
|
|
|
|
var depType issues_model.DependencyType
|
|
|
|
switch depTypeStr {
|
|
case "blockedBy":
|
|
depType = issues_model.DependencyTypeBlockedBy
|
|
case "blocking":
|
|
depType = issues_model.DependencyTypeBlocking
|
|
default:
|
|
ctx.Error(http.StatusBadRequest, "GetDependencyType")
|
|
return
|
|
}
|
|
|
|
// Dependency
|
|
dep, err := issues_model.GetIssueByID(ctx, depID)
|
|
if err != nil {
|
|
ctx.ServerError("GetIssueByID", err)
|
|
return
|
|
}
|
|
|
|
if err = issues_model.RemoveIssueDependency(ctx, ctx.Doer, issue, dep, depType); err != nil {
|
|
if issues_model.IsErrDependencyNotExists(err) {
|
|
ctx.Flash.Error(ctx.Tr("repo.issues.dependency.add_error_dep_not_exist"))
|
|
return
|
|
}
|
|
ctx.ServerError("RemoveIssueDependency", err)
|
|
return
|
|
}
|
|
|
|
// Redirect
|
|
ctx.Redirect(issue.Link())
|
|
}
|