Skip to content

Commit 098cd07

Browse files
technoweenietechnoweenie
authored
Merge branch 'master' into windows-no-admin
2 parents d36b189 + 43cad0b commit 098cd07

File tree

12 files changed

+349
-18
lines changed

12 files changed

+349
-18
lines changed

‎docs/man/git-lfs-config.5.ronn

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,16 @@ lfs option can be scoped inside the configuration for a remote.
9999

100100
Specifies how many retries LFS will attempt per OID before marking the
101101
transfer as failed. Must be an integer which is at least one. If the value is
102-
not an integer, is less than one, or is not given, a value of one will be used
103-
instead.
102+
not an integer, is less than one, or is not given, a value of eight will be
103+
used instead.
104+
105+
* `lfs.transfer.maxverifies`
106+
107+
Specifies how many verification requests LFS will attempt per OID before
108+
marking the transfer as failed, if the object has a verification action
109+
associated with it. Must be an integer which is at least one. If the value is
110+
not an integer, is less than one, or is not given, a default value of three
111+
will be used instead.
104112

105113
### Fetch settings
106114

‎lfsapi/ssh.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"path/filepath"
99
"strings"
1010

11+
"github.com/git-lfs/git-lfs/tools"
1112
"github.com/rubyist/tracerx"
1213
)
1314

