Skip to content

Commit 48d23c1

Browse files
gitstergitster
committed
Merge branch 'dt/smart-http-detect-server-going-away' into maint
When the http server gives an incomplete response to a smart-http rpc call, it could lead to client waiting for a full response that will never come. Teach the client side to notice this condition and abort the transfer. An improvement counterproposal has failed. cf. <20161114194049.mktpsvgdhex2f4zv@sigill.intra.peff.net> * dt/smart-http-detect-server-going-away: upload-pack: optionally allow fetching any sha1 remote-curl: don't hang when a server dies before any output
2 parents 8554ee1 + f8edeaa commit 48d23c1

File tree

5 files changed

+77
-4
lines changed

5 files changed

+77
-4
lines changed

‎Documentation/config.txt‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2971,6 +2971,11 @@ uploadpack.allowReachableSHA1InWant::
29712971
calculating object reachability is computationally expensive.
29722972
Defaults to `false`.
29732973

2974+
uploadpack.allowAnySHA1InWant::
2975+
Allow `upload-pack` to accept a fetch request that asks for any
2976+
object at all.
2977+
Defaults to `false`.
2978+
29742979
uploadpack.keepAlive::
29752980
When `upload-pack` has started `pack-objects`, there may be a
29762981
quiet period while `pack-objects` prepares the pack. Normally

‎Documentation/git-fetch-pack.txt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,9 +119,9 @@ be in a separate packet, and the list must end with a flush packet.
119119
$GIT_DIR (e.g. "HEAD", "refs/heads/master"). When
120120
unspecified, update from all heads the remote side has.
121121
+
122-
If the remote has enabled the options `uploadpack.allowTipSHA1InWant` or
123-
`uploadpack.allowReachableSHA1InWant`, they may alternatively be 40-hex
124-
sha1s present on the remote.
122+
If the remote has enabled the options `uploadpack.allowTipSHA1InWant`,
123+
`uploadpack.allowReachableSHA1InWant`, or `uploadpack.allowAnySHA1InWant`,
124+
they may alternatively be 40-hex sha1s present on the remote.
125125

126126
SEE ALSO
127127
--------

