Merge branch 'release/v1.23' into lunny/add_webhook_test2

Author: GiteaBot

modules/git/commit_test.go

@@ -360,5 +360,5 @@ func Test_GetCommitBranchStart(t *testing.T) {
 	startCommitID, err := repo.GetCommitBranchStart(os.Environ(), "branch1", commit.ID.String())
 	assert.NoError(t, err)
 	assert.NotEmpty(t, startCommitID)
-	assert.EqualValues(t, "9c9aef8dd84e02bc7ec12641deb4c930a7c30185", startCommitID)
+	assert.EqualValues(t, "95bb4d39648ee7e325106df01a621c530863a653", startCommitID)
 }

modules/git/repo_commit.go

@@ -519,6 +519,7 @@ func (repo *Repository) AddLastCommitCache(cacheKey, fullName, sha string) error
 	return nil
 }
 
+// GetCommitBranchStart returns the commit where the branch diverged
 func (repo *Repository) GetCommitBranchStart(env []string, branch, endCommitID string) (string, error) {
 	cmd := NewCommand(repo.Ctx, "log", prettyLogFormat)
 	cmd.AddDynamicArguments(endCommitID)
@@ -533,19 +534,18 @@ func (repo *Repository) GetCommitBranchStart(env []string, branch, endCommitID s
 
 	parts := bytes.Split(bytes.TrimSpace(stdout), []byte{'\n'})
 
-	var startCommitID string
+	// check the commits one by one until we find a commit contained by another branch
+	// and we think this commit is the divergence point
 	for _, commitID := range parts {
 		branches, err := repo.getBranches(env, string(commitID), 2)
 		if err != nil {
 			return "", err
 		}
 		for _, b := range branches {
 			if b != branch {
-				return startCommitID, nil
+				return string(commitID), nil
 			}
 		}
-
-		startCommitID = string(commitID)
 	}
 
 	return "", nil

modules/httplib/request.go

@@ -99,10 +99,10 @@ func (r *Request) Param(key, value string) *Request {
 	return r
 }
 
-// Body adds request raw body.
-// it supports string and []byte.
+// Body adds request raw body. It supports string, []byte and io.Reader as body.
 func (r *Request) Body(data any) *Request {
 	switch t := data.(type) {
+	case nil: // do nothing
 	case string:
 		bf := bytes.NewBufferString(t)
 		r.req.Body = io.NopCloser(bf)
@@ -111,6 +111,12 @@ func (r *Request) Body(data any) *Request {
 		bf := bytes.NewBuffer(t)
 		r.req.Body = io.NopCloser(bf)
 		r.req.ContentLength = int64(len(t))
+	case io.ReadCloser:
+		r.req.Body = t
+	case io.Reader:
+		r.req.Body = io.NopCloser(t)
+	default:
+		panic(fmt.Sprintf("unsupported request body type %T", t))
 	}
 	return r
 }
@@ -141,7 +147,7 @@ func (r *Request) getResponse() (*http.Response, error) {
 		}
 	} else if r.req.Method == "POST" && r.req.Body == nil && len(paramBody) > 0 {
 		r.Header("Content-Type", "application/x-www-form-urlencoded")
-		r.Body(paramBody)
+		r.Body(paramBody) // string
 	}
 
 	var err error
@@ -185,6 +191,7 @@ func (r *Request) getResponse() (*http.Response, error) {
 }
 
 // Response executes request client gets response manually.
+// Caller MUST close the response body if no error occurs
 func (r *Request) Response() (*http.Response, error) {
 	return r.getResponse()
 }

modules/lfs/http_client.go

@@ -72,10 +72,14 @@ func (c *HTTPClient) batch(ctx context.Context, operation string, objects []Poin
 
 	url := fmt.Sprintf("%s/objects/batch", c.endpoint)
 
+	// Original:  In some lfs server implementations, they require the ref attribute. #32838
 	// `ref` is an "optional object describing the server ref that the objects belong to"
-	// but some (incorrect) lfs servers require it, so maybe adding an empty ref here doesn't break the correct ones.
+	// but some (incorrect) lfs servers like aliyun require it, so maybe adding an empty ref here doesn't break the correct ones.
 	// https://github.com/git-lfs/git-lfs/blob/a32a02b44bf8a511aa14f047627c49e1a7fd5021/docs/api/batch.md?plain=1#L37
-	request := &BatchRequest{operation, c.transferNames(), &Reference{}, objects}
+	//
+	// UPDATE: it can't use "empty ref" here because it breaks others like https://github.com/go-gitea/gitea/issues/33453
+	request := &BatchRequest{operation, c.transferNames(), nil, objects}
+
 	payload := new(bytes.Buffer)
 	err := json.NewEncoder(payload).Encode(request)
 	if err != nil {

modules/lfstransfer/backend/backend.go

@@ -4,7 +4,6 @@
 package backend
 
 import (
-	"bytes"
 	"context"
 	"encoding/base64"
 	"fmt"
@@ -29,7 +28,7 @@ var Capabilities = []string{
 	"locking",
 }
 
-var _ transfer.Backend = &GiteaBackend{}
+var _ transfer.Backend = (*GiteaBackend)(nil)
 
 // GiteaBackend is an adapter between git-lfs-transfer library and Gitea's internal LFS API
 type GiteaBackend struct {
@@ -78,17 +77,17 @@ func (g *GiteaBackend) Batch(_ string, pointers []transfer.BatchItem, args trans
 		headerAccept:            mimeGitLFS,
 		headerContentType:       mimeGitLFS,
 	}
-	req := newInternalRequest(g.ctx, url, http.MethodPost, headers, bodyBytes)
+	req := newInternalRequestLFS(g.ctx, url, http.MethodPost, headers, bodyBytes)
 	resp, err := req.Response()
 	if err != nil {
 		g.logger.Log("http request error", err)
 		return nil, err
 	}
+	defer resp.Body.Close()
 	if resp.StatusCode != http.StatusOK {
 		g.logger.Log("http statuscode error", resp.StatusCode, statusCodeToErr(resp.StatusCode))
 		return nil, statusCodeToErr(resp.StatusCode)
 	}
-	defer resp.Body.Close()
 	respBytes, err := io.ReadAll(resp.Body)
 	if err != nil {
 		g.logger.Log("http read error", err)
@@ -158,8 +157,7 @@ func (g *GiteaBackend) Batch(_ string, pointers []transfer.BatchItem, args trans
 	return pointers, nil
 }
 
-// Download implements transfer.Backend. The returned reader must be closed by the
-// caller.
+// Download implements transfer.Backend. The returned reader must be closed by the caller.
 func (g *GiteaBackend) Download(oid string, args transfer.Args) (io.ReadCloser, int64, error) {
 	idMapStr, exists := args[argID]
 	if !exists {
@@ -187,25 +185,25 @@ func (g *GiteaBackend) Download(oid string, args transfer.Args) (io.ReadCloser,
 		headerGiteaInternalAuth: g.internalAuth,
 		headerAccept:            mimeOctetStream,
 	}
-	req := newInternalRequest(g.ctx, url, http.MethodGet, headers, nil)
+	req := newInternalRequestLFS(g.ctx, url, http.MethodGet, headers, nil)
 	resp, err := req.Response()
 	if err != nil {
-		return nil, 0, err
+		return nil, 0, fmt.Errorf("failed to get response: %w", err)
 	}
+	// no need to close the body here by "defer resp.Body.Close()", see below
 	if resp.StatusCode != http.StatusOK {
 		return nil, 0, statusCodeToErr(resp.StatusCode)
 	}
-	defer resp.Body.Close()
-	respBytes, err := io.ReadAll(resp.Body)
+
+	respSize, err := strconv.ParseInt(resp.Header.Get("X-Gitea-LFS-Content-Length"), 10, 64)
 	if err != nil {
-		return nil, 0, err
+		return nil, 0, fmt.Errorf("failed to parse content length: %w", err)
 	}
-	respSize := int64(len(respBytes))
-	respBuf := io.NopCloser(bytes.NewBuffer(respBytes))
-	return respBuf, respSize, nil
+	// transfer.Backend will check io.Closer interface and close this Body reader
+	return resp.Body, respSize, nil
 }
 
-// StartUpload implements transfer.Backend.
+// Upload implements transfer.Backend.
 func (g *GiteaBackend) Upload(oid string, size int64, r io.Reader, args transfer.Args) error {
 	idMapStr, exists := args[argID]
 	if !exists {
@@ -234,15 +232,14 @@ func (g *GiteaBackend) Upload(oid string, size int64, r io.Reader, args transfer
 		headerContentType:       mimeOctetStream,
 		headerContentLength:     strconv.FormatInt(size, 10),
 	}
-	reqBytes, err := io.ReadAll(r)
-	if err != nil {
-		return err
-	}
-	req := newInternalRequest(g.ctx, url, http.MethodPut, headers, reqBytes)
+
+	req := newInternalRequestLFS(g.ctx, url, http.MethodPut, headers, nil)
+	req.Body(r)
 	resp, err := req.Response()
 	if err != nil {
 		return err
 	}
+	defer resp.Body.Close()
 	if resp.StatusCode != http.StatusOK {
 		return statusCodeToErr(resp.StatusCode)
 	}
@@ -284,11 +281,12 @@ func (g *GiteaBackend) Verify(oid string, size int64, args transfer.Args) (trans
 		headerAccept:            mimeGitLFS,
 		headerContentType:       mimeGitLFS,
 	}
-	req := newInternalRequest(g.ctx, url, http.MethodPost, headers, bodyBytes)
+	req := newInternalRequestLFS(g.ctx, url, http.MethodPost, headers, bodyBytes)
 	resp, err := req.Response()
 	if err != nil {
 		return transfer.NewStatus(transfer.StatusInternalServerError), err
 	}
+	defer resp.Body.Close()
 	if resp.StatusCode != http.StatusOK {
 		return transfer.NewStatus(uint32(resp.StatusCode), http.StatusText(resp.StatusCode)), statusCodeToErr(resp.StatusCode)
 	}

modules/lfstransfer/backend/lock.go

@@ -50,7 +50,7 @@ func (g *giteaLockBackend) Create(path, refname string) (transfer.Lock, error) {
 		headerAccept:            mimeGitLFS,
 		headerContentType:       mimeGitLFS,
 	}
-	req := newInternalRequest(g.ctx, url, http.MethodPost, headers, bodyBytes)
+	req := newInternalRequestLFS(g.ctx, url, http.MethodPost, headers, bodyBytes)
 	resp, err := req.Response()
 	if err != nil {
 		g.logger.Log("http request error", err)
@@ -102,7 +102,7 @@ func (g *giteaLockBackend) Unlock(lock transfer.Lock) error {
 		headerAccept:            mimeGitLFS,
 		headerContentType:       mimeGitLFS,
 	}
-	req := newInternalRequest(g.ctx, url, http.MethodPost, headers, bodyBytes)
+	req := newInternalRequestLFS(g.ctx, url, http.MethodPost, headers, bodyBytes)
 	resp, err := req.Response()
 	if err != nil {
 		g.logger.Log("http request error", err)
@@ -185,7 +185,7 @@ func (g *giteaLockBackend) queryLocks(v url.Values) ([]transfer.Lock, string, er
 		headerAccept:            mimeGitLFS,
 		headerContentType:       mimeGitLFS,
 	}
-	req := newInternalRequest(g.ctx, url, http.MethodGet, headers, nil)
+	req := newInternalRequestLFS(g.ctx, url, http.MethodGet, headers, nil)
 	resp, err := req.Response()
 	if err != nil {
 		g.logger.Log("http request error", err)

modules/lfstransfer/backend/util.go

@@ -5,15 +5,12 @@ package backend
 
 import (
 	"context"
-	"crypto/tls"
 	"fmt"
-	"net"
+	"io"
 	"net/http"
-	"time"
 
 	"code.gitea.io/gitea/modules/httplib"
-	"code.gitea.io/gitea/modules/proxyprotocol"
-	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/private"
 
 	"github.com/charmbracelet/git-lfs-transfer/transfer"
 )
@@ -89,53 +86,19 @@ func statusCodeToErr(code int) error {
 	}
 }
 
-func newInternalRequest(ctx context.Context, url, method string, headers map[string]string, body []byte) *httplib.Request {
-	req := httplib.NewRequest(url, method).
-		SetContext(ctx).
-		SetTimeout(10*time.Second, 60*time.Second).
-		SetTLSClientConfig(&tls.Config{
-			InsecureSkipVerify: true,
-		})
-
-	if setting.Protocol == setting.HTTPUnix {
-		req.SetTransport(&http.Transport{
-			DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) {
-				var d net.Dialer
-				conn, err := d.DialContext(ctx, "unix", setting.HTTPAddr)
-				if err != nil {
-					return conn, err
-				}
-				if setting.LocalUseProxyProtocol {
-					if err = proxyprotocol.WriteLocalHeader(conn); err != nil {
-						_ = conn.Close()
-						return nil, err
-					}
-				}
-				return conn, err
-			},
-		})
-	} else if setting.LocalUseProxyProtocol {
-		req.SetTransport(&http.Transport{
-			DialContext: func(ctx context.Context, network, address string) (net.Conn, error) {
-				var d net.Dialer
-				conn, err := d.DialContext(ctx, network, address)
-				if err != nil {
-					return conn, err
-				}
-				if err = proxyprotocol.WriteLocalHeader(conn); err != nil {
-					_ = conn.Close()
-					return nil, err
-				}
-				return conn, err
-			},
-		})
-	}
-
+func newInternalRequestLFS(ctx context.Context, url, method string, headers map[string]string, body any) *httplib.Request {
+	req := private.NewInternalRequest(ctx, url, method)
 	for k, v := range headers {
 		req.Header(k, v)
 	}
-
-	req.Body(body)
-
+	switch body := body.(type) {
+	case nil: // do nothing
+	case []byte:
+		req.Body(body) // []byte
+	case io.Reader:
+		req.Body(body) // io.Reader or io.ReadCloser
+	default:
+		panic(fmt.Sprintf("unsupported request body type %T", body))
+	}
 	return req
 }

modules/private/actions.go

@@ -17,7 +17,7 @@ type GenerateTokenRequest struct {
 func GenerateActionsRunnerToken(ctx context.Context, scope string) (*ResponseText, ResponseExtra) {
 	reqURL := setting.LocalURL + "api/internal/actions/generate_actions_runner_token"
 
-	req := newInternalRequest(ctx, reqURL, "POST", GenerateTokenRequest{
+	req := newInternalRequestAPI(ctx, reqURL, "POST", GenerateTokenRequest{
 		Scope: scope,
 	})
 

modules/private/hook.go

@@ -85,7 +85,7 @@ type HookProcReceiveRefResult struct {
 // HookPreReceive check whether the provided commits are allowed
 func HookPreReceive(ctx context.Context, ownerName, repoName string, opts HookOptions) ResponseExtra {
 	reqURL := setting.LocalURL + fmt.Sprintf("api/internal/hook/pre-receive/%s/%s", url.PathEscape(ownerName), url.PathEscape(repoName))
-	req := newInternalRequest(ctx, reqURL, "POST", opts)
+	req := newInternalRequestAPI(ctx, reqURL, "POST", opts)
 	req.SetReadWriteTimeout(time.Duration(60+len(opts.OldCommitIDs)) * time.Second)
 	_, extra := requestJSONResp(req, &ResponseText{})
 	return extra
@@ -94,7 +94,7 @@ func HookPreReceive(ctx context.Context, ownerName, repoName string, opts HookOp
 // HookPostReceive updates services and users
 func HookPostReceive(ctx context.Context, ownerName, repoName string, opts HookOptions) (*HookPostReceiveResult, ResponseExtra) {
 	reqURL := setting.LocalURL + fmt.Sprintf("api/internal/hook/post-receive/%s/%s", url.PathEscape(ownerName), url.PathEscape(repoName))
-	req := newInternalRequest(ctx, reqURL, "POST", opts)
+	req := newInternalRequestAPI(ctx, reqURL, "POST", opts)
 	req.SetReadWriteTimeout(time.Duration(60+len(opts.OldCommitIDs)) * time.Second)
 	return requestJSONResp(req, &HookPostReceiveResult{})
 }
@@ -103,7 +103,7 @@ func HookPostReceive(ctx context.Context, ownerName, repoName string, opts HookO
 func HookProcReceive(ctx context.Context, ownerName, repoName string, opts HookOptions) (*HookProcReceiveResult, ResponseExtra) {
 	reqURL := setting.LocalURL + fmt.Sprintf("api/internal/hook/proc-receive/%s/%s", url.PathEscape(ownerName), url.PathEscape(repoName))
 
-	req := newInternalRequest(ctx, reqURL, "POST", opts)
+	req := newInternalRequestAPI(ctx, reqURL, "POST", opts)
 	req.SetReadWriteTimeout(time.Duration(60+len(opts.OldCommitIDs)) * time.Second)
 	return requestJSONResp(req, &HookProcReceiveResult{})
 }
@@ -115,15 +115,15 @@ func SetDefaultBranch(ctx context.Context, ownerName, repoName, branch string) R
 		url.PathEscape(repoName),
 		url.PathEscape(branch),
 	)
-	req := newInternalRequest(ctx, reqURL, "POST")
+	req := newInternalRequestAPI(ctx, reqURL, "POST")
 	_, extra := requestJSONResp(req, &ResponseText{})
 	return extra
 }
 
 // SSHLog sends ssh error log response
 func SSHLog(ctx context.Context, isErr bool, msg string) error {
 	reqURL := setting.LocalURL + "api/internal/ssh/log"
-	req := newInternalRequest(ctx, reqURL, "POST", &SSHLogOption{IsError: isErr, Message: msg})
+	req := newInternalRequestAPI(ctx, reqURL, "POST", &SSHLogOption{IsError: isErr, Message: msg})
 	_, extra := requestJSONResp(req, &ResponseText{})
 	return extra.Error
 }