lunny/git-lfs
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

t: add tests with missing Retry-After header

Browse Source 
A prior commit in this PR resolves a bug where a 429 response to an
upload or download request causes a Go panic in the client if the
response lacks a Retry-After header.

We can simulate this condition and confirm that the changes fix the
problem by updating our test server to respond with a 429 but no
Retry-After header when passed a new sentinel value, and then adding
a pair of tests which send this value.  Both tests generate the
Go panic condition without the other changes in this PR.
This commit is contained in:
Chris Darroch 2024-06-18 13:14:23 -07:00
parent 6d578a0e46
commit 43c0a1fae8
2 changed files with 82 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
t/cmd/lfstest-gitserver.go
View File

@ -57,7 +57,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-storage-503",
"status-batch-resume-206", "batch-resume-fail-fallback", "return-expired-action", "return-expired-action-forever", "return-invalid-size",
"object-authenticated", "storage-download-retry", "storage-upload-retry", "storage-upload-retry-later", "unknown-oid",
"object-authenticated", "storage-download-retry", "storage-upload-retry", "storage-upload-retry-later", "storage-upload-retry-later-no-header", "unknown-oid",
"send-verify-action", "send-deprecated-links", "redirect-storage-upload", "storage-compress", "batch-hash-algo-empty", "batch-hash-algo-invalid",
"auth-bearer",
}
@ -709,6 +709,14 @@ func storageHandler(w http.ResponseWriter, r *http.Request) {
fmt.Println("Setting header to: ", strconv.Itoa(timeLeft))
return
}
case "storage-upload-retry-later-no-header":
if _, isWaiting := checkRateLimit("storage", "upload", repo, oid); isWaiting {
w.WriteHeader(http.StatusTooManyRequests)
w.Write([]byte("rate limit reached"))
fmt.Println("Not setting Retry-After header")
return
}
case "storage-compress":
if r.Header.Get("Accept-Encoding") != "gzip" {
w.WriteHeader(500)
@ -758,6 +766,12 @@ func storageHandler(w http.ResponseWriter, r *http.Request) {
by = []byte("rate limit reached")
fmt.Println("Setting header to: ", strconv.Itoa(secsToWait))
}
} else if len(by) == len("storage-download-retry-later-no-header") && string(by) == "storage-download-retry-later-no-header" {
if _, wait := checkRateLimit("storage", "download", repo, oid); wait {
statusCode = http.StatusTooManyRequests
by = []byte("rate limit reached")
fmt.Println("Not setting Retry-After header")
}
} else if len(by) == len("storage-download-retry") && string(by) == "storage-download-retry" {
if retries, ok := incrementRetriesFor("storage", "download", repo, oid, false); ok && retries < 3 {
statusCode = 500

67
t/t-batch-storage-retries-ratelimit.sh
View File

@ -101,3 +101,70 @@ begin_test "batch clone causes retries"
popd
)
end_test
begin_test "batch storage upload causes retries (missing header)"
(
set -e
reponame="batch-storage-upload-retry-later-no-header"
setup_remote_repo "$reponame"
clone_repo "$reponame" batch-storage-repo-upload-no-header
contents="storage-upload-retry-later-no-header"
oid="$(calc_oid "$contents")"
printf "%s" "$contents" > a.dat
git lfs track "*.dat"
git add .gitattributes a.dat
git commit -m "initial commit"
GIT_TRACE=1 git push origin main 2>&1 | tee push.log
if [ "0" -ne "${PIPESTATUS[0]}" ]; then
echo >&2 "fatal: expected \`git push origin main\` to succeed ..."
exit 1
fi
assert_server_object "$reponame" "$oid"
)
end_test
begin_test "batch storage download causes retries (missing header)"
(
set -e
reponame="batch-storage-download-retry-later-no-header"
setup_remote_repo "$reponame"
clone_repo "$reponame" batch-storage-repo-download-no-header
contents="storage-download-retry-later-no-header"
oid="$(calc_oid "$contents")"
printf "%s" "$contents" > a.dat
git lfs track "*.dat"
git add .gitattributes a.dat
git commit -m "initial commit"
git push origin main
assert_server_object "$reponame" "$oid"
pushd ..
git \
-c "filter.lfs.process=" \
-c "filter.lfs.smudge=cat" \
-c "filter.lfs.required=false" \
clone "$GITSERVER/$reponame" "$reponame-assert"
cd "$reponame-assert"
git config credential.helper lfstest
GIT_TRACE=1 git lfs pull origin main 2>&1 | tee pull.log
if [ "0" -ne "${PIPESTATUS[0]}" ]; then
echo >&2 "fatal: expected \`git lfs pull origin main\` to succeed ..."
exit 1
fi
assert_local_object "$oid" "${#contents}"
popd
)
end_test