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 <art27@cantab.net>

Co-authored-by: zeripath <art27@cantab.net>

1 files changed, 104 insertions, 13 deletions

diff --git a/integrations/lfs_getobject_test.go b/integrations/lfs_getobject_test.go
index 15d9c52a75..45e94406a5 100644
--- a/integrations/lfs_getobject_test.go
+++ b/integrations/lfs_getobject_test.go
@@ -12,6 +12,7 @@ import (
 	"io"
 	"io/ioutil"
 	"net/http"
+	"net/http/httptest"
 	"testing"
 
 	"code.gitea.io/gitea/models"
@@ -56,13 +57,7 @@ func storeObjectInRepo(t *testing.T, repositoryID int64, content *[]byte) string
 	return oid
 }
 
-func doLfs(t *testing.T, content *[]byte, expectGzip bool) {
-	defer prepareTestEnv(t)()
-	setting.CheckLFSVersion()
-	if !setting.LFS.StartServer {
-		t.Skip()
-		return
-	}
+func storeAndGetLfs(t *testing.T, content *[]byte, extraHeader *http.Header, expectedStatus int) *httptest.ResponseRecorder {
 	repo, err := models.GetRepositoryByOwnerAndName("user2", "repo1")
 	assert.NoError(t, err)
 	oid := storeObjectInRepo(t, repo.ID, content)
@@ -73,8 +68,19 @@ func doLfs(t *testing.T, content *[]byte, expectGzip bool) {
 	// Request OID
 	req := NewRequest(t, "GET", "/user2/repo1.git/info/lfs/objects/"+oid+"/test")
 	req.Header.Set("Accept-Encoding", "gzip")
-	resp := session.MakeRequest(t, req, http.StatusOK)
+	if extraHeader != nil {
+		for key, values := range *extraHeader {
+			for _, value := range values {
+				req.Header.Add(key, value)
+			}
+		}
+	}
+	resp := session.MakeRequest(t, req, expectedStatus)
 
+	return resp
+}
+
+func checkResponseTestContentEncoding(t *testing.T, content *[]byte, resp *httptest.ResponseRecorder, expectGzip bool) {
 	contentEncoding := resp.Header().Get("Content-Encoding")
 	if !expectGzip || !setting.EnableGzip {
 		assert.NotContains(t, contentEncoding, "gzip")
@@ -89,23 +95,44 @@ func doLfs(t *testing.T, content *[]byte, expectGzip bool) {
 		assert.NoError(t, err)
 		assert.Equal(t, *content, result)
 	}
-
 }
 
 func TestGetLFSSmall(t *testing.T) {
+	defer prepareTestEnv(t)()
+	setting.CheckLFSVersion()
+	if !setting.LFS.StartServer {
+		t.Skip()
+		return
+	}
 	content := []byte("A very small file\n")
-	doLfs(t, &content, false)
+
+	resp := storeAndGetLfs(t, &content, nil, http.StatusOK)
+	checkResponseTestContentEncoding(t, &content, resp, false)
 }
 
 func TestGetLFSLarge(t *testing.T) {
+	defer prepareTestEnv(t)()
+	setting.CheckLFSVersion()
+	if !setting.LFS.StartServer {
+		t.Skip()
+		return
+	}
 	content := make([]byte, gzip.MinSize*10)
 	for i := range content {
 		content[i] = byte(i % 256)
 	}
-	doLfs(t, &content, true)
+
+	resp := storeAndGetLfs(t, &content, nil, http.StatusOK)
+	checkResponseTestContentEncoding(t, &content, resp, true)
 }
 
 func TestGetLFSGzip(t *testing.T) {
+	defer prepareTestEnv(t)()
+	setting.CheckLFSVersion()
+	if !setting.LFS.StartServer {
+		t.Skip()
+		return
+	}
 	b := make([]byte, gzip.MinSize*10)
 	for i := range b {
 		b[i] = byte(i % 256)
@@ -115,10 +142,18 @@ func TestGetLFSGzip(t *testing.T) {
 	gzippWriter.Write(b)
 	gzippWriter.Close()
 	content := outputBuffer.Bytes()
-	doLfs(t, &content, false)
+
+	resp := storeAndGetLfs(t, &content, nil, http.StatusOK)
+	checkResponseTestContentEncoding(t, &content, resp, false)
 }
 
 func TestGetLFSZip(t *testing.T) {
+	defer prepareTestEnv(t)()
+	setting.CheckLFSVersion()
+	if !setting.LFS.StartServer {
+		t.Skip()
+		return
+	}
 	b := make([]byte, gzip.MinSize*10)
 	for i := range b {
 		b[i] = byte(i % 256)
@@ -130,5 +165,61 @@ func TestGetLFSZip(t *testing.T) {
 	fileWriter.Write(b)
 	zipWriter.Close()
 	content := outputBuffer.Bytes()
-	doLfs(t, &content, false)
+
+	resp := storeAndGetLfs(t, &content, nil, http.StatusOK)
+	checkResponseTestContentEncoding(t, &content, resp, false)
+}
+
+func TestGetLFSRangeNo(t *testing.T) {
+	defer prepareTestEnv(t)()
+	setting.CheckLFSVersion()
+	if !setting.LFS.StartServer {
+		t.Skip()
+		return
+	}
+	content := []byte("123456789\n")
+
+	resp := storeAndGetLfs(t, &content, nil, http.StatusOK)
+	assert.Equal(t, content, resp.Body.Bytes())
+}
+
+func TestGetLFSRange(t *testing.T) {
+	defer prepareTestEnv(t)()
+	setting.CheckLFSVersion()
+	if !setting.LFS.StartServer {
+		t.Skip()
+		return
+	}
+	content := []byte("123456789\n")
+
+	tests := []struct {
+		in     string
+		out    string
+		status int
+	}{
+		{"bytes=0-0", "1", http.StatusPartialContent},
+		{"bytes=0-1", "12", http.StatusPartialContent},
+		{"bytes=1-1", "2", http.StatusPartialContent},
+		{"bytes=1-3", "234", http.StatusPartialContent},
+		{"bytes=1-", "23456789\n", http.StatusPartialContent},
+		// end-range smaller than start-range is ignored
+		{"bytes=1-0", "23456789\n", http.StatusPartialContent},
+		{"bytes=0-10", "123456789\n", http.StatusPartialContent},
+		// end-range bigger than length-1 is ignored
+		{"bytes=0-11", "123456789\n", http.StatusPartialContent},
+		{"bytes=11-", "", http.StatusPartialContent},
+		// incorrect header value cause whole header to be ignored
+		{"bytes=-", "123456789\n", http.StatusOK},
+		{"foobar", "123456789\n", http.StatusOK},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.in, func(t *testing.T) {
+			h := http.Header{
+				"Range": []string{tt.in},
+			}
+			resp := storeAndGetLfs(t, &content, &h, tt.status)
+			assert.Equal(t, tt.out, resp.Body.String())
+		})
+	}
 }