Skip to content

Commit d8e6acd

Browse files
burbonzeripath
andauthored
Support Range header end in lfs (#11314)
* Initial support of end Range header option in lfs * Allow end range option to be unspecified * Declare toByte for consistency * Factor out content encoding tests from doLfs This is so Range tests could still use doLfs but without repeating not related tests * Add Range header test * implemented extraHeader * parametrized expectedStatus * Add more test cases of Range header * Fix S1030: should use resp.Body.String() * Add more tests for edge cases * Fix tests Signed-off-by: Andrew Thornton <[email protected]> Co-authored-by: zeripath <[email protected]>
1 parent 742e26f commit d8e6acd

File tree

2 files changed

+119
-18
lines changed

2 files changed

+119
-18
lines changed

integrations/lfs_getobject_test.go

Lines changed: 104 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"io"
1313
"io/ioutil"
1414
"net/http"
15+
"net/http/httptest"
1516
"testing"
1617

1718
"code.gitea.io/gitea/models"
@@ -56,13 +57,7 @@ func storeObjectInRepo(t *testing.T, repositoryID int64, content *[]byte) string
5657
return oid
5758
}
5859

59-
func doLfs(t *testing.T, content *[]byte, expectGzip bool) {
60-
defer prepareTestEnv(t)()
61-
setting.CheckLFSVersion()
62-
if !setting.LFS.StartServer {
63-
t.Skip()
64-
return
65-
}
60+
func storeAndGetLfs(t *testing.T, content *[]byte, extraHeader *http.Header, expectedStatus int) *httptest.ResponseRecorder {
6661
repo, err := models.GetRepositoryByOwnerAndName("user2", "repo1")
6762
assert.NoError(t, err)
6863
oid := storeObjectInRepo(t, repo.ID, content)
@@ -73,8 +68,19 @@ func doLfs(t *testing.T, content *[]byte, expectGzip bool) {
7368
// Request OID
7469
req := NewRequest(t, "GET", "/user2/repo1.git/info/lfs/objects/"+oid+"/test")
7570
req.Header.Set("Accept-Encoding", "gzip")
76-
resp := session.MakeRequest(t, req, http.StatusOK)
71+
if extraHeader != nil {
72+
for key, values := range *extraHeader {
73+
for _, value := range values {
74+
req.Header.Add(key, value)
75+
}
76+
}
77+
}
78+
resp := session.MakeRequest(t, req, expectedStatus)
7779

80+
return resp
81+
}
82+
83+
func checkResponseTestContentEncoding(t *testing.T, content *[]byte, resp *httptest.ResponseRecorder, expectGzip bool) {
7884
contentEncoding := resp.Header().Get("Content-Encoding")
7985
if !expectGzip || !setting.EnableGzip {
8086
assert.NotContains(t, contentEncoding, "gzip")
@@ -89,23 +95,44 @@ func doLfs(t *testing.T, content *[]byte, expectGzip bool) {
8995
assert.NoError(t, err)
9096
assert.Equal(t, *content, result)
9197
}
92-
9398
}
9499

95100
func TestGetLFSSmall(t *testing.T) {
101+
defer prepareTestEnv(t)()
102+
setting.CheckLFSVersion()
103+
if !setting.LFS.StartServer {
104+
t.Skip()
105+
return
106+
}
96107
content := []byte("A very small file\n")
97-
doLfs(t, &content, false)
108+
109+
resp := storeAndGetLfs(t, &content, nil, http.StatusOK)
110+
checkResponseTestContentEncoding(t, &content, resp, false)
98111
}
99112

100113
func TestGetLFSLarge(t *testing.T) {
114+
defer prepareTestEnv(t)()
115+
setting.CheckLFSVersion()
116+
if !setting.LFS.StartServer {
117+
t.Skip()
118+
return
119+
}
101120
content := make([]byte, gzip.MinSize*10)
102121
for i := range content {
103122
content[i] = byte(i % 256)
104123
}
105-
doLfs(t, &content, true)
124+
125+
resp := storeAndGetLfs(t, &content, nil, http.StatusOK)
126+
checkResponseTestContentEncoding(t, &content, resp, true)
106127
}
107128

108129
func TestGetLFSGzip(t *testing.T) {
130+
defer prepareTestEnv(t)()
131+
setting.CheckLFSVersion()
132+
if !setting.LFS.StartServer {
133+
t.Skip()
134+
return
135+
}
109136
b := make([]byte, gzip.MinSize*10)
110137
for i := range b {
111138
b[i] = byte(i % 256)
@@ -115,10 +142,18 @@ func TestGetLFSGzip(t *testing.T) {
115142
gzippWriter.Write(b)
116143
gzippWriter.Close()
117144
content := outputBuffer.Bytes()
118-
doLfs(t, &content, false)
145+
146+
resp := storeAndGetLfs(t, &content, nil, http.StatusOK)
147+
checkResponseTestContentEncoding(t, &content, resp, false)
119148
}
120149

121150
func TestGetLFSZip(t *testing.T) {
151+
defer prepareTestEnv(t)()
152+
setting.CheckLFSVersion()
153+
if !setting.LFS.StartServer {
154+
t.Skip()
155+
return
156+
}
122157
b := make([]byte, gzip.MinSize*10)
123158
for i := range b {
124159
b[i] = byte(i % 256)
@@ -130,5 +165,61 @@ func TestGetLFSZip(t *testing.T) {
130165
fileWriter.Write(b)
131166
zipWriter.Close()
132167
content := outputBuffer.Bytes()
133-
doLfs(t, &content, false)
168+
169+
resp := storeAndGetLfs(t, &content, nil, http.StatusOK)
170+
checkResponseTestContentEncoding(t, &content, resp, false)
171+
}
172+
173+
func TestGetLFSRangeNo(t *testing.T) {
174+
defer prepareTestEnv(t)()
175+
setting.CheckLFSVersion()
176+
if !setting.LFS.StartServer {
177+
t.Skip()
178+
return
179+
}
180+
content := []byte("123456789\n")
181+
182+
resp := storeAndGetLfs(t, &content, nil, http.StatusOK)
183+
assert.Equal(t, content, resp.Body.Bytes())
184+
}
185+
186+
func TestGetLFSRange(t *testing.T) {
187+
defer prepareTestEnv(t)()
188+
setting.CheckLFSVersion()
189+
if !setting.LFS.StartServer {
190+
t.Skip()
191+
return
192+
}
193+
content := []byte("123456789\n")
194+
195+
tests := []struct {
196+
in string
197+
out string
198+
status int
199+
}{
200+
{"bytes=0-0", "1", http.StatusPartialContent},
201+
{"bytes=0-1", "12", http.StatusPartialContent},
202+
{"bytes=1-1", "2", http.StatusPartialContent},
203+
{"bytes=1-3", "234", http.StatusPartialContent},
204+
{"bytes=1-", "23456789\n", http.StatusPartialContent},
205+
// end-range smaller than start-range is ignored
206+
{"bytes=1-0", "23456789\n", http.StatusPartialContent},
207+
{"bytes=0-10", "123456789\n", http.StatusPartialContent},
208+
// end-range bigger than length-1 is ignored
209+
{"bytes=0-11", "123456789\n", http.StatusPartialContent},
210+
{"bytes=11-", "", http.StatusPartialContent},
211+
// incorrect header value cause whole header to be ignored
212+
{"bytes=-", "123456789\n", http.StatusOK},
213+
{"foobar", "123456789\n", http.StatusOK},
214+
}
215+
216+
for _, tt := range tests {
217+
t.Run(tt.in, func(t *testing.T) {
218+
h := http.Header{
219+
"Range": []string{tt.in},
220+
}
221+
resp := storeAndGetLfs(t, &content, &h, tt.status)
222+
assert.Equal(t, tt.out, resp.Body.String())
223+
})
224+
}
134225
}

modules/lfs/server.go

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -165,15 +165,24 @@ func getContentHandler(ctx *context.Context) {
165165
}
166166

167167
// Support resume download using Range header
168-
var fromByte int64
168+
var fromByte, toByte int64
169+
toByte = meta.Size - 1
169170
statusCode := 200
170171
if rangeHdr := ctx.Req.Header.Get("Range"); rangeHdr != "" {
171-
regex := regexp.MustCompile(`bytes=(\d+)\-.*`)
172+
regex := regexp.MustCompile(`bytes=(\d+)\-(\d*).*`)
172173
match := regex.FindStringSubmatch(rangeHdr)
173174
if len(match) > 1 {
174175
statusCode = 206
175176
fromByte, _ = strconv.ParseInt(match[1], 10, 32)
176-
ctx.Resp.Header().Set("Content-Range", fmt.Sprintf("bytes %d-%d/%d", fromByte, meta.Size-1, meta.Size-fromByte))
177+
178+
if match[2] != "" {
179+
_toByte, _ := strconv.ParseInt(match[2], 10, 32)
180+
if _toByte >= fromByte && _toByte < toByte {
181+
toByte = _toByte
182+
}
183+
}
184+
185+
ctx.Resp.Header().Set("Content-Range", fmt.Sprintf("bytes %d-%d/%d", fromByte, toByte, meta.Size-fromByte))
177186
}
178187
}
179188

@@ -186,7 +195,8 @@ func getContentHandler(ctx *context.Context) {
186195
}
187196
defer content.Close()
188197

189-
ctx.Resp.Header().Set("Content-Length", strconv.FormatInt(meta.Size-fromByte, 10))
198+
contentLength := toByte + 1 - fromByte
199+
ctx.Resp.Header().Set("Content-Length", strconv.FormatInt(contentLength, 10))
190200
ctx.Resp.Header().Set("Content-Type", "application/octet-stream")
191201

192202
filename := ctx.Params("filename")
@@ -198,7 +208,7 @@ func getContentHandler(ctx *context.Context) {
198208
}
199209

200210
ctx.Resp.WriteHeader(statusCode)
201-
if written, err := io.Copy(ctx.Resp, content); err != nil {
211+
if written, err := io.CopyN(ctx.Resp, content, contentLength); err != nil {
202212
log.Error("Error whilst copying LFS OID[%s] to the response after %d bytes. Error: %v", meta.Oid, written, err)
203213
}
204214
logRequest(ctx.Req, statusCode)

0 commit comments

Comments
 (0)