Merging of PRs not possible for some projects since gitea 1.16

[smainz]

Gitea Version

Gitea Version: 1.16.1

Git Version

from official docker image

Operating System

Linux (docker image)

How are you running Gitea?

gitea is running with the official docker image

docker image ls
...
gitea/gitea                                 1.16                                  3fccf68da11d
...

Database

PostgreSQL

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Description

Error

After upgrading our gitea server to version 1.16.1 we are encountering problems during merging of PRs to protected branches. The pre-receive-hook is denying the merge commits from the ui. In the logs the warning from line 288 of file hook_pre_receive.go ("Forbidden: User %d is not allowed to push to protected branch: %s in %-v") can be observed.

After debugging the code we found out that the pull request id cannot be parsed from the payload of the internal post-request to the pre-receive-hook and thus, a wrong code path is used.

The go-json library is throwing an error for the empty GitPushOptions for an input string of exactly this size.

The error of this library is not checked in

gitea/routers/private/internal.go

Lines 42 to 46 in 704bdf8

	return web.Wrap(func(ctx *context.PrivateContext) {
	theObj := reflect.New(tp).Interface() // create a new form obj for every request but not use obj directly
	binding.Bind(ctx.Req, theObj)
	web.SetForm(ctx, theObj)
	})

which results in the message above. The code should read:

   return web.Wrap(func(ctx *context.PrivateContext) {
        var theObj = reflect.New(tp).Interface() // create a new form obj for every request but not use obj directly
        err := binding.Bind(ctx.Req, theObj)
        if err != nil {
            log.Error("Error %#v", err)  // what else to do here panic()?
        }
        web.SetForm(ctx, theObj)
    })

Workaround

Until this library is fixed, not much can be done, but shifting the place of the GitPushOptions in the type HookOptions struct circumvents the issue. See

gitea/modules/private/hook.go

Lines 48 to 61 in 704bdf8

	type HookOptions struct {
	OldCommitIDs []string
	NewCommitIDs []string
	RefFullNames []string
	UserID int64
	UserName string
	GitObjectDirectory string
	GitAlternativeObjectDirectories string
	GitQuarantinePath string
	GitPushOptions GitPushOptions
	PullRequestID int64
	IsDeployKey bool
	IsWiki bool
	}

Root Cause with example code

The json-Library used for binding the payload of a web request to a go-structure has issues as seen in the code example below.

package main

import (
	"encoding/json"
	"fmt"
	"reflect"
	"strings"

	jsonLibrary "github.com/goccy/go-json"
)

func main() {

	const jsonString = `{"OldCommitIDs":["4bd3240099b6ee57a119885831259e314e46f0a5"],"NewCommitIDs":["3a48205c4ed8429ce309bb3d17bf08ce14c2b3dc"],"RefFullNames":["refs/heads/develop"],"UserID":27,"UserName":"","GitObjectDirectory":"/data/gitea/repositories/xxxxxxxxxx/import-service.git/./objects/incoming-MODbLf","GitAlternativeObjectDirectories":"/data/gitea/repositories/xxxxxxxxxx/import-service.git/./objects","GitQuarantinePath":"/data/gitea/repositories/xxxxxxxxxx/import-service.git/./objects/incoming-MODbLf","GitPushOptions":{},"PullRequestID":504,"IsDeployKey":false,"IsWiki":false}`

	type GitPushOptions map[string]string

	type HookOptions struct {
		OldCommitIDs                    []string
		NewCommitIDs                    []string
		RefFullNames                    []string
		UserID                          int64
		UserName                        string
		GitObjectDirectory              string
		GitAlternativeObjectDirectories string
		GitQuarantinePath               string
		GitPushOptions                  GitPushOptions
		PullRequestID                   int64
		IsDeployKey                     bool
		IsWiki                          bool
	}

	fmt.Println("Build in Json")
	jsonStruct := reflect.New(reflect.TypeOf(HookOptions{})).Interface()
	err := json.NewDecoder(strings.NewReader(jsonString)).Decode(jsonStruct)
	fmt.Printf("Error %#v\n", err)
	fmt.Printf("Object %#v\n\n", jsonStruct)

	fmt.Println("Json Library Go-Json")
	jsonStruct2 := reflect.New(reflect.TypeOf(HookOptions{})).Interface()
	err2 := jsonLibrary.NewDecoder(strings.NewReader(jsonString)).Decode(jsonStruct2)
	fmt.Printf("Error %#v\n", err2)
	fmt.Printf("Object %#v\n", jsonStruct2)
}

An issue has been opend for the go-json library: goccy/go-json#337

Screenshots

No response

[Gusted]

@smainz Do you happen to have a stacktrace?, go-json isn't a directly a dependency, it seems that it's only incorporated by xorm, this is just to confirm where to update the dependency.

It is: https://gitea.com/go-chi/binding

[Gusted]

https://gitea.com/go-chi/binding/pulls/11

[6543]

got merged ...