@@ -66,7 +67,7 @@ func sshGetExeAndArgs(osEnv Env, e Endpoint) (exe string, baseargs []string) {
6667

6768
ssh, _ := osEnv.Get("GIT_SSH")
6869
sshCmd, _ := osEnv.Get("GIT_SSH_COMMAND")
69-
cmdArgs := strings.Fields(sshCmd)
70+
cmdArgs := tools.QuotedFields(sshCmd)
7071
if len(cmdArgs) > 0 {
7172
ssh = cmdArgs[0]
7273
cmdArgs = cmdArgs[1:]

‎lfsapi/ssh_test.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,20 @@ func TestSSHGetExeAndArgsSshCommandArgs(t *testing.T) {
177177
assert.Equal(t, []string{"--args", "1", "[email protected]"}, args)
178178
}
179179

180+
func TestSSHGetExeAndArgsSshCommandArgsWithMixedQuotes(t *testing.T) {
181+
cli, err := NewClient(TestEnv(map[string]string{
182+
"GIT_SSH_COMMAND": "sshcmd foo 'bar \"baz\"'",
183+
}), nil)
184+
require.Nil(t, err)
185+
186+
endpoint := cli.Endpoints.Endpoint("download", "")
187+
endpoint.SshUserAndHost = "[email protected]"
188+
189+
exe, args := sshGetExeAndArgs(cli.OSEnv(), endpoint)
190+
assert.Equal(t, "sshcmd", exe)
191+
assert.Equal(t, []string{"foo", `bar "baz"`, "[email protected]"}, args)
192+
}
193+
180194
func TestSSHGetExeAndArgsSshCommandCustomPort(t *testing.T) {
181195
cli, err := NewClient(TestEnv(map[string]string{
182196
"GIT_SSH_COMMAND": "sshcmd",

‎test/cmd/lfstest-gitserver.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ var (
5959
"status-storage-403", "status-storage-404", "status-storage-410", "status-storage-422", "status-storage-500", "status-storage-503",
6060
"status-batch-resume-206", "batch-resume-fail-fallback", "return-expired-action", "return-expired-action-forever", "return-invalid-size",
6161
"object-authenticated", "storage-download-retry", "storage-upload-retry", "unknown-oid",
62+
"send-verify-action",
6263
}
6364
)
6465

@@ -98,6 +99,7 @@ func main() {
9899
})
99100

100101
mux.HandleFunc("/storage/", storageHandler)
102+
mux.HandleFunc("/verify", verifyHandler)
101103
mux.HandleFunc("/redirect307/", redirect307Handler)
102104
mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
103105
id, ok := reqId(w)
@@ -402,6 +404,15 @@ func lfsBatchHandler(w http.ResponseWriter, r *http.Request, id, repo string) {
402404
}
403405
o.Actions[action] = a
404406
}
407+
408+
if handler == "send-verify-action" {
409+
o.Actions["verify"] = lfsLink{
410+
Href: server.URL + "/verify",
411+
Header: map[string]string{
412+
"repo": repo,
413+
},
414+
}
415+
}
405416
}
406417

407418
if testingChunked && addAction {
@@ -458,6 +469,46 @@ func serveExpired(repo string) {
458469
var batchResumeFailFallbackStorageAttempts = 0
459470
var tusStorageAttempts = 0
460471

472+
var (
473+
vmu sync.Mutex
474+
verifyCounts = make(map[string]int)
475+
verifyRetryRe = regexp.MustCompile(`verify-fail-(\d+)-times?$`)
476+
)
477+
478+
func verifyHandler(w http.ResponseWriter, r *http.Request) {
479+
repo := r.Header.Get("repo")
480+
var payload struct {
481+
Oid string `json:"oid"`
482+
Size int64 `json:"size"`
483+
}
484+
485+
if err := json.NewDecoder(r.Body).Decode(&payload); err != nil {
486+
writeLFSError(w, http.StatusUnprocessableEntity, err.Error())
487+
return
488+
}
489+
490+
var max int
491+
if matches := verifyRetryRe.FindStringSubmatch(repo); len(matches) < 2 {
492+
return
493+
} else {
494+
max, _ = strconv.Atoi(matches[1])
495+
}
496+
497+
key := strings.Join([]string{repo, payload.Oid}, ":")
498+
499+
vmu.Lock()
500+
verifyCounts[key] = verifyCounts[key] + 1
501+
count := verifyCounts[key]
502+
vmu.Unlock()
503+
504+
if count < max {
505+
writeLFSError(w, http.StatusServiceUnavailable, fmt.Sprintf(
506+
"intentionally failing verify request %d (out of %d)", count, max,
507+
))
508+
return
509+
}
510+
}
511+
461512
// handles any /storage/{oid} requests
462513
func storageHandler(w http.ResponseWriter, r *http.Request) {
463514
id, ok := reqId(w)

‎test/test-verify.sh

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
#!/usr/bin/env bash
2+
3+
. "test/testlib.sh"
4+
5+
begin_test "verify with retries"
6+
(
7+
set -e
8+
9+
reponame="verify-fail-2-times"
10+
setup_remote_repo "$reponame"
11+
clone_repo "$reponame" "$reponame"
12+
13+
git lfs track "*.dat"
14+
git add .gitattributes
15+
git commit -m "initial commit"
16+
17+
contents="send-verify-action"
18+
contents_oid="$(calc_oid "$contents")"
19+
contents_short_oid="$(echo "$contents_oid" | head -c 7)"
20+
printf "$contents" > a.dat
21+
22+
git add a.dat
23+
git commit -m "add a.dat"
24+
25+
GIT_TRACE=1 git push origin master 2>&1 | tee push.log
26+
27+
[ "0" -eq "${PIPESTATUS[0]}" ]
28+
[ "2" -eq "$(grep -c "verify $contents_short_oid attempt" push.log)" ]
29+
)
30+
end_test
31+
32+
begin_test "verify with retries (success without retry)"
33+
(
34+
set -e
35+
36+
reponame="verify-fail-0-times"
37+
setup_remote_repo "$reponame"
38+
clone_repo "$reponame" "$reponame"
39+
40+
git lfs track "*.dat"
41+
git add .gitattributes
42+
git commit -m "initial commit"
43+
44+
contents="send-verify-action"
45+
contents_oid="$(calc_oid "$contents")"
46+
contents_short_oid="$(echo "$contents_oid" | head -c 7)"
47+
printf "$contents" > a.dat
48+
49+
git add a.dat
50+
git commit -m "add a.dat"
51+
52+
GIT_TRACE=1 git push origin master 2>&1 | tee push.log
53+
54+
[ "0" -eq "${PIPESTATUS[0]}" ]
55+
[ "1" -eq "$(grep -c "verify $contents_short_oid attempt" push.log)" ]
56+
)
57+
end_test
58+
59+
begin_test "verify with retries (insufficient retries)"
60+
(
61+
set -e
62+
63+
reponame="verify-fail-10-times"
64+
setup_remote_repo "$reponame"
65+
clone_repo "$reponame" "$reponame"
66+
67+
git lfs track "*.dat"
68+
git add .gitattributes
69+
git commit -m "initial commit"
70+
71+
contents="send-verify-action"
72+
contents_oid="$(calc_oid "$contents")"
73+
contents_short_oid="$(echo "$contents_oid" | head -c 7)"
74+
printf "$contents" > a.dat
75+
76+
git add a.dat
77+
git commit -m "add a.dat"
78+
79+
set +e
80+
GIT_TRACE=1 git push origin master 2>&1 | tee push.log
81+
if [ "0" -eq "${PIPESTATUS[0]}" ]; then
82+
echo >&2 "verify: expected \"git push\" to fail, didn't ..."
83+
exit 1
84+
fi
85+
set -e
86+
87+
[ "3" -eq "$(grep -c "verify $contents_short_oid attempt" push.log)" ]
88+
)
89+
end_test
90+
91+
begin_test "verify with retries (bad .gitconfig)"
92+
(
93+
set -e
94+
95+
reponame="bad-config-verify-fail-2-times"
96+
setup_remote_repo "$reponame"
97+
clone_repo "$reponame" "$reponame"
98+
99+
# Invalid `lfs.transfer.maxverifies` will default to 3.
100+
git config "lfs.transfer.maxverifies" "-1"
101+
102+
git lfs track "*.dat"
103+
git add .gitattributes
104+
git commit -m "initial commit"
105+
106+
contents="send-verify-action"
107+
contents_oid="$(calc_oid "$contents")"
108+
contents_short_oid="$(echo "$contents_oid" | head -c 7)"
109+
printf "$contents" > a.dat
110+
111+
git add a.dat
112+
git commit -m "add a.dat"
113+
114+
GIT_TRACE=1 git push origin master 2>&1 | tee push.log
115+
116+
[ "0" -eq "${PIPESTATUS[0]}" ]
117+
[ "2" -eq "$(grep -c "verify $contents_short_oid attempt" push.log)" ]
118+
)
119+
end_test

‎tools/str_tools.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
package tools
2+
3+
import "regexp"
4+
5+
var (
6+
// quoteFieldRe greedily matches between matching pairs of '', "", or
7+
// non-word characters.
8+
quoteFieldRe = regexp.MustCompile("'(.*)'|\"(.*)\"|(\\S*)")
9+
)
10+
11+
// QuotedFields is an alternative to strings.Fields (see:
12+
// https://golang.org/pkg/strings#Fields) that respects spaces between matching
13+
// pairs of quotation delimeters.
14+
//
15+
// For instance, the quoted fields of the string "foo bar 'baz etc'" would be:
16+
// []string{"foo", "bar", "baz etc"}
17+
//
18+
// Whereas the same argument given to strings.Fields, would return:
19+
// []string{"foo", "bar", "'baz", "etc'"}
20+
func QuotedFields(s string) []string {
21+
submatches := quoteFieldRe.FindAllStringSubmatch(s, -1)
22+
out := make([]string, 0, len(submatches))
23+
24+
for _, matches := range submatches {
25+
// if a leading or trailing space is found, ignore that
26+
if matches[0] == "" {
27+
continue
28+
}
29+
30+
// otherwise, find the first non-empty match (inside balanced
31+
// quotes, or a space-delimited string)
32+
var str string
33+
for _, m := range matches[1:] {
34+
if len(m) > 0 {
35+
str = m
36+
break
37+
}
38+
}
39+
40+
out = append(out, str)
41+
}
42+
43+
return out
44+
}

‎tools/str_tools_test.go

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
package tools
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
)
8+
9+
type QuotedFieldsTestCase struct {
10+
Given string
11+
Expected []string
12+
}
13+
14+
func (c *QuotedFieldsTestCase) Assert(t *testing.T) {
15+
actual := QuotedFields(c.Given)
16+
17+
assert.Equal(t, c.Expected, actual,
18+
"tools: expected QuotedFields(%q) to equal %#v (was %#v)",
19+
c.Given, c.Expected, actual,
20+
)
21+
}
22+
23+
func TestQuotedFields(t *testing.T) {
24+
for desc, c := range map[string]QuotedFieldsTestCase{
25+
"simple": {`foo bar`, []string{"foo", "bar"}},
26+
"simple trailing": {`foo bar `, []string{"foo", "bar"}},
27+
"simple leading": {` foo bar`, []string{"foo", "bar"}},
28+
29+
"single quotes": {`foo 'bar baz'`, []string{"foo", "bar baz"}},
30+
"single quotes trailing": {`foo 'bar baz' `, []string{"foo", "bar baz"}},
31+
"single quotes leading": {` foo 'bar baz'`, []string{"foo", "bar baz"}},
32+
33+
"single quotes empty": {`foo ''`, []string{"foo", ""}},
34+
"single quotes trailing empty": {`foo '' `, []string{"foo", ""}},
35+
"single quotes leading empty": {` foo ''`, []string{"foo", ""}},
36+
37+
"double quotes": {`foo "bar baz"`, []string{"foo", "bar baz"}},
38+
"double quotes trailing": {`foo "bar baz" `, []string{"foo", "bar baz"}},
39+
"double quotes leading": {` foo "bar baz"`, []string{"foo", "bar baz"}},
40+
41+
"double quotes empty": {`foo ""`, []string{"foo", ""}},
42+
"double quotes trailing empty": {`foo "" `, []string{"foo", ""}},
43+
"double quotes leading empty": {` foo ""`, []string{"foo", ""}},
44+
45+
"nested single quotes": {`foo 'bar 'baz''`, []string{"foo", "bar 'baz'"}},
46+
"nested single quotes trailing": {`foo 'bar 'baz'' `, []string{"foo", "bar 'baz'"}},
47+
"nested single quotes leading": {` foo 'bar 'baz''`, []string{"foo", "bar 'baz'"}},
48+
49+
"nested single quotes empty": {`foo 'bar '''`, []string{"foo", "bar ''"}},
50+
"nested single quotes trailing empty": {`foo 'bar ''' `, []string{"foo", "bar ''"}},
51+
"nested single quotes leading empty": {` foo 'bar '''`, []string{"foo", "bar ''"}},
52+
53+
"nested double quotes": {`foo "bar "baz""`, []string{"foo", `bar "baz"`}},
54+
"nested double quotes trailing": {`foo "bar "baz"" `, []string{"foo", `bar "baz"`}},
55+
"nested double quotes leading": {` foo "bar "baz""`, []string{"foo", `bar "baz"`}},
56+
57+
"nested double quotes empty": {`foo "bar """`, []string{"foo", `bar ""`}},
58+
"nested double quotes trailing empty": {`foo "bar """ `, []string{"foo", `bar ""`}},
59+
"nested double quotes leading empty": {` foo "bar """`, []string{"foo", `bar ""`}},
60+
61+
"mixed quotes": {`foo 'bar "baz"'`, []string{"foo", `bar "baz"`}},
62+
"mixed quotes trailing": {`foo 'bar "baz"' `, []string{"foo", `bar "baz"`}},
63+
"mixed quotes leading": {` foo 'bar "baz"'`, []string{"foo", `bar "baz"`}},
64+
65+
"mixed quotes empty": {`foo 'bar ""'`, []string{"foo", `bar ""`}},
66+
"mixed quotes trailing empty": {`foo 'bar ""' `, []string{"foo", `bar ""`}},
67+
"mixed quotes leading empty": {` foo 'bar ""'`, []string{"foo", `bar ""`}},
68+
} {
69+
t.Log(desc)
70+
c.Assert(t)
71+
}
72+
}

‎tq/manifest.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88
)
99

1010
const (
11-
defaultMaxRetries = 1
11+
defaultMaxRetries = 8
1212
defaultConcurrentTransfers = 3
1313
)
1414

0 commit comments

Comments
 (0)