Download resume fallback to re-download on 416 error

test/cmd/lfstest-gitserver.go

@@ -45,7 +45,7 @@ var (
 		"status-batch-403", "status-batch-404", "status-batch-410", "status-batch-422", "status-batch-500",
 		"status-storage-403", "status-storage-404", "status-storage-410", "status-storage-422", "status-storage-500",
 		"status-legacy-404", "status-legacy-410", "status-legacy-422", "status-legacy-403", "status-legacy-500",
-		"status-batch-resume-206",
+		"status-batch-resume-206", "batch-resume-fail-fallback",
 	}
 )
 
@@ -345,7 +345,7 @@ func lfsBatchHandler(w http.ResponseWriter, r *http.Request, repo string) {
 			o.Err = &lfsError{Code: 422, Message: "welp"}
 		case "status-batch-500":
 			o.Err = &lfsError{Code: 500, Message: "welp"}
-		case "status-batch-resume-206":
+		case "status-batch-resume-206", "batch-resume-fail-fallback":
 			for _, t := range objs.Transfers {
 				if t == "http-range" {
 					transferChoice = "http-range"
@@ -385,12 +385,14 @@ func lfsBatchHandler(w http.ResponseWriter, r *http.Request, repo string) {
 	w.Write(by)
 }
 
+// Persistent state across requests
+var batchResumeFailFallbackStorageAttempts = 0
+
 // handles any /storage/{oid} requests
 func storageHandler(w http.ResponseWriter, r *http.Request) {
 	repo := r.URL.Query().Get("r")
 	parts := strings.Split(r.URL.Path, "/")
 	oid := parts[len(parts)-1]
-
 	if missingRequiredCreds(w, r, repo) {
 		return
 	}
@@ -456,10 +458,24 @@ func storageHandler(w http.ResponseWriter, r *http.Request) {
 					if match != nil && len(match) > 1 {
 						statusCode = 206
 						resumeAt, _ = strconv.ParseInt(match[1], 10, 32)
+						w.Header().Set("Content-Range", fmt.Sprintf("bytes=%d-%d", resumeAt, len(by)))
 					}
 				} else {
 					byteLimit = 10
 				}
+			} else if len(by) == len("batch-resume-fail-fallback") && string(by) == "batch-resume-fail-fallback" {
+				// Fail any Range: request even though we said we supported it
+				// To make sure client can fall back
+				if rangeHdr := r.Header.Get("Range"); rangeHdr != "" {
+					w.WriteHeader(416)
+					return
+				}
+				if batchResumeFailFallbackStorageAttempts == 0 {
+					// Truncate output on FIRST attempt to cause resume
+					// Second attempt (without range header) is fallback, complete successfully
+					byteLimit = 8
+					batchResumeFailFallbackStorageAttempts++
+				}
 			}
 			w.WriteHeader(statusCode)
 			if byteLimit > 0 {

test/test-resume-http-range.sh

@@ -43,3 +43,46 @@ begin_test "resume-http-range"
 )
 end_test
 
+begin_test "resume-http-range-fallback"
+(
+  set -e
+
+  reponame="resume-http-range-fallback"
+  setup_remote_repo "$reponame"
+
+  clone_repo "$reponame" $reponame
+
+  git lfs track "*.dat" 2>&1 | tee track.log
+  grep "Tracking \*.dat" track.log
+
+  # this string announces to server that we want it to abort the download part
+  # way and tell us it supports http-range, but reject the Range: header and
+  # fall back on re-downloading instead
+  contents="batch-resume-fail-fallback"
+  contents_oid=$(calc_oid "$contents")
+
+  printf "$contents" > a.dat
+  git add a.dat
+  git add .gitattributes
+  git commit -m "add a.dat" 2>&1 | tee commit.log
+  git push origin master 
+
+  assert_server_object "$reponame" "$contents_oid"
+
+  # delete local copy then fetch it back
+  # server will abort the transfer mid way (so will error) when not resuming
+  # then we can restart it
+  rm -rf .git/lfs/objects
+  git lfs fetch 2>&1 | tee fetchinterrupted.log
+  refute_local_object "$contents_oid"
+
+  # now fetch again, this should try to resume but server should reject the Range
+  # header, which should cause client to re-download
+  GIT_TRACE=1 git lfs fetch 2>&1 | tee fetchresumefallback.log
+  grep "http-range: server rejected resume" fetchresumefallback.log
+  # re-download should still have worked
+  assert_local_object "$contents_oid" "${#contents}"
+
+)
+end_test
+

transfer/http_range.go

@@ -8,13 +8,11 @@ import (
 	"os"
 	"path/filepath"
 
-	"github.com/github/git-lfs/localstorage"
-
-	"github.com/rubyist/tracerx"
-
 	"github.com/github/git-lfs/errutil"
 	"github.com/github/git-lfs/httputil"
+	"github.com/github/git-lfs/localstorage"
 	"github.com/github/git-lfs/tools"
+	"github.com/rubyist/tracerx"
 )
 
 const (
@@ -110,6 +108,13 @@ func (a *httpRangeAdapter) download(t *Transfer, cb TransferProgressCallback, au
 
 	res, err := httputil.DoHttpRequest(req, true)
 	if err != nil {
+		// Special-case status code 416 () - fall back
+		if fromByte > 0 && dlFile != nil && res.StatusCode == 416 {
+			tracerx.Printf("http-range: server rejected resume download request for %q from byte %d; re-downloading from start", t.Object.Oid, fromByte)
+			dlFile.Close()
+			os.Remove(dlFile.Name())
+			return a.download(t, cb, authOkFunc, nil, 0, nil)
+		}
 		return errutil.NewRetriableError(err)
 	}
 	httputil.LogTransfer("lfs.data.download", res)
@@ -158,7 +163,7 @@ func (a *httpRangeAdapter) download(t *Transfer, cb TransferProgressCallback, au
 
 	if dlFile == nil {
 		// New file start
-		dlFile, err := os.OpenFile(a.downloadFilename(t), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644)
+		dlFile, err = os.OpenFile(a.downloadFilename(t), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644)
 		if err != nil {
 			return err
 		}