Skip to content

Commit 0e8738b

Browse files
wxiaoguangwxiaoguang
authored
Fix SSH LFS memory usage (#33455)
Fix #33448
1 parent 4f3cc26 commit 0e8738b

File tree

13 files changed

+77
-103
lines changed

13 files changed

+77
-103
lines changed

‎modules/httplib/request.go

+10-3
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,10 @@ func (r *Request) Param(key, value string) *Request {
9999
return r
100100
}
101101

102-
// Body adds request raw body.
103-
// it supports string and []byte.
102+
// Body adds request raw body. It supports string, []byte and io.Reader as body.
104103
func (r *Request) Body(data any) *Request {
105104
switch t := data.(type) {
105+
case nil: // do nothing
106106
case string:
107107
bf := bytes.NewBufferString(t)
108108
r.req.Body = io.NopCloser(bf)
@@ -111,6 +111,12 @@ func (r *Request) Body(data any) *Request {
111111
bf := bytes.NewBuffer(t)
112112
r.req.Body = io.NopCloser(bf)
113113
r.req.ContentLength = int64(len(t))
114+
case io.ReadCloser:
115+
r.req.Body = t
116+
case io.Reader:
117+
r.req.Body = io.NopCloser(t)
118+
default:
119+
panic(fmt.Sprintf("unsupported request body type %T", t))
114120
}
115121
return r
116122
}
@@ -141,7 +147,7 @@ func (r *Request) getResponse() (*http.Response, error) {
141147
}
142148
} else if r.req.Method == "POST" && r.req.Body == nil && len(paramBody) > 0 {
143149
r.Header("Content-Type", "application/x-www-form-urlencoded")
144-
r.Body(paramBody)
150+
r.Body(paramBody) // string
145151
}
146152

147153
var err error
@@ -185,6 +191,7 @@ func (r *Request) getResponse() (*http.Response, error) {
185191
}
186192

187193
// Response executes request client gets response manually.
194+
// Caller MUST close the response body if no error occurs
188195
func (r *Request) Response() (*http.Response, error) {
189196
return r.getResponse()
190197
}

‎modules/lfstransfer/backend/backend.go

+19-21
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
package backend
55

66
import (
7-
"bytes"
87
"context"
98
"encoding/base64"
109
"fmt"
@@ -29,7 +28,7 @@ var Capabilities = []string{
2928
"locking",
3029
}
3130

32-
var _ transfer.Backend = &GiteaBackend{}
31+
var _ transfer.Backend = (*GiteaBackend)(nil)
3332

3433
// GiteaBackend is an adapter between git-lfs-transfer library and Gitea's internal LFS API
3534
type GiteaBackend struct {
@@ -78,17 +77,17 @@ func (g *GiteaBackend) Batch(_ string, pointers []transfer.BatchItem, args trans
7877
headerAccept: mimeGitLFS,
7978
headerContentType: mimeGitLFS,
8079
}
81-
req := newInternalRequest(g.ctx, url, http.MethodPost, headers, bodyBytes)
80+
req := newInternalRequestLFS(g.ctx, url, http.MethodPost, headers, bodyBytes)
8281
resp, err := req.Response()
8382
if err != nil {
8483
g.logger.Log("http request error", err)
8584
return nil, err
8685
}
86+
defer resp.Body.Close()
8787
if resp.StatusCode != http.StatusOK {
8888
g.logger.Log("http statuscode error", resp.StatusCode, statusCodeToErr(resp.StatusCode))
8989
return nil, statusCodeToErr(resp.StatusCode)
9090
}
91-
defer resp.Body.Close()
9291
respBytes, err := io.ReadAll(resp.Body)
9392
if err != nil {
9493
g.logger.Log("http read error", err)
@@ -158,8 +157,7 @@ func (g *GiteaBackend) Batch(_ string, pointers []transfer.BatchItem, args trans
158157
return pointers, nil
159158
}
160159

161-
// Download implements transfer.Backend. The returned reader must be closed by the
162-
// caller.
160+
// Download implements transfer.Backend. The returned reader must be closed by the caller.
163161
func (g *GiteaBackend) Download(oid string, args transfer.Args) (io.ReadCloser, int64, error) {
164162
idMapStr, exists := args[argID]
165163
if !exists {
@@ -187,25 +185,25 @@ func (g *GiteaBackend) Download(oid string, args transfer.Args) (io.ReadCloser,
187185
headerGiteaInternalAuth: g.internalAuth,
188186
headerAccept: mimeOctetStream,
189187
}
190-
req := newInternalRequest(g.ctx, url, http.MethodGet, headers, nil)
188+
req := newInternalRequestLFS(g.ctx, url, http.MethodGet, headers, nil)
191189
resp, err := req.Response()
192190
if err != nil {
193-
return nil, 0, err
191+
return nil, 0, fmt.Errorf("failed to get response: %w", err)
194192
}
193+
// no need to close the body here by "defer resp.Body.Close()", see below
195194
if resp.StatusCode != http.StatusOK {
196195
return nil, 0, statusCodeToErr(resp.StatusCode)
197196
}
198-
defer resp.Body.Close()
199-
respBytes, err := io.ReadAll(resp.Body)
197+
198+
respSize, err := strconv.ParseInt(resp.Header.Get("X-Gitea-LFS-Content-Length"), 10, 64)
200199
if err != nil {
201-
return nil, 0, err
200+
return nil, 0, fmt.Errorf("failed to parse content length: %w", err)
202201
}
203-
respSize := int64(len(respBytes))
204-
respBuf := io.NopCloser(bytes.NewBuffer(respBytes))
205-
return respBuf, respSize, nil
202+
// transfer.Backend will check io.Closer interface and close this Body reader
203+
return resp.Body, respSize, nil
206204
}
207205

208-
// StartUpload implements transfer.Backend.
206+
// Upload implements transfer.Backend.
209207
func (g *GiteaBackend) Upload(oid string, size int64, r io.Reader, args transfer.Args) error {
210208
idMapStr, exists := args[argID]
211209
if !exists {
@@ -234,15 +232,14 @@ func (g *GiteaBackend) Upload(oid string, size int64, r io.Reader, args transfer
234232
headerContentType: mimeOctetStream,
235233
headerContentLength: strconv.FormatInt(size, 10),
236234
}
237-
reqBytes, err := io.ReadAll(r)
238-
if err != nil {
239-
return err
240-
}
241-
req := newInternalRequest(g.ctx, url, http.MethodPut, headers, reqBytes)
235+
236+
req := newInternalRequestLFS(g.ctx, url, http.MethodPut, headers, nil)
237+
req.Body(r)
242238
resp, err := req.Response()
243239
if err != nil {
244240
return err
245241
}
242+
defer resp.Body.Close()
246243
if resp.StatusCode != http.StatusOK {
247244
return statusCodeToErr(resp.StatusCode)
248245
}
@@ -284,11 +281,12 @@ func (g *GiteaBackend) Verify(oid string, size int64, args transfer.Args) (trans
284281
headerAccept: mimeGitLFS,
285282
headerContentType: mimeGitLFS,
286283
}
287-
req := newInternalRequest(g.ctx, url, http.MethodPost, headers, bodyBytes)
284+
req := newInternalRequestLFS(g.ctx, url, http.MethodPost, headers, bodyBytes)
288285
resp, err := req.Response()
289286
if err != nil {
290287
return transfer.NewStatus(transfer.StatusInternalServerError), err
291288
}
289+
defer resp.Body.Close()
292290
if resp.StatusCode != http.StatusOK {
293291
return transfer.NewStatus(uint32(resp.StatusCode), http.StatusText(resp.StatusCode)), statusCodeToErr(resp.StatusCode)
294292
}

‎modules/lfstransfer/backend/lock.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func (g *giteaLockBackend) Create(path, refname string) (transfer.Lock, error) {
5050
headerAccept: mimeGitLFS,
5151
headerContentType: mimeGitLFS,
5252
}
53-
req := newInternalRequest(g.ctx, url, http.MethodPost, headers, bodyBytes)
53+
req := newInternalRequestLFS(g.ctx, url, http.MethodPost, headers, bodyBytes)
5454
resp, err := req.Response()
5555
if err != nil {
5656
g.logger.Log("http request error", err)
@@ -102,7 +102,7 @@ func (g *giteaLockBackend) Unlock(lock transfer.Lock) error {
102102
headerAccept: mimeGitLFS,
103103
headerContentType: mimeGitLFS,
104104
}
105-
req := newInternalRequest(g.ctx, url, http.MethodPost, headers, bodyBytes)
105+
req := newInternalRequestLFS(g.ctx, url, http.MethodPost, headers, bodyBytes)
106106
resp, err := req.Response()
107107
if err != nil {
108108
g.logger.Log("http request error", err)
@@ -185,7 +185,7 @@ func (g *giteaLockBackend) queryLocks(v url.Values) ([]transfer.Lock, string, er
185185
headerAccept: mimeGitLFS,
186186
headerContentType: mimeGitLFS,
187187
}
188-
req := newInternalRequest(g.ctx, url, http.MethodGet, headers, nil)
188+
req := newInternalRequestLFS(g.ctx, url, http.MethodGet, headers, nil)
189189
resp, err := req.Response()
190190
if err != nil {
191191
g.logger.Log("http request error", err)

‎modules/lfstransfer/backend/util.go

+13-50
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,12 @@ package backend
55

66
import (
77
"context"
8-
"crypto/tls"
98
"fmt"
10-
"net"
9+
"io"
1110
"net/http"
12-
"time"
1311

1412
"code.gitea.io/gitea/modules/httplib"
15-
"code.gitea.io/gitea/modules/proxyprotocol"
16-
"code.gitea.io/gitea/modules/setting"
13+
"code.gitea.io/gitea/modules/private"
1714

1815
"github.com/charmbracelet/git-lfs-transfer/transfer"
1916
)
@@ -89,53 +86,19 @@ func statusCodeToErr(code int) error {
8986
}
9087
}
9188

92-
func newInternalRequest(ctx context.Context, url, method string, headers map[string]string, body []byte) *httplib.Request {
93-
req := httplib.NewRequest(url, method).
94-
SetContext(ctx).
95-
SetTimeout(10*time.Second, 60*time.Second).
96-
SetTLSClientConfig(&tls.Config{
97-
InsecureSkipVerify: true,
98-
})
99-
100-
if setting.Protocol == setting.HTTPUnix {
101-
req.SetTransport(&http.Transport{
102-
DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) {
103-
var d net.Dialer
104-
conn, err := d.DialContext(ctx, "unix", setting.HTTPAddr)
105-
if err != nil {
106-
return conn, err
107-
}
108-
if setting.LocalUseProxyProtocol {
109-
if err = proxyprotocol.WriteLocalHeader(conn); err != nil {
110-
_ = conn.Close()
111-
return nil, err
112-
}
113-
}
114-
return conn, err
115-
},
116-
})
117-
} else if setting.LocalUseProxyProtocol {
118-
req.SetTransport(&http.Transport{
119-
DialContext: func(ctx context.Context, network, address string) (net.Conn, error) {
120-
var d net.Dialer
121-
conn, err := d.DialContext(ctx, network, address)
122-
if err != nil {
123-
return conn, err
124-
}
125-
if err = proxyprotocol.WriteLocalHeader(conn); err != nil {
126-
_ = conn.Close()
127-
return nil, err
128-
}
129-
return conn, err
130-
},
131-
})
132-
}
133-
89+
func newInternalRequestLFS(ctx context.Context, url, method string, headers map[string]string, body any) *httplib.Request {
90+
req := private.NewInternalRequest(ctx, url, method)
13491
for k, v := range headers {
13592
req.Header(k, v)
13693
}
137-
138-
req.Body(body)
139-
94+
switch body := body.(type) {
95+
case nil: // do nothing
96+
case []byte:
97+
req.Body(body) // []byte
98+
case io.Reader:
99+
req.Body(body) // io.Reader or io.ReadCloser
100+
default:
101+
panic(fmt.Sprintf("unsupported request body type %T", body))
102+
}
140103
return req
141104
}

‎modules/private/actions.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ type GenerateTokenRequest struct {
1717
func GenerateActionsRunnerToken(ctx context.Context, scope string) (*ResponseText, ResponseExtra) {
1818
reqURL := setting.LocalURL + "api/internal/actions/generate_actions_runner_token"
1919

20-
req := newInternalRequest(ctx, reqURL, "POST", GenerateTokenRequest{
20+
req := newInternalRequestAPI(ctx, reqURL, "POST", GenerateTokenRequest{
2121
Scope: scope,
2222
})
2323

‎modules/private/hook.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ type HookProcReceiveRefResult struct {
8585
// HookPreReceive check whether the provided commits are allowed
8686
func HookPreReceive(ctx context.Context, ownerName, repoName string, opts HookOptions) ResponseExtra {
8787
reqURL := setting.LocalURL + fmt.Sprintf("api/internal/hook/pre-receive/%s/%s", url.PathEscape(ownerName), url.PathEscape(repoName))
88-
req := newInternalRequest(ctx, reqURL, "POST", opts)
88+
req := newInternalRequestAPI(ctx, reqURL, "POST", opts)
8989
req.SetReadWriteTimeout(time.Duration(60+len(opts.OldCommitIDs)) * time.Second)
9090
_, extra := requestJSONResp(req, &ResponseText{})
9191
return extra
@@ -94,7 +94,7 @@ func HookPreReceive(ctx context.Context, ownerName, repoName string, opts HookOp
9494
// HookPostReceive updates services and users
9595
func HookPostReceive(ctx context.Context, ownerName, repoName string, opts HookOptions) (*HookPostReceiveResult, ResponseExtra) {
9696
reqURL := setting.LocalURL + fmt.Sprintf("api/internal/hook/post-receive/%s/%s", url.PathEscape(ownerName), url.PathEscape(repoName))
97-
req := newInternalRequest(ctx, reqURL, "POST", opts)
97+
req := newInternalRequestAPI(ctx, reqURL, "POST", opts)
9898
req.SetReadWriteTimeout(time.Duration(60+len(opts.OldCommitIDs)) * time.Second)
9999
return requestJSONResp(req, &HookPostReceiveResult{})
100100
}
@@ -103,7 +103,7 @@ func HookPostReceive(ctx context.Context, ownerName, repoName string, opts HookO
103103
func HookProcReceive(ctx context.Context, ownerName, repoName string, opts HookOptions) (*HookProcReceiveResult, ResponseExtra) {
104104
reqURL := setting.LocalURL + fmt.Sprintf("api/internal/hook/proc-receive/%s/%s", url.PathEscape(ownerName), url.PathEscape(repoName))
105105

106-
req := newInternalRequest(ctx, reqURL, "POST", opts)
106+
req := newInternalRequestAPI(ctx, reqURL, "POST", opts)
107107
req.SetReadWriteTimeout(time.Duration(60+len(opts.OldCommitIDs)) * time.Second)
108108
return requestJSONResp(req, &HookProcReceiveResult{})
109109
}
@@ -115,15 +115,15 @@ func SetDefaultBranch(ctx context.Context, ownerName, repoName, branch string) R
115115
url.PathEscape(repoName),
116116
url.PathEscape(branch),
117117
)
118-
req := newInternalRequest(ctx, reqURL, "POST")
118+
req := newInternalRequestAPI(ctx, reqURL, "POST")
119119
_, extra := requestJSONResp(req, &ResponseText{})
120120
return extra
121121
}
122122

123123
// SSHLog sends ssh error log response
124124
func SSHLog(ctx context.Context, isErr bool, msg string) error {
125125
reqURL := setting.LocalURL + "api/internal/ssh/log"
126-
req := newInternalRequest(ctx, reqURL, "POST", &SSHLogOption{IsError: isErr, Message: msg})
126+
req := newInternalRequestAPI(ctx, reqURL, "POST", &SSHLogOption{IsError: isErr, Message: msg})
127127
_, extra := requestJSONResp(req, &ResponseText{})
128128
return extra.Error
129129
}

‎modules/private/internal.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ func getClientIP() string {
3434
return strings.Fields(sshConnEnv)[0]
3535
}
3636

37-
func newInternalRequest(ctx context.Context, url, method string, body ...any) *httplib.Request {
37+
func NewInternalRequest(ctx context.Context, url, method string) *httplib.Request {
3838
if setting.InternalToken == "" {
3939
log.Fatal(`The INTERNAL_TOKEN setting is missing from the configuration file: %q.
4040
Ensure you are running in the correct environment or set the correct configuration file with -c.`, setting.CustomConf)
@@ -82,13 +82,17 @@ Ensure you are running in the correct environment or set the correct configurati
8282
},
8383
})
8484
}
85+
return req
86+
}
8587

88+
func newInternalRequestAPI(ctx context.Context, url, method string, body ...any) *httplib.Request {
89+
req := NewInternalRequest(ctx, url, method)
8690
if len(body) == 1 {
8791
req.Header("Content-Type", "application/json")
8892
jsonBytes, _ := json.Marshal(body[0])
8993
req.Body(jsonBytes)
9094
} else if len(body) > 1 {
91-
log.Fatal("Too many arguments for newInternalRequest")
95+
log.Fatal("Too many arguments for newInternalRequestAPI")
9296
}
9397

9498
req.SetTimeout(10*time.Second, 60*time.Second)

‎modules/private/key.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import (
1414
func UpdatePublicKeyInRepo(ctx context.Context, keyID, repoID int64) error {
1515
// Ask for running deliver hook and test pull request tasks.
1616
reqURL := setting.LocalURL + fmt.Sprintf("api/internal/ssh/%d/update/%d", keyID, repoID)
17-
req := newInternalRequest(ctx, reqURL, "POST")
17+
req := newInternalRequestAPI(ctx, reqURL, "POST")
1818
_, extra := requestJSONResp(req, &ResponseText{})
1919
return extra.Error
2020
}
@@ -24,7 +24,7 @@ func UpdatePublicKeyInRepo(ctx context.Context, keyID, repoID int64) error {
2424
func AuthorizedPublicKeyByContent(ctx context.Context, content string) (*ResponseText, ResponseExtra) {
2525
// Ask for running deliver hook and test pull request tasks.
2626
reqURL := setting.LocalURL + "api/internal/ssh/authorized_keys"
27-
req := newInternalRequest(ctx, reqURL, "POST")
27+
req := newInternalRequestAPI(ctx, reqURL, "POST")
2828
req.Param("content", content)
2929
return requestJSONResp(req, &ResponseText{})
3030
}

‎modules/private/mail.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ type Email struct {
2323
func SendEmail(ctx context.Context, subject, message string, to []string) (*ResponseText, ResponseExtra) {
2424
reqURL := setting.LocalURL + "api/internal/mail/send"
2525

26-
req := newInternalRequest(ctx, reqURL, "POST", Email{
26+
req := newInternalRequestAPI(ctx, reqURL, "POST", Email{
2727
Subject: subject,
2828
Message: message,
2929
To: to,

0 commit comments

Comments
 (0)