‎remote-curl.c‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,7 @@ struct rpc_state {
404404
size_t pos;
405405
int in;
406406
int out;
407+
int any_written;
407408
struct strbuf result;
408409
unsigned gzip_request : 1;
409410
unsigned initial_buffer : 1;
@@ -460,6 +461,8 @@ static size_t rpc_in(char *ptr, size_t eltsize,
460461
{
461462
size_t size = eltsize * nmemb;
462463
struct rpc_state *rpc = buffer_;
464+
if (size)
465+
rpc->any_written = 1;
463466
write_or_die(rpc->in, ptr, size);
464467
return size;
465468
}
@@ -663,6 +666,8 @@ static int post_rpc(struct rpc_state *rpc)
663666
curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, rpc_in);
664667
curl_easy_setopt(slot->curl, CURLOPT_FILE, rpc);
665668

669+
670+
rpc->any_written = 0;
666671
err = run_slot(slot, NULL);
667672
if (err == HTTP_REAUTH && !large_request) {
668673
credential_fill(&http_auth);
@@ -671,6 +676,9 @@ static int post_rpc(struct rpc_state *rpc)
671676
if (err != HTTP_OK)
672677
err = -1;
673678

679+
if (!rpc->any_written)
680+
err = -1;
681+
674682
curl_slist_free_all(headers);
675683
free(gzip_body);
676684
return err;

‎t/t5551-http-fetch-smart.sh‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,58 @@ test_expect_success 'large fetch-pack requests can be split across POSTs' '
280280
test_line_count = 2 posts
281281
'
282282

283+
test_expect_success 'test allowreachablesha1inwant' '
284+
test_when_finished "rm -rf test_reachable.git" &&
285+
server="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
286+
master_sha=$(git -C "$server" rev-parse refs/heads/master) &&
287+
git -C "$server" config uploadpack.allowreachablesha1inwant 1 &&
288+
289+
git init --bare test_reachable.git &&
290+
git -C test_reachable.git remote add origin "$HTTPD_URL/smart/repo.git" &&
291+
git -C test_reachable.git fetch origin "$master_sha"
292+
'
293+
294+
test_expect_success 'test allowreachablesha1inwant with unreachable' '
295+
test_when_finished "rm -rf test_reachable.git; git reset --hard $(git rev-parse HEAD)" &&
296+
297+
#create unreachable sha
298+
echo content >file2 &&
299+
git add file2 &&
300+
git commit -m two &&
301+
git push public HEAD:refs/heads/doomed &&
302+
git push public :refs/heads/doomed &&
303+
304+
server="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
305+
master_sha=$(git -C "$server" rev-parse refs/heads/master) &&
306+
git -C "$server" config uploadpack.allowreachablesha1inwant 1 &&
307+
308+
git init --bare test_reachable.git &&
309+
git -C test_reachable.git remote add origin "$HTTPD_URL/smart/repo.git" &&
310+
test_must_fail git -C test_reachable.git fetch origin "$(git rev-parse HEAD)"
311+
'
312+
313+
test_expect_success 'test allowanysha1inwant with unreachable' '
314+
test_when_finished "rm -rf test_reachable.git; git reset --hard $(git rev-parse HEAD)" &&
315+
316+
#create unreachable sha
317+
echo content >file2 &&
318+
git add file2 &&
319+
git commit -m two &&
320+
git push public HEAD:refs/heads/doomed &&
321+
git push public :refs/heads/doomed &&
322+
323+
server="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
324+
master_sha=$(git -C "$server" rev-parse refs/heads/master) &&
325+
git -C "$server" config uploadpack.allowreachablesha1inwant 1 &&
326+
327+
git init --bare test_reachable.git &&
328+
git -C test_reachable.git remote add origin "$HTTPD_URL/smart/repo.git" &&
329+
test_must_fail git -C test_reachable.git fetch origin "$(git rev-parse HEAD)" &&
330+
331+
git -C "$server" config uploadpack.allowanysha1inwant 1 &&
332+
git -C test_reachable.git fetch origin "$(git rev-parse HEAD)"
333+
'
334+
283335
test_expect_success EXPENSIVE 'http can handle enormous ref negotiation' '
284336
(
285337
cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&

‎upload-pack.c‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ static int no_progress, daemon_mode;
4646
#define ALLOW_TIP_SHA1 01
4747
/* Allow request of a sha1 if it is reachable from a ref (possibly hidden ref). */
4848
#define ALLOW_REACHABLE_SHA1 02
49+
/* Allow request of any sha1. Implies ALLOW_TIP_SHA1 and ALLOW_REACHABLE_SHA1. */
50+
#define ALLOW_ANY_SHA1 07
4951
static unsigned int allow_unadvertised_object_request;
5052
static int shallow_nr;
5153
static struct object_array have_obj;
@@ -825,7 +827,8 @@ static void receive_needs(void)
825827
sha1_to_hex(sha1_buf));
826828
if (!(o->flags & WANTED)) {
827829
o->flags |= WANTED;
828-
if (!is_our_ref(o))
830+
if (!((allow_unadvertised_object_request & ALLOW_ANY_SHA1) == ALLOW_ANY_SHA1
831+
|| is_our_ref(o)))
829832
has_non_tip = 1;
830833
add_object_array(o, NULL, &want_obj);
831834
}
@@ -1008,6 +1011,11 @@ static int upload_pack_config(const char *var, const char *value, void *unused)
10081011
allow_unadvertised_object_request |= ALLOW_REACHABLE_SHA1;
10091012
else
10101013
allow_unadvertised_object_request &= ~ALLOW_REACHABLE_SHA1;
1014+
} else if (!strcmp("uploadpack.allowanysha1inwant", var)) {
1015+
if (git_config_bool(var, value))
1016+
allow_unadvertised_object_request |= ALLOW_ANY_SHA1;
1017+
else
1018+
allow_unadvertised_object_request &= ~ALLOW_ANY_SHA1;
10111019
} else if (!strcmp("uploadpack.keepalive", var)) {
10121020
keepalive = git_config_int(var, value);
10131021
if (!keepalive)

0 commit comments

Comments
 (0)