diff --git a/test/cmd/lfstest-gitserver.go b/test/cmd/lfstest-gitserver.go index 61a738dc..0736e527 100644 --- a/test/cmd/lfstest-gitserver.go +++ b/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 { diff --git a/test/test-resume-http-range.sh b/test/test-resume-http-range.sh index de2a93b5..8864ef38 100755 --- a/test/test-resume-http-range.sh +++ b/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 + diff --git a/transfer/http_range.go b/transfer/http_range.go index 77dc31f7..0e5076fe 100644 --- a/transfer/http_range.go +++ b/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 }