Use LFS object size instead of blob size when viewing a LFS file (#35679) (#35680)

Backport #35679 by surya-purohit

shows the main LFS filesize instead of the pointer filesize when viewing
a file

Co-authored-by: Surya Purohit <suryaprakash.sharma@sourcefuse.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

go.mod

@@ -35,7 +35,7 @@ require (
 	github.com/bohde/codel v0.2.0
 	github.com/buildkite/terminal-to-html/v3 v3.16.8
 	github.com/caddyserver/certmagic v0.24.0
-	github.com/charmbracelet/git-lfs-transfer v0.2.0
+	github.com/charmbracelet/git-lfs-transfer v0.1.1-0.20251013092601-6327009efd21
 	github.com/chi-middleware/proxy v1.1.1
 	github.com/dimiro1/reply v0.0.0-20200315094148-d0136a4c9e21
 	github.com/djherbis/buffer v1.2.0
@@ -56,7 +56,7 @@ require (
 	github.com/go-co-op/gocron v1.37.0
 	github.com/go-enry/go-enry/v2 v2.9.2
 	github.com/go-git/go-billy/v5 v5.6.2
-	github.com/go-git/go-git/v5 v5.16.2
+	github.com/go-git/go-git/v5 v5.16.3
 	github.com/go-ldap/ldap/v3 v3.4.11
 	github.com/go-redsync/redsync/v4 v4.13.0
 	github.com/go-sql-driver/mysql v1.9.3
@@ -121,7 +121,7 @@ require (
 	golang.org/x/net v0.44.0
 	golang.org/x/oauth2 v0.30.0
 	golang.org/x/sync v0.17.0
-	golang.org/x/sys v0.36.0
+	golang.org/x/sys v0.37.0
 	golang.org/x/text v0.30.0
 	google.golang.org/grpc v1.75.0
 	google.golang.org/protobuf v1.36.8
@@ -298,9 +298,6 @@ replace github.com/hashicorp/go-version => github.com/6543/go-version v1.3.1
 
 replace github.com/nektos/act => gitea.com/gitea/act v0.261.7-0.20251003180512-ac6e4b751763
 
-// TODO: the only difference is in `PutObject`: the fork doesn't use `NewVerifyingReader(r, sha256.New(), oid, expectedSize)`, need to figure out why
-replace github.com/charmbracelet/git-lfs-transfer => gitea.com/gitea/git-lfs-transfer v0.2.0
-
 replace git.sr.ht/~mariusor/go-xsd-duration => gitea.com/gitea/go-xsd-duration v0.0.0-20220703122237-02e73435a078
 
 exclude github.com/gofrs/uuid v3.2.0+incompatible

go.sum

@@ -33,8 +33,6 @@ filippo.io/edwards25519 v1.1.0 h1:FNf4tywRC1HmFuKW5xopWpigGjJKiJSV0Cqo0cJWDaA=
 filippo.io/edwards25519 v1.1.0/go.mod h1:BxyFTGdWcka3PhytdK4V28tE5sGfRvvvRV7EaN4VDT4=
 gitea.com/gitea/act v0.261.7-0.20251003180512-ac6e4b751763 h1:ohdxegvslDEllZmRNDqpKun6L4Oq81jNdEDtGgHEV2c=
 gitea.com/gitea/act v0.261.7-0.20251003180512-ac6e4b751763/go.mod h1:Pg5C9kQY1CEA3QjthjhlrqOC/QOT5NyWNjOjRHw23Ok=
-gitea.com/gitea/git-lfs-transfer v0.2.0 h1:baHaNoBSRaeq/xKayEXwiDQtlIjps4Ac/Ll4KqLMB40=
-gitea.com/gitea/git-lfs-transfer v0.2.0/go.mod h1:UrXUCm3xLQkq15fu7qlXHUMlrhdlXHoi13KH2Dfiits=
 gitea.com/gitea/go-xsd-duration v0.0.0-20220703122237-02e73435a078 h1:BAFmdZpRW7zMQZQDClaCWobRj9uL1MR3MzpCVJvc5s4=
 gitea.com/gitea/go-xsd-duration v0.0.0-20220703122237-02e73435a078/go.mod h1:g/V2Hjas6Z1UHUp4yIx6bATpNzJ7DYtD0FG3+xARWxs=
 gitea.com/go-chi/binding v0.0.0-20240430071103-39a851e106ed h1:EZZBtilMLSZNWtHHcgq2mt6NSGhJSZBuduAlinMEmso=
@@ -219,6 +217,8 @@ github.com/cention-sany/utf7 v0.0.0-20170124080048-26cad61bd60a h1:MISbI8sU/PSK/
 github.com/cention-sany/utf7 v0.0.0-20170124080048-26cad61bd60a/go.mod h1:2GxOXOlEPAMFPfp014mK1SWq8G8BN8o7/dfYqJrVGn8=
 github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs=
 github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
+github.com/charmbracelet/git-lfs-transfer v0.1.1-0.20251013092601-6327009efd21 h1:2d64+4Jek9vjYwhY93AjbleiVH+AeWvPwPmDi1mfKFQ=
+github.com/charmbracelet/git-lfs-transfer v0.1.1-0.20251013092601-6327009efd21/go.mod h1:fNlYtCHWTRC8MofQERZkVUNUWaOvZeTBqHn/amSbKZI=
 github.com/chi-middleware/proxy v1.1.1 h1:4HaXUp8o2+bhHr1OhVy+VjN0+L7/07JDcn6v7YrTjrQ=
 github.com/chi-middleware/proxy v1.1.1/go.mod h1:jQwMEJct2tz9VmtCELxvnXoMfa+SOdikvbVJVHv/M+0=
 github.com/chromedp/cdproto v0.0.0-20230802225258-3cf4e6d46a89/go.mod h1:GKljq0VrfU4D5yc+2qA6OVr8pmO/MBbPEWqWQ/oqGEs=
@@ -339,8 +339,8 @@ github.com/go-git/go-billy/v5 v5.6.2 h1:6Q86EsPXMa7c3YZ3aLAQsMA0VlWmy43r6FHqa/UN
 github.com/go-git/go-billy/v5 v5.6.2/go.mod h1:rcFC2rAsp/erv7CMz9GczHcuD0D32fWzH+MJAU+jaUU=
 github.com/go-git/go-git-fixtures/v4 v4.3.2-0.20231010084843-55a94097c399 h1:eMje31YglSBqCdIqdhKBW8lokaMrL3uTkpGYlE2OOT4=
 github.com/go-git/go-git-fixtures/v4 v4.3.2-0.20231010084843-55a94097c399/go.mod h1:1OCfN199q1Jm3HZlxleg+Dw/mwps2Wbk9frAWm+4FII=
-github.com/go-git/go-git/v5 v5.16.2 h1:fT6ZIOjE5iEnkzKyxTHK1W4HGAsPhqEqiSAssSO77hM=
-github.com/go-git/go-git/v5 v5.16.2/go.mod h1:4Ge4alE/5gPs30F2H1esi2gPd69R0C39lolkucHBOp8=
+github.com/go-git/go-git/v5 v5.16.3 h1:Z8BtvxZ09bYm/yYNgPKCzgWtaRqDTgIKRgIRHBfU6Z8=
+github.com/go-git/go-git/v5 v5.16.3/go.mod h1:4Ge4alE/5gPs30F2H1esi2gPd69R0C39lolkucHBOp8=
 github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU=
 github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8=
 github.com/go-ini/ini v1.67.0 h1:z6ZrTEZqSWOTyH2FlglNbNgARyHG8oLW9gMELqKr06A=
@@ -978,8 +978,8 @@ golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
 golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
 golang.org/x/sys v0.29.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
 golang.org/x/sys v0.30.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
-golang.org/x/sys v0.36.0 h1:KVRy2GtZBrk1cBYA7MKu5bEZFxQk4NIDV6RLVcC8o0k=
-golang.org/x/sys v0.36.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks=
+golang.org/x/sys v0.37.0 h1:fdNQudmxPjkdUTPnLn5mdQv7Zwvbvpaxqs831goi9kQ=
+golang.org/x/sys v0.37.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks=
 golang.org/x/telemetry v0.0.0-20240228155512-f48c80bd79b2/go.mod h1:TeRTkGYfJXctD9OcfyVLyj2J3IxLnKwHJR8f4D8a3YE=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
 golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=

modules/git/url/url_test.go

@@ -34,12 +34,12 @@ func TestParseGitURLs(t *testing.T) {
 			},
 		},
 		{
-			kase: "git@[fe80:14fc:cec5:c174:d88%2510]:go-gitea/gitea.git",
+			kase: "git@[fe80::14fc:cec5:c174:d88%2510]:go-gitea/gitea.git",
 			expected: &GitURL{
 				URL: &url.URL{
 					Scheme: "ssh",
 					User:   url.User("git"),
-					Host:   "[fe80:14fc:cec5:c174:d88%10]",
+					Host:   "[fe80::14fc:cec5:c174:d88%10]",
 					Path:   "go-gitea/gitea.git",
 				},
 				extraMark: 1,
@@ -137,11 +137,11 @@ func TestParseGitURLs(t *testing.T) {
 			},
 		},
 		{
-			kase: "https://[fe80:14fc:cec5:c174:d88%2510]:20/go-gitea/gitea.git",
+			kase: "https://[fe80::14fc:cec5:c174:d88%2510]:20/go-gitea/gitea.git",
 			expected: &GitURL{
 				URL: &url.URL{
 					Scheme: "https",
-					Host:   "[fe80:14fc:cec5:c174:d88%10]:20",
+					Host:   "[fe80::14fc:cec5:c174:d88%10]:20",
 					Path:   "/go-gitea/gitea.git",
 				},
 				extraMark: 0,

modules/httplib/request.go

@@ -7,54 +7,53 @@ package httplib
 import (
 	"bytes"
 	"context"
-	"crypto/tls"
-	"errors"
 	"fmt"
 	"io"
 	"net"
 	"net/http"
 	"net/url"
 	"strings"
+	"sync"
 	"time"
 )
 
-var defaultSetting = Settings{"GiteaServer", 60 * time.Second, 60 * time.Second, nil, nil}
-
-// newRequest returns *Request with specific method
-func newRequest(url, method string) *Request {
-	var resp http.Response
-	req := http.Request{
-		Method:     method,
-		Header:     make(http.Header),
-		Proto:      "HTTP/1.1",
-		ProtoMajor: 1,
-		ProtoMinor: 1,
+var defaultTransport = sync.OnceValue(func() http.RoundTripper {
+	return &http.Transport{
+		Proxy:       http.ProxyFromEnvironment,
+		DialContext: DialContextWithTimeout(10 * time.Second), // it is good enough in modern days
+	}
+})
+
+func DialContextWithTimeout(timeout time.Duration) func(ctx context.Context, network, address string) (net.Conn, error) {
+	return func(ctx context.Context, network, address string) (net.Conn, error) {
+		return (&net.Dialer{Timeout: timeout}).DialContext(ctx, network, address)
 	}
-	return &Request{url, &req, map[string]string{}, defaultSetting, &resp, nil}
 }
 
-// NewRequest returns *Request with specific method
 func NewRequest(url, method string) *Request {
-	return newRequest(url, method)
+	return &Request{
+		url: url,
+		req: &http.Request{
+			Method:     method,
+			Header:     make(http.Header),
+			Proto:      "HTTP/1.1", // FIXME: from legacy httplib, it shouldn't be hardcoded
+			ProtoMajor: 1,
+			ProtoMinor: 1,
+		},
+		params: map[string]string{},
+
+		// ATTENTION: from legacy httplib, callers must pay more attention to it, it will cause annoying bugs when the response takes a long time
+		readWriteTimeout: 60 * time.Second,
+	}
 }
 
-// Settings is the default settings for http client
-type Settings struct {
-	UserAgent        string
-	ConnectTimeout   time.Duration
-	ReadWriteTimeout time.Duration
-	TLSClientConfig  *tls.Config
-	Transport        http.RoundTripper
-}
-
-// Request provides more useful methods for requesting one url than http.Request.
 type Request struct {
-	url     string
-	req     *http.Request
-	params  map[string]string
-	setting Settings
-	resp    *http.Response
-	body    []byte
+	url    string
+	req    *http.Request
+	params map[string]string
+
+	readWriteTimeout time.Duration
+	transport        http.RoundTripper
 }
 
 // SetContext sets the request's Context
@@ -63,36 +62,24 @@ func (r *Request) SetContext(ctx context.Context) *Request {
 	return r
 }
 
-// SetTimeout sets connect time out and read-write time out for BeegoRequest.
-func (r *Request) SetTimeout(connectTimeout, readWriteTimeout time.Duration) *Request {
-	r.setting.ConnectTimeout = connectTimeout
-	r.setting.ReadWriteTimeout = readWriteTimeout
+// SetTransport sets the request transport, if not set, will use httplib's default transport with environment proxy support
+// ATTENTION: the http.Transport has a connection pool, so it should be reused as much as possible, do not create a lot of transports
+func (r *Request) SetTransport(transport http.RoundTripper) *Request {
+	r.transport = transport
 	return r
 }
 
 func (r *Request) SetReadWriteTimeout(readWriteTimeout time.Duration) *Request {
-	r.setting.ReadWriteTimeout = readWriteTimeout
+	r.readWriteTimeout = readWriteTimeout
 	return r
 }
 
-// SetTLSClientConfig sets tls connection configurations if visiting https url.
-func (r *Request) SetTLSClientConfig(config *tls.Config) *Request {
-	r.setting.TLSClientConfig = config
-	return r
-}
-
-// Header add header item string in request.
+// Header set header item string in request.
 func (r *Request) Header(key, value string) *Request {
 	r.req.Header.Set(key, value)
 	return r
 }
 
-// SetTransport sets transport to
-func (r *Request) SetTransport(transport http.RoundTripper) *Request {
-	r.setting.Transport = transport
-	return r
-}
-
 // Param adds query param in to request.
 // params build query string as ?key1=value1&key2=value2...
 func (r *Request) Param(key, value string) *Request {
@@ -125,11 +112,9 @@ func (r *Request) Body(data any) *Request {
 	return r
 }
 
-func (r *Request) getResponse() (*http.Response, error) {
-	if r.resp.StatusCode != 0 {
-		return r.resp, nil
-	}
-
+// Response executes request client and returns the response.
+// Caller MUST close the response body if no error occurs.
+func (r *Request) Response() (*http.Response, error) {
 	var paramBody string
 	if len(r.params) > 0 {
 		var buf bytes.Buffer
@@ -160,59 +145,19 @@ func (r *Request) getResponse() (*http.Response, error) {
 		return nil, err
 	}
 
-	trans := r.setting.Transport
-	if trans == nil {
-		// create default transport
-		trans = &http.Transport{
-			TLSClientConfig: r.setting.TLSClientConfig,
-			Proxy:           http.ProxyFromEnvironment,
-			DialContext:     TimeoutDialer(r.setting.ConnectTimeout),
-		}
-	} else if t, ok := trans.(*http.Transport); ok {
-		if t.TLSClientConfig == nil {
-			t.TLSClientConfig = r.setting.TLSClientConfig
-		}
-		if t.DialContext == nil {
-			t.DialContext = TimeoutDialer(r.setting.ConnectTimeout)
-		}
-	}
-
 	client := &http.Client{
-		Transport: trans,
-		Timeout:   r.setting.ReadWriteTimeout,
+		Transport: r.transport,
+		Timeout:   r.readWriteTimeout,
+	}
+	if client.Transport == nil {
+		client.Transport = defaultTransport()
 	}
 
-	if len(r.setting.UserAgent) > 0 && len(r.req.Header.Get("User-Agent")) == 0 {
-		r.req.Header.Set("User-Agent", r.setting.UserAgent)
+	if r.req.Header.Get("User-Agent") == "" {
+		r.req.Header.Set("User-Agent", "GiteaHttpLib")
 	}
 
-	resp, err := client.Do(r.req)
-	if err != nil {
-		return nil, err
-	}
-	r.resp = resp
-	return resp, nil
-}
-
-// Response executes request client gets response manually.
-// Caller MUST close the response body if no error occurs
-func (r *Request) Response() (*http.Response, error) {
-	if r == nil {
-		return nil, errors.New("invalid request")
-	}
-	return r.getResponse()
-}
-
-// TimeoutDialer returns functions of connection dialer with timeout settings for http.Transport Dial field.
-func TimeoutDialer(cTimeout time.Duration) func(ctx context.Context, net, addr string) (c net.Conn, err error) {
-	return func(ctx context.Context, netw, addr string) (net.Conn, error) {
-		d := net.Dialer{Timeout: cTimeout}
-		conn, err := d.DialContext(ctx, netw, addr)
-		if err != nil {
-			return nil, err
-		}
-		return conn, nil
-	}
+	return client.Do(r.req)
 }
 
 func (r *Request) GoString() string {

modules/lfstransfer/backend/backend.go

@@ -157,7 +157,7 @@ func (g *GiteaBackend) Batch(_ string, pointers []transfer.BatchItem, args trans
 }
 
 // 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) {
+func (g *GiteaBackend) Download(oid string, args transfer.Args) (_ io.ReadCloser, _ int64, retErr error) {
 	idMapStr, exists := args[argID]
 	if !exists {
 		return nil, 0, ErrMissingID
@@ -188,7 +188,15 @@ func (g *GiteaBackend) Download(oid string, args transfer.Args) (io.ReadCloser,
 	if err != nil {
 		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
+	// We must return the ReaderCloser but not "ReadAll", to avoid OOM.
+	// "transfer.Backend" will check io.Closer interface and close the Body reader.
+	// So only close the Body when error occurs
+	defer func() {
+		if retErr != nil {
+			_ = resp.Body.Close()
+		}
+	}()
+
 	if resp.StatusCode != http.StatusOK {
 		return nil, 0, statusCodeToErr(resp.StatusCode)
 	}
@@ -197,7 +205,6 @@ func (g *GiteaBackend) Download(oid string, args transfer.Args) (io.ReadCloser,
 	if err != nil {
 		return nil, 0, fmt.Errorf("failed to parse content length: %w", err)
 	}
-	// transfer.Backend will check io.Closer interface and close this Body reader
 	return resp.Body, respSize, nil
 }
 

modules/private/internal.go

@@ -10,6 +10,7 @@ import (
 	"net/http"
 	"os"
 	"strings"
+	"sync"
 	"time"
 
 	"code.gitea.io/gitea/modules/httplib"
@@ -33,6 +34,35 @@ func getClientIP() string {
 	return strings.Fields(sshConnEnv)[0]
 }
 
+func dialContextInternalAPI(ctx context.Context, network, address string) (conn net.Conn, err error) {
+	d := net.Dialer{Timeout: 10 * time.Second}
+	if setting.Protocol == setting.HTTPUnix {
+		conn, err = d.DialContext(ctx, "unix", setting.HTTPAddr)
+	} else {
+		conn, err = d.DialContext(ctx, network, address)
+	}
+	if err != nil {
+		return nil, err
+	}
+	if setting.LocalUseProxyProtocol {
+		if err = proxyprotocol.WriteLocalHeader(conn); err != nil {
+			_ = conn.Close()
+			return nil, err
+		}
+	}
+	return conn, nil
+}
+
+var internalAPITransport = sync.OnceValue(func() http.RoundTripper {
+	return &http.Transport{
+		DialContext: dialContextInternalAPI,
+		TLSClientConfig: &tls.Config{
+			InsecureSkipVerify: true,
+			ServerName:         setting.Domain,
+		},
+	}
+})
+
 func NewInternalRequest(ctx context.Context, url, method string) *httplib.Request {
 	if setting.InternalToken == "" {
 		log.Fatal(`The INTERNAL_TOKEN setting is missing from the configuration file: %q.
@@ -43,49 +73,11 @@ Ensure you are running in the correct environment or set the correct configurati
 		log.Fatal("Invalid internal request URL: %q", url)
 	}
 
-	req := httplib.NewRequest(url, method).
+	return httplib.NewRequest(url, method).
 		SetContext(ctx).
+		SetTransport(internalAPITransport()).
 		Header("X-Real-IP", getClientIP()).
-		Header("X-Gitea-Internal-Auth", "Bearer "+setting.InternalToken).
-		SetTLSClientConfig(&tls.Config{
-			InsecureSkipVerify: true,
-			ServerName:         setting.Domain,
-		})
-
-	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
-			},
-		})
-	}
-	return req
+		Header("X-Gitea-Internal-Auth", "Bearer "+setting.InternalToken)
 }
 
 func newInternalRequestAPI(ctx context.Context, url, method string, body ...any) *httplib.Request {
@@ -98,6 +90,6 @@ func newInternalRequestAPI(ctx context.Context, url, method string, body ...any)
 		log.Fatal("Too many arguments for newInternalRequestAPI")
 	}
 
-	req.SetTimeout(10*time.Second, 60*time.Second)
+	req.SetReadWriteTimeout(60 * time.Second)
 	return req
 }

modules/private/restore_repo.go

@@ -6,7 +6,6 @@ package private
 import (
 	"context"
 	"fmt"
-	"time"
 
 	"code.gitea.io/gitea/modules/setting"
 )
@@ -31,6 +30,6 @@ func RestoreRepo(ctx context.Context, repoDir, ownerName, repoName string, units
 		Units:      units,
 		Validation: validation,
 	})
-	req.SetTimeout(3*time.Second, 0) // since the request will spend much time, don't timeout
+	req.SetReadWriteTimeout(0) // since the request will spend much time, don't timeout
 	return requestJSONClientMsg(req, fmt.Sprintf("Restore repo %s/%s successfully", ownerName, repoName))
 }

routers/web/auth/oauth2_provider.go

@@ -636,6 +636,7 @@ func handleAuthorizationCode(ctx *context.Context, form forms.AccessTokenForm, s
 			ErrorCode:        oauth2_provider.AccessTokenErrorCodeInvalidRequest,
 			ErrorDescription: "cannot proceed your request",
 		})
+		return
 	}
 	resp, tokenErr := oauth2_provider.NewAccessTokenResponse(ctx, authorizationCode.Grant, serverKey, clientKey)
 	if tokenErr != nil {

routers/web/repo/editor.go

@@ -295,14 +295,14 @@ func EditFile(ctx *context.Context) {
 		}
 		defer dataRc.Close()
 
-		ctx.Data["FileSize"] = fInfo.fileSize
+		ctx.Data["FileSize"] = fInfo.blobOrLfsSize
 
 		// Only some file types are editable online as text.
 		if fInfo.isLFSFile() {
 			ctx.Data["NotEditableReason"] = ctx.Tr("repo.editor.cannot_edit_lfs_files")
 		} else if !fInfo.st.IsRepresentableAsText() {
 			ctx.Data["NotEditableReason"] = ctx.Tr("repo.editor.cannot_edit_non_text_files")
-		} else if fInfo.fileSize >= setting.UI.MaxDisplayFileSize {
+		} else if fInfo.blobOrLfsSize >= setting.UI.MaxDisplayFileSize {
 			ctx.Data["NotEditableReason"] = ctx.Tr("repo.editor.cannot_edit_too_large_file")
 		}
 

routers/web/repo/view.go

@@ -60,9 +60,9 @@ const (
 )
 
 type fileInfo struct {
-	fileSize int64
-	lfsMeta  *lfs.Pointer
-	st       typesniffer.SniffedType
+	blobOrLfsSize int64
+	lfsMeta       *lfs.Pointer
+	st            typesniffer.SniffedType
 }
 
 func (fi *fileInfo) isLFSFile() bool {
@@ -81,7 +81,7 @@ func getFileReader(ctx gocontext.Context, repoID int64, blob *git.Blob) (buf []b
 	n, _ := util.ReadAtMost(dataRc, buf)
 	buf = buf[:n]
 
-	fi = &fileInfo{fileSize: blob.Size(), st: typesniffer.DetectContentType(buf)}
+	fi = &fileInfo{blobOrLfsSize: blob.Size(), st: typesniffer.DetectContentType(buf)}
 
 	// FIXME: what happens when README file is an image?
 	if !fi.st.IsText() || !setting.LFS.StartServer {
@@ -114,7 +114,7 @@ func getFileReader(ctx gocontext.Context, repoID int64, blob *git.Blob) (buf []b
 	}
 	buf = buf[:n]
 	fi.st = typesniffer.DetectContentType(buf)
-	fi.fileSize = blob.Size()
+	fi.blobOrLfsSize = meta.Pointer.Size
 	fi.lfsMeta = &meta.Pointer
 	return buf, dataRc, fi, nil
 }

routers/web/repo/view_file.go

@@ -226,7 +226,7 @@ func prepareFileView(ctx *context.Context, entry *git.TreeEntry) {
 	}
 
 	ctx.Data["IsLFSFile"] = fInfo.isLFSFile()
-	ctx.Data["FileSize"] = fInfo.fileSize
+	ctx.Data["FileSize"] = fInfo.blobOrLfsSize
 	ctx.Data["IsRepresentableAsText"] = fInfo.st.IsRepresentableAsText()
 	ctx.Data["IsExecutable"] = entry.IsExecutable()
 	ctx.Data["CanCopyContent"] = fInfo.st.IsRepresentableAsText() || fInfo.st.IsImage()
@@ -243,7 +243,7 @@ func prepareFileView(ctx *context.Context, entry *git.TreeEntry) {
 
 	utf8Reader := charset.ToUTF8WithFallbackReader(io.MultiReader(bytes.NewReader(buf), dataRc), charset.ConvertOpts{})
 	switch {
-	case fInfo.fileSize >= setting.UI.MaxDisplayFileSize:
+	case fInfo.blobOrLfsSize >= setting.UI.MaxDisplayFileSize:
 		ctx.Data["IsFileTooLarge"] = true
 	case handleFileViewRenderMarkup(ctx, entry.Name(), fInfo.st, buf, utf8Reader):
 		// it also sets ctx.Data["FileContent"] and more

routers/web/repo/view_readme.go

@@ -170,7 +170,7 @@ func prepareToRenderReadmeFile(ctx *context.Context, subfolder string, readmeFil
 
 	ctx.Data["FileIsText"] = fInfo.st.IsText()
 	ctx.Data["FileTreePath"] = readmeFullPath
-	ctx.Data["FileSize"] = fInfo.fileSize
+	ctx.Data["FileSize"] = fInfo.blobOrLfsSize
 	ctx.Data["IsLFSFile"] = fInfo.isLFSFile()
 
 	if fInfo.isLFSFile() {
@@ -182,7 +182,7 @@ func prepareToRenderReadmeFile(ctx *context.Context, subfolder string, readmeFil
 		return
 	}
 
-	if fInfo.fileSize >= setting.UI.MaxDisplayFileSize {
+	if fInfo.blobOrLfsSize >= setting.UI.MaxDisplayFileSize {
 		// Pretend that this is a normal text file to display 'This file is too large to be shown'
 		ctx.Data["IsFileTooLarge"] = true
 		return

services/issue/comments.go

@@ -15,6 +15,7 @@ import (
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/gitrepo"
 	"code.gitea.io/gitea/modules/json"
+	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/timeutil"
 	git_service "code.gitea.io/gitea/services/git"
 	notify_service "code.gitea.io/gitea/services/notify"
@@ -151,15 +152,15 @@ func DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_m
 }
 
 // LoadCommentPushCommits Load push commits
-func LoadCommentPushCommits(ctx context.Context, c *issues_model.Comment) (err error) {
+func LoadCommentPushCommits(ctx context.Context, c *issues_model.Comment) error {
 	if c.Content == "" || c.Commits != nil || c.Type != issues_model.CommentTypePullRequestPush {
 		return nil
 	}
 
 	var data issues_model.PushActionContent
-	err = json.Unmarshal([]byte(c.Content), &data)
-	if err != nil {
-		return err
+	if err := json.Unmarshal([]byte(c.Content), &data); err != nil {
+		log.Debug("Unmarshal: %v", err) // no need to show 500 error to end user when the JSON is broken
+		return nil
 	}
 
 	c.IsForcePush = data.IsForcePush
@@ -168,9 +169,15 @@ func LoadCommentPushCommits(ctx context.Context, c *issues_model.Comment) (err e
 		if len(data.CommitIDs) != 2 {
 			return nil
 		}
-		c.OldCommit = data.CommitIDs[0]
-		c.NewCommit = data.CommitIDs[1]
+		c.OldCommit, c.NewCommit = data.CommitIDs[0], data.CommitIDs[1]
 	} else {
+		if err := c.LoadIssue(ctx); err != nil {
+			return err
+		}
+		if err := c.Issue.LoadRepo(ctx); err != nil {
+			return err
+		}
+
 		gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, c.Issue.Repo)
 		if err != nil {
 			return err
@@ -179,10 +186,11 @@ func LoadCommentPushCommits(ctx context.Context, c *issues_model.Comment) (err e
 
 		c.Commits, err = git_service.ConvertFromGitCommit(ctx, gitRepo.GetCommitsFromIDs(data.CommitIDs), c.Issue.Repo)
 		if err != nil {
-			return err
+			log.Debug("ConvertFromGitCommit: %v", err) // no need to show 500 error to end user when the commit does not exist
+		} else {
+			c.CommitsNum = int64(len(c.Commits))
 		}
-		c.CommitsNum = int64(len(c.Commits))
 	}
 
-	return err
+	return nil
 }

services/pull/merge.go

@@ -248,6 +248,11 @@ func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.U
 	}
 	defer releaser()
 	defer func() {
+		// This is a duplicated call to AddTestPullRequestTask (it will also be called by the post-receive hook, via a push queue).
+		// This call will do some operations (push to base repo, sync commit divergence, add PR conflict check queue task, etc)
+		// immediately instead of waiting for the "push queue"'s task. The code is from https://github.com/go-gitea/gitea/pull/7082.
+		// But it's really questionable whether it's worth to do it ahead without waiting for the "push queue" task to run.
+		// TODO: DUPLICATE-PR-TASK: maybe can try to remove this in 1.26 to see if there is any issue.
 		go AddTestPullRequestTask(TestPullRequestOptions{
 			RepoID:      pr.BaseRepo.ID,
 			Doer:        doer,

services/pull/pull.go

@@ -379,10 +379,8 @@ type TestPullRequestOptions struct {
 func AddTestPullRequestTask(opts TestPullRequestOptions) {
 	log.Trace("AddTestPullRequestTask [head_repo_id: %d, head_branch: %s]: finding pull requests", opts.RepoID, opts.Branch)
 	graceful.GetManager().RunWithShutdownContext(func(ctx context.Context) {
-		// There is no sensible way to shut this down ":-("
-		// If you don't let it run all the way then you will lose data
-		// TODO: graceful: AddTestPullRequestTask needs to become a queue!
-
+		// this function does a lot of operations to various models, if the process gets killed in the middle,
+		// there is no way to recover at the moment. The best workaround is to let end user push again.
 		// GetUnmergedPullRequestsByHeadInfo() only return open and unmerged PR.
 		prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(ctx, opts.RepoID, opts.Branch)
 		if err != nil {
@@ -401,11 +399,15 @@ func AddTestPullRequestTask(opts TestPullRequestOptions) {
 				continue
 			}
 
-			StartPullRequestCheckImmediately(ctx, pr)
+			// create push comment before check pull request status,
+			// then when the status is mergeable, the comment is already in database, to make testing easy and stable
 			comment, err := CreatePushPullComment(ctx, opts.Doer, pr, opts.OldCommitID, opts.NewCommitID, opts.IsForcePush)
 			if err == nil && comment != nil {
 				notify_service.PullRequestPushCommits(ctx, opts.Doer, pr, comment)
 			}
+			// The caller can be in a goroutine or a "push queue", "conflict check" can be time-consuming,
+			// and the concurrency should be limited, so the conflict check will be done in another queue
+			StartPullRequestCheckImmediately(ctx, pr)
 		}
 
 		if opts.IsSync {

services/pull/update.go

@@ -59,6 +59,9 @@ func Update(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.
 	}
 
 	defer func() {
+		// The code is from https://github.com/go-gitea/gitea/pull/9784,
+		// it seems a simple copy-paste from https://github.com/go-gitea/gitea/pull/7082 without a real reason.
+		// TODO: DUPLICATE-PR-TASK: search and see another TODO comment for more details
 		go AddTestPullRequestTask(TestPullRequestOptions{
 			RepoID:      pr.BaseRepo.ID,
 			Doer:        doer,

templates/repo/file_info.tmpl

@@ -11,11 +11,11 @@
 	{{end}}
 	{{if ne .FileSize nil}}
 		<div class="file-info-entry">
-			{{FileSize .FileSize}}{{if .IsLFSFile}}<span class="ui label">LFS</span>{{end}}
+			<span class="file-info-size">{{FileSize .FileSize}}</span>{{if .IsLFSFile}}<span class="ui label">LFS</span>{{end}}
 		</div>
 	{{end}}
 	{{if .LFSLock}}
-		<div class="file-info-entry ui" data-tooltip-content="{{.LFSLockHint}}">
+		<div class="file-info-entry" data-tooltip-content="{{.LFSLockHint}}">
 			{{svg "octicon-lock" 16 "tw-mr-1"}}
 			<a href="{{.LFSLockOwnerHomeLink}}">{{.LFSLockOwner}}</a>
 		</div>

tests/integration/git_lfs_ssh_test.go

@@ -5,6 +5,8 @@ package integration
 
 import (
 	"net/url"
+	"os"
+	"path/filepath"
 	"slices"
 	"strings"
 	"sync"
@@ -23,7 +25,8 @@ import (
 
 func TestGitLFSSSH(t *testing.T) {
 	onGiteaRun(t, func(t *testing.T, u *url.URL) {
-		dstPath := t.TempDir()
+		localRepoForUpload := filepath.Join(t.TempDir(), "test-upload")
+		localRepoForDownload := filepath.Join(t.TempDir(), "test-download")
 		apiTestContext := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
 
 		var mu sync.Mutex
@@ -37,7 +40,7 @@ func TestGitLFSSSH(t *testing.T) {
 		withKeyFile(t, "my-testing-key", func(keyFile string) {
 			t.Run("CreateUserKey", doAPICreateUserKey(apiTestContext, "test-key", keyFile))
 			cloneURL := createSSHUrl(apiTestContext.GitPath(), u)
-			t.Run("Clone", doGitClone(dstPath, cloneURL))
+			t.Run("CloneOrigin", doGitClone(localRepoForUpload, cloneURL))
 
 			cfg, err := setting.CfgProvider.PrepareSaving()
 			require.NoError(t, err)
@@ -45,9 +48,14 @@ func TestGitLFSSSH(t *testing.T) {
 			setting.LFS.AllowPureSSH = true
 			require.NoError(t, cfg.Save())
 
-			_, _, cmdErr := gitcmd.NewCommand("config", "lfs.sshtransfer", "always").RunStdString(t.Context(), &gitcmd.RunOpts{Dir: dstPath})
+			_, _, cmdErr := gitcmd.NewCommand("config", "lfs.sshtransfer", "always").RunStdString(t.Context(), &gitcmd.RunOpts{Dir: localRepoForUpload})
 			assert.NoError(t, cmdErr)
-			lfsCommitAndPushTest(t, dstPath, 10)
+			pushedFiles := lfsCommitAndPushTest(t, localRepoForUpload, 10)
+
+			t.Run("CloneLFS", doGitClone(localRepoForDownload, cloneURL))
+			content, err := os.ReadFile(filepath.Join(localRepoForDownload, pushedFiles[0]))
+			assert.NoError(t, err)
+			assert.Len(t, content, 10)
 		})
 
 		countBatch := slices.ContainsFunc(routerCalls, func(s string) bool {
@@ -56,12 +64,16 @@ func TestGitLFSSSH(t *testing.T) {
 		countUpload := slices.ContainsFunc(routerCalls, func(s string) bool {
 			return strings.Contains(s, "PUT /api/internal/repo/user2/repo1.git/info/lfs/objects/")
 		})
+		countDownload := slices.ContainsFunc(routerCalls, func(s string) bool {
+			return strings.Contains(s, "GET /api/internal/repo/user2/repo1.git/info/lfs/objects/")
+		})
 		nonAPIRequests := slices.ContainsFunc(routerCalls, func(s string) bool {
 			fields := strings.Fields(s)
 			return !strings.HasPrefix(fields[1], "/api/")
 		})
 		assert.NotZero(t, countBatch)
 		assert.NotZero(t, countUpload)
+		assert.NotZero(t, countDownload)
 		assert.Zero(t, nonAPIRequests)
 	})
 }

tests/integration/lfs_view_test.go

@@ -72,6 +72,8 @@ func TestLFSRender(t *testing.T) {
 
 		fileInfo := doc.Find("div.file-info-entry").First().Text()
 		assert.Contains(t, fileInfo, "LFS")
+		fileSize := doc.Find("div.file-info-entry > .file-info-size").Text()
+		assert.Equal(t, "2.0 KiB", fileSize)
 
 		// find new file view container
 		fileViewContainer := doc.Find("[data-global-init=initRepoFileView]")

tests/integration/repo_test.go

@@ -257,10 +257,12 @@ func testViewFileInRepo(t *testing.T) {
 	description := htmlDoc.doc.Find(".repo-description")
 	repoTopics := htmlDoc.doc.Find("#repo-topics")
 	repoSummary := htmlDoc.doc.Find(".repository-summary")
+	fileSize := htmlDoc.Find("div.file-info-entry > .file-info-size").Text()
 
 	assert.Equal(t, 0, description.Length())
 	assert.Equal(t, 0, repoTopics.Length())
 	assert.Equal(t, 0, repoSummary.Length())
+	assert.Equal(t, "30 B", fileSize)
 }
 
 // TestBlameFileInRepo repo description, topics and summary should not be displayed when running blame on a file

web_src/css/repo.css

@@ -1582,6 +1582,7 @@ tbody.commit-list {
   display: flex;
   align-items: center;
   width: max-content;
+  gap: 0.25em;
 }
 
 .file-info-entry + .file-info-entry {

web_src/js/components/RepoActionView.vue

@@ -601,7 +601,8 @@ export default defineComponent({
             </div>
           </div>
         </div>
-        <div class="job-step-container" ref="stepsContainer" v-if="currentJob.steps.length">
+        <!-- always create the node because we have our own event listeners on it, don't use "v-if" -->
+        <div class="job-step-container" ref="stepsContainer" v-show="currentJob.steps.length">
           <div class="job-step-section" v-for="(jobStep, i) in currentJob.steps" :key="i">
             <div class="job-step-summary" @click.stop="isExpandable(jobStep.status) && toggleStepLogs(i)" :class="[currentJobStepsStates[i].expanded ? 'selected' : '', isExpandable(jobStep.status) && 'step-expandable']">
               <!-- If the job is done and the job step log is loaded for the first time, show the loading icon