mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2024-09-10 20:22:14 +02:00
Refactor legacy strange git operations (#22756)
During the refactoring of the git module, I found there were some strange operations. This PR tries to fix 2 of them 1. The empty argument `--` in repo_attribute.go, which was introduced by #16773. It seems unnecessary because nothing else would be added later. 2. The complex git service logic in repo/http.go. * Before: the `hasAccess` only allow `service == "upload-pack" || service == "receive-pack"` * After: unrelated code is removed. No need to call ToTrustedCmdArgs anymore. Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
This commit is contained in:
parent
d987ac6bf1
commit
50111c71c3
2 changed files with 28 additions and 51 deletions
|
@ -135,8 +135,7 @@ func (c *CheckAttributeReader) Init(ctx context.Context) error {
|
||||||
|
|
||||||
c.env = append(c.env, "GIT_FLUSH=1")
|
c.env = append(c.env, "GIT_FLUSH=1")
|
||||||
|
|
||||||
// The empty "--" comes from #16773 , and it seems unnecessary because nothing else would be added later.
|
c.cmd.AddDynamicArguments(c.Attributes...)
|
||||||
c.cmd.AddDynamicArguments(c.Attributes...).AddArguments("--")
|
|
||||||
|
|
||||||
var err error
|
var err error
|
||||||
|
|
||||||
|
|
|
@ -424,60 +424,40 @@ func (h *serviceHandler) sendFile(contentType, file string) {
|
||||||
// one or more key=value pairs separated by colons
|
// one or more key=value pairs separated by colons
|
||||||
var safeGitProtocolHeader = regexp.MustCompile(`^[0-9a-zA-Z]+=[0-9a-zA-Z]+(:[0-9a-zA-Z]+=[0-9a-zA-Z]+)*$`)
|
var safeGitProtocolHeader = regexp.MustCompile(`^[0-9a-zA-Z]+=[0-9a-zA-Z]+(:[0-9a-zA-Z]+=[0-9a-zA-Z]+)*$`)
|
||||||
|
|
||||||
func getGitConfig(ctx gocontext.Context, option, dir string) string {
|
func prepareGitCmdWithAllowedService(service string, h *serviceHandler) (*git.Command, error) {
|
||||||
out, _, err := git.NewCommand(ctx, "config").AddDynamicArguments(option).RunStdString(&git.RunOpts{Dir: dir})
|
if service == "receive-pack" && h.cfg.ReceivePack {
|
||||||
if err != nil {
|
return git.NewCommand(h.r.Context(), "receive-pack"), nil
|
||||||
log.Error("%v - %s", err, out)
|
|
||||||
}
|
}
|
||||||
return out[0 : len(out)-1]
|
if service == "upload-pack" && h.cfg.UploadPack {
|
||||||
|
return git.NewCommand(h.r.Context(), "upload-pack"), nil
|
||||||
|
}
|
||||||
|
|
||||||
|
return nil, fmt.Errorf("service %q is not allowed", service)
|
||||||
}
|
}
|
||||||
|
|
||||||
func getConfigSetting(ctx gocontext.Context, service, dir string) bool {
|
func serviceRPC(h *serviceHandler, service string) {
|
||||||
service = strings.ReplaceAll(service, "-", "")
|
|
||||||
setting := getGitConfig(ctx, "http."+service, dir)
|
|
||||||
|
|
||||||
if service == "uploadpack" {
|
|
||||||
return setting != "false"
|
|
||||||
}
|
|
||||||
|
|
||||||
return setting == "true"
|
|
||||||
}
|
|
||||||
|
|
||||||
func hasAccess(ctx gocontext.Context, service string, h serviceHandler, checkContentType bool) bool {
|
|
||||||
if checkContentType {
|
|
||||||
if h.r.Header.Get("Content-Type") != fmt.Sprintf("application/x-git-%s-request", service) {
|
|
||||||
return false
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
if !(service == "upload-pack" || service == "receive-pack") {
|
|
||||||
return false
|
|
||||||
}
|
|
||||||
if service == "receive-pack" {
|
|
||||||
return h.cfg.ReceivePack
|
|
||||||
}
|
|
||||||
if service == "upload-pack" {
|
|
||||||
return h.cfg.UploadPack
|
|
||||||
}
|
|
||||||
|
|
||||||
return getConfigSetting(ctx, service, h.dir)
|
|
||||||
}
|
|
||||||
|
|
||||||
func serviceRPC(ctx gocontext.Context, h serviceHandler, service string) {
|
|
||||||
defer func() {
|
defer func() {
|
||||||
if err := h.r.Body.Close(); err != nil {
|
if err := h.r.Body.Close(); err != nil {
|
||||||
log.Error("serviceRPC: Close: %v", err)
|
log.Error("serviceRPC: Close: %v", err)
|
||||||
}
|
}
|
||||||
}()
|
}()
|
||||||
|
|
||||||
if !hasAccess(ctx, service, h, true) {
|
expectedContentType := fmt.Sprintf("application/x-git-%s-request", service)
|
||||||
|
if h.r.Header.Get("Content-Type") != expectedContentType {
|
||||||
|
log.Error("Content-Type (%q) doesn't match expected: %q", h.r.Header.Get("Content-Type"), expectedContentType)
|
||||||
|
h.w.WriteHeader(http.StatusUnauthorized)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
cmd, err := prepareGitCmdWithAllowedService(service, h)
|
||||||
|
if err != nil {
|
||||||
|
log.Error("Failed to prepareGitCmdWithService: %v", err)
|
||||||
h.w.WriteHeader(http.StatusUnauthorized)
|
h.w.WriteHeader(http.StatusUnauthorized)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
h.w.Header().Set("Content-Type", fmt.Sprintf("application/x-git-%s-result", service))
|
h.w.Header().Set("Content-Type", fmt.Sprintf("application/x-git-%s-result", service))
|
||||||
|
|
||||||
var err error
|
|
||||||
reqBody := h.r.Body
|
reqBody := h.r.Body
|
||||||
|
|
||||||
// Handle GZIP.
|
// Handle GZIP.
|
||||||
|
@ -498,8 +478,7 @@ func serviceRPC(ctx gocontext.Context, h serviceHandler, service string) {
|
||||||
}
|
}
|
||||||
|
|
||||||
var stderr bytes.Buffer
|
var stderr bytes.Buffer
|
||||||
// the service is generated by ourselves, so it's safe to trust it
|
cmd.AddArguments("--stateless-rpc").AddDynamicArguments(h.dir)
|
||||||
cmd := git.NewCommand(h.r.Context(), git.ToTrustedCmdArgs([]string{service})...).AddArguments("--stateless-rpc").AddDynamicArguments(h.dir)
|
|
||||||
cmd.SetDescription(fmt.Sprintf("%s %s %s [repo_path: %s]", git.GitExecutable, service, "--stateless-rpc", h.dir))
|
cmd.SetDescription(fmt.Sprintf("%s %s %s [repo_path: %s]", git.GitExecutable, service, "--stateless-rpc", h.dir))
|
||||||
if err := cmd.Run(&git.RunOpts{
|
if err := cmd.Run(&git.RunOpts{
|
||||||
Dir: h.dir,
|
Dir: h.dir,
|
||||||
|
@ -520,7 +499,7 @@ func serviceRPC(ctx gocontext.Context, h serviceHandler, service string) {
|
||||||
func ServiceUploadPack(ctx *context.Context) {
|
func ServiceUploadPack(ctx *context.Context) {
|
||||||
h := httpBase(ctx)
|
h := httpBase(ctx)
|
||||||
if h != nil {
|
if h != nil {
|
||||||
serviceRPC(ctx, *h, "upload-pack")
|
serviceRPC(h, "upload-pack")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -528,7 +507,7 @@ func ServiceUploadPack(ctx *context.Context) {
|
||||||
func ServiceReceivePack(ctx *context.Context) {
|
func ServiceReceivePack(ctx *context.Context) {
|
||||||
h := httpBase(ctx)
|
h := httpBase(ctx)
|
||||||
if h != nil {
|
if h != nil {
|
||||||
serviceRPC(ctx, *h, "receive-pack")
|
serviceRPC(h, "receive-pack")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -537,7 +516,7 @@ func getServiceType(r *http.Request) string {
|
||||||
if !strings.HasPrefix(serviceType, "git-") {
|
if !strings.HasPrefix(serviceType, "git-") {
|
||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
return strings.Replace(serviceType, "git-", "", 1)
|
return strings.TrimPrefix(serviceType, "git-")
|
||||||
}
|
}
|
||||||
|
|
||||||
func updateServerInfo(ctx gocontext.Context, dir string) []byte {
|
func updateServerInfo(ctx gocontext.Context, dir string) []byte {
|
||||||
|
@ -563,16 +542,15 @@ func GetInfoRefs(ctx *context.Context) {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
h.setHeaderNoCache()
|
h.setHeaderNoCache()
|
||||||
if hasAccess(ctx, getServiceType(h.r), *h, false) {
|
service := getServiceType(h.r)
|
||||||
service := getServiceType(h.r)
|
cmd, err := prepareGitCmdWithAllowedService(service, h)
|
||||||
|
if err == nil {
|
||||||
if protocol := h.r.Header.Get("Git-Protocol"); protocol != "" && safeGitProtocolHeader.MatchString(protocol) {
|
if protocol := h.r.Header.Get("Git-Protocol"); protocol != "" && safeGitProtocolHeader.MatchString(protocol) {
|
||||||
h.environ = append(h.environ, "GIT_PROTOCOL="+protocol)
|
h.environ = append(h.environ, "GIT_PROTOCOL="+protocol)
|
||||||
}
|
}
|
||||||
h.environ = append(os.Environ(), h.environ...)
|
h.environ = append(os.Environ(), h.environ...)
|
||||||
|
|
||||||
// the service is generated by ourselves, so we can trust it
|
refs, _, err := cmd.AddArguments("--stateless-rpc", "--advertise-refs", ".").RunStdBytes(&git.RunOpts{Env: h.environ, Dir: h.dir})
|
||||||
refs, _, err := git.NewCommand(ctx, git.ToTrustedCmdArgs([]string{service})...).AddArguments("--stateless-rpc", "--advertise-refs", ".").RunStdBytes(&git.RunOpts{Env: h.environ, Dir: h.dir})
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
log.Error(fmt.Sprintf("%v - %s", err, string(refs)))
|
log.Error(fmt.Sprintf("%v - %s", err, string(refs)))
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue