From 4bcd37d3bc436f202307897093d5ca162a688be2 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Wed, 20 Jun 2018 14:32:28 -0700 Subject: test-pkt-line: add unpack-sideband subcommand Add an 'unpack-sideband' subcommand to the test-pkt-line helper to enable unpacking packet line data sent multiplexed using a sideband. Signed-off-by: Brandon Williams Signed-off-by: Junio C Hamano --- t/helper/test-pkt-line.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) (limited to 't') diff --git a/t/helper/test-pkt-line.c b/t/helper/test-pkt-line.c index 0f19e53c75..30775f986f 100644 --- a/t/helper/test-pkt-line.c +++ b/t/helper/test-pkt-line.c @@ -1,3 +1,4 @@ +#include "cache.h" #include "pkt-line.h" static void pack_line(const char *line) @@ -48,6 +49,36 @@ static void unpack(void) } } +static void unpack_sideband(void) +{ + struct packet_reader reader; + packet_reader_init(&reader, 0, NULL, 0, + PACKET_READ_GENTLE_ON_EOF | + PACKET_READ_CHOMP_NEWLINE); + + while (packet_reader_read(&reader) != PACKET_READ_EOF) { + int band; + int fd; + + switch (reader.status) { + case PACKET_READ_EOF: + break; + case PACKET_READ_NORMAL: + band = reader.line[0] & 0xff; + if (band < 1 || band > 2) + die("unexpected side band %d", band); + fd = band; + + write_or_die(fd, reader.line + 1, reader.pktlen - 1); + break; + case PACKET_READ_FLUSH: + return; + case PACKET_READ_DELIM: + break; + } + } +} + int cmd_main(int argc, const char **argv) { if (argc < 2) @@ -57,6 +88,8 @@ int cmd_main(int argc, const char **argv) pack(argc - 2, argv + 2); else if (!strcmp(argv[1], "unpack")) unpack(); + else if (!strcmp(argv[1], "unpack-sideband")) + unpack_sideband(); else die("invalid argument '%s'", argv[1]); -- cgit v1.2.3 From 516e2b76bdcf53e757309481fa0e663217ee8039 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Wed, 27 Jun 2018 15:30:17 -0700 Subject: upload-pack: implement ref-in-want Currently, while performing packfile negotiation, clients are only allowed to specify their desired objects using object ids. This causes a vulnerability to failure when an object turns non-existent during negotiation, which may happen if, for example, the desired repository is provided by multiple Git servers in a load-balancing arrangement and there exists replication delay. In order to eliminate this vulnerability, implement the ref-in-want feature for the 'fetch' command in protocol version 2. This feature enables the 'fetch' command to support requests in the form of ref names through a new "want-ref " parameter. At the conclusion of negotiation, the server will send a list of all of the wanted references (as provided by "want-ref" lines) in addition to the generated packfile. Signed-off-by: Brandon Williams Signed-off-by: Junio C Hamano --- t/t5703-upload-pack-ref-in-want.sh | 160 +++++++++++++++++++++++++++++++++++++ 1 file changed, 160 insertions(+) create mode 100755 t/t5703-upload-pack-ref-in-want.sh (limited to 't') diff --git a/t/t5703-upload-pack-ref-in-want.sh b/t/t5703-upload-pack-ref-in-want.sh new file mode 100755 index 0000000000..da86f7cde0 --- /dev/null +++ b/t/t5703-upload-pack-ref-in-want.sh @@ -0,0 +1,160 @@ +#!/bin/sh + +test_description='upload-pack ref-in-want' + +. ./test-lib.sh + +get_actual_refs () { + sed -n -e '/wanted-refs/,/0001/{ + /wanted-refs/d + /0001/d + p + }' actual_refs +} + +get_actual_commits () { + sed -n -e '/packfile/,/0000/{ + /packfile/d + p + }' o.pack && + git index-pack o.pack && + git verify-pack -v o.idx | grep commit | cut -c-40 | sort >actual_commits +} + +check_output () { + get_actual_refs && + test_cmp expected_refs actual_refs && + get_actual_commits && + test_cmp expected_commits actual_commits +} + +# c(o/foo) d(o/bar) +# \ / +# b e(baz) f(master) +# \__ | __/ +# \ | / +# a +test_expect_success 'setup repository' ' + test_commit a && + git checkout -b o/foo && + test_commit b && + test_commit c && + git checkout -b o/bar b && + test_commit d && + git checkout -b baz a && + test_commit e && + git checkout master && + test_commit f +' + +test_expect_success 'config controls ref-in-want advertisement' ' + git serve --advertise-capabilities >out && + ! grep -a ref-in-want out && + + git config uploadpack.allowRefInWant false && + git serve --advertise-capabilities >out && + ! grep -a ref-in-want out && + + git config uploadpack.allowRefInWant true && + git serve --advertise-capabilities >out && + grep -a ref-in-want out +' + +test_expect_success 'invalid want-ref line' ' + test-pkt-line pack >in <<-EOF && + command=fetch + 0001 + no-progress + want-ref refs/heads/non-existent + done + 0000 + EOF + + test_must_fail git serve --stateless-rpc 2>out expected_refs <<-EOF && + $(git rev-parse f) refs/heads/master + EOF + git rev-parse f | sort >expected_commits && + + test-pkt-line pack >in <<-EOF && + command=fetch + 0001 + no-progress + want-ref refs/heads/master + have $(git rev-parse a) + done + 0000 + EOF + + git serve --stateless-rpc >out expected_refs <<-EOF && + $(git rev-parse c) refs/heads/o/foo + $(git rev-parse d) refs/heads/o/bar + EOF + git rev-parse c d | sort >expected_commits && + + test-pkt-line pack >in <<-EOF && + command=fetch + 0001 + no-progress + want-ref refs/heads/o/foo + want-ref refs/heads/o/bar + have $(git rev-parse b) + done + 0000 + EOF + + git serve --stateless-rpc >out expected_refs <<-EOF && + $(git rev-parse f) refs/heads/master + EOF + git rev-parse e f | sort >expected_commits && + + test-pkt-line pack >in <<-EOF && + command=fetch + 0001 + no-progress + want-ref refs/heads/master + want $(git rev-parse e) + have $(git rev-parse a) + done + 0000 + EOF + + git serve --stateless-rpc >out expected_refs <<-EOF && + $(git rev-parse c) refs/heads/o/foo + EOF + >expected_commits && + + test-pkt-line pack >in <<-EOF && + command=fetch + 0001 + no-progress + want-ref refs/heads/o/foo + have $(git rev-parse c) + done + 0000 + EOF + + git serve --stateless-rpc >out Date: Wed, 27 Jun 2018 15:30:18 -0700 Subject: upload-pack: test negotiation with changing repository Add tests to check the behavior of fetching from a repository which changes between rounds of negotiation (for example, when different servers in a load-balancing agreement participate in the same stateless RPC negotiation). This forms a baseline of comparison to the ref-in-want functionality (which will be introduced to the client in subsequent commits), and ensures that subsequent commits do not change existing behavior. As part of this effort, a mechanism to substitute strings in a single HTTP response is added. Signed-off-by: Brandon Williams Signed-off-by: Junio C Hamano --- t/lib-httpd.sh | 1 + t/lib-httpd/apache.conf | 8 +++++ t/lib-httpd/apply-one-time-sed.sh | 22 ++++++++++++ t/t5703-upload-pack-ref-in-want.sh | 68 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 99 insertions(+) create mode 100644 t/lib-httpd/apply-one-time-sed.sh (limited to 't') diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh index 435a37465a..ed41b155af 100644 --- a/t/lib-httpd.sh +++ b/t/lib-httpd.sh @@ -132,6 +132,7 @@ prepare_httpd() { cp "$TEST_PATH"/passwd "$HTTPD_ROOT_PATH" install_script broken-smart-http.sh install_script error.sh + install_script apply-one-time-sed.sh ln -s "$LIB_HTTPD_MODULE_PATH" "$HTTPD_ROOT_PATH/modules" diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf index 724d9ae462..581c010d8f 100644 --- a/t/lib-httpd/apache.conf +++ b/t/lib-httpd/apache.conf @@ -111,9 +111,14 @@ Alias /auth/dumb/ www/auth/dumb/ SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH} SetEnv GIT_HTTP_EXPORT_ALL + + SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH} + SetEnv GIT_HTTP_EXPORT_ALL + ScriptAliasMatch /smart_*[^/]*/(.*) ${GIT_EXEC_PATH}/git-http-backend/$1 ScriptAlias /broken_smart/ broken-smart-http.sh/ ScriptAlias /error/ error.sh/ +ScriptAliasMatch /one_time_sed/(.*) apply-one-time-sed.sh/$1 Options FollowSymlinks @@ -123,6 +128,9 @@ ScriptAlias /error/ error.sh/ Options ExecCGI + + Options ExecCGI + Options ExecCGI diff --git a/t/lib-httpd/apply-one-time-sed.sh b/t/lib-httpd/apply-one-time-sed.sh new file mode 100644 index 0000000000..fcef728925 --- /dev/null +++ b/t/lib-httpd/apply-one-time-sed.sh @@ -0,0 +1,22 @@ +#!/bin/sh + +# If "one-time-sed" exists in $HTTPD_ROOT_PATH, run sed on the HTTP response, +# using the contents of "one-time-sed" as the sed command to be run. If the +# response was modified as a result, delete "one-time-sed" so that subsequent +# HTTP responses are no longer modified. +# +# This can be used to simulate the effects of the repository changing in +# between HTTP request-response pairs. +if [ -e one-time-sed ]; then + "$GIT_EXEC_PATH/git-http-backend" >out + sed "$(cat one-time-sed)" out_modified + + if diff out out_modified >/dev/null; then + cat out + else + cat out_modified + rm one-time-sed + fi +else + "$GIT_EXEC_PATH/git-http-backend" +fi diff --git a/t/t5703-upload-pack-ref-in-want.sh b/t/t5703-upload-pack-ref-in-want.sh index da86f7cde0..32527a59c4 100755 --- a/t/t5703-upload-pack-ref-in-want.sh +++ b/t/t5703-upload-pack-ref-in-want.sh @@ -157,4 +157,72 @@ test_expect_success 'want-ref with ref we already have commit for' ' check_output ' +. "$TEST_DIRECTORY"/lib-httpd.sh +start_httpd + +REPO="$HTTPD_DOCUMENT_ROOT_PATH/repo" +LOCAL_PRISTINE="$(pwd)/local_pristine" + +test_expect_success 'setup repos for change-while-negotiating test' ' + ( + git init "$REPO" && + cd "$REPO" && + >.git/git-daemon-export-ok && + test_commit m1 && + git tag -d m1 && + + # Local repo with many commits (so that negotiation will take + # more than 1 request/response pair) + git clone "http://127.0.0.1:$LIB_HTTPD_PORT/smart/repo" "$LOCAL_PRISTINE" && + cd "$LOCAL_PRISTINE" && + git checkout -b side && + for i in $(seq 1 33); do test_commit s$i; done && + + # Add novel commits to upstream + git checkout master && + cd "$REPO" && + test_commit m2 && + test_commit m3 && + git tag -d m2 m3 + ) && + git -C "$LOCAL_PRISTINE" remote set-url origin "http://127.0.0.1:$LIB_HTTPD_PORT/one_time_sed/repo" && + git -C "$LOCAL_PRISTINE" config protocol.version 2 +' + +inconsistency () { + # Simulate that the server initially reports $2 as the ref + # corresponding to $1, and after that, $1 as the ref corresponding to + # $1. This corresponds to the real-life situation where the server's + # repository appears to change during negotiation, for example, when + # different servers in a load-balancing arrangement serve (stateless) + # RPCs during a single negotiation. + printf "s/%s/%s/" \ + $(git -C "$REPO" rev-parse $1 | tr -d "\n") \ + $(git -C "$REPO" rev-parse $2 | tr -d "\n") \ + >"$HTTPD_ROOT_PATH/one-time-sed" +} + +test_expect_success 'server is initially ahead - no ref in want' ' + git -C "$REPO" config uploadpack.allowRefInWant false && + rm -rf local && + cp -r "$LOCAL_PRISTINE" local && + inconsistency master 1234567890123456789012345678901234567890 && + test_must_fail git -C local fetch 2>err && + grep "ERR upload-pack: not our ref" err +' + +test_expect_success 'server is initially behind - no ref in want' ' + git -C "$REPO" config uploadpack.allowRefInWant false && + rm -rf local && + cp -r "$LOCAL_PRISTINE" local && + inconsistency master "master^" && + git -C local fetch && + + git -C "$REPO" rev-parse --verify "master^" >expected && + git -C local rev-parse --verify refs/remotes/origin/master >actual && + test_cmp expected actual +' + +stop_httpd + test_done -- cgit v1.2.3 From 733020517a1baa6f4f76bb7bf48d8d8d14eecd6c Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Wed, 27 Jun 2018 15:30:23 -0700 Subject: fetch-pack: implement ref-in-want Implement ref-in-want on the client side so that when a server supports the "ref-in-want" feature, a client will send "want-ref" lines for each reference the client wants to fetch. This feature allows clients to tolerate inconsistencies that exist when a remote repository's refs change during the course of negotiation. This allows a client to request to request a particular ref without specifying the OID of the ref. This means that instead of hitting an error when a ref no longer points at the OID it did at the beginning of negotiation, negotiation can continue and the value of that ref will be sent at the termination of negotiation, just before a packfile is sent. More information on the ref-in-want feature can be found in Documentation/technical/protocol-v2.txt. Signed-off-by: Brandon Williams Signed-off-by: Junio C Hamano --- t/t5703-upload-pack-ref-in-want.sh | 149 +++++++++++++++++++++++++++++++++++++ 1 file changed, 149 insertions(+) (limited to 't') diff --git a/t/t5703-upload-pack-ref-in-want.sh b/t/t5703-upload-pack-ref-in-want.sh index 32527a59c4..a73c55a47e 100755 --- a/t/t5703-upload-pack-ref-in-want.sh +++ b/t/t5703-upload-pack-ref-in-want.sh @@ -211,6 +211,18 @@ test_expect_success 'server is initially ahead - no ref in want' ' grep "ERR upload-pack: not our ref" err ' +test_expect_success 'server is initially ahead - ref in want' ' + git -C "$REPO" config uploadpack.allowRefInWant true && + rm -rf local && + cp -r "$LOCAL_PRISTINE" local && + inconsistency master 1234567890123456789012345678901234567890 && + git -C local fetch && + + git -C "$REPO" rev-parse --verify master >expected && + git -C local rev-parse --verify refs/remotes/origin/master >actual && + test_cmp expected actual +' + test_expect_success 'server is initially behind - no ref in want' ' git -C "$REPO" config uploadpack.allowRefInWant false && rm -rf local && @@ -223,6 +235,143 @@ test_expect_success 'server is initially behind - no ref in want' ' test_cmp expected actual ' +test_expect_success 'server is initially behind - ref in want' ' + git -C "$REPO" config uploadpack.allowRefInWant true && + rm -rf local && + cp -r "$LOCAL_PRISTINE" local && + inconsistency master "master^" && + git -C local fetch && + + git -C "$REPO" rev-parse --verify "master" >expected && + git -C local rev-parse --verify refs/remotes/origin/master >actual && + test_cmp expected actual +' + +test_expect_success 'server loses a ref - ref in want' ' + git -C "$REPO" config uploadpack.allowRefInWant true && + rm -rf local && + cp -r "$LOCAL_PRISTINE" local && + echo "s/master/raster/" >"$HTTPD_ROOT_PATH/one-time-sed" && + test_must_fail git -C local fetch 2>err && + + grep "ERR unknown ref refs/heads/raster" err +' + stop_httpd +REPO="$(pwd)/repo" +LOCAL_PRISTINE="$(pwd)/local_pristine" + +# $REPO +# c(o/foo) d(o/bar) +# \ / +# b e(baz) f(master) +# \__ | __/ +# \ | / +# a +# +# $LOCAL_PRISTINE +# s32(side) +# | +# . +# . +# | +# a(master) +test_expect_success 'setup repos for fetching with ref-in-want tests' ' + ( + git init "$REPO" && + cd "$REPO" && + test_commit a && + + # Local repo with many commits (so that negotiation will take + # more than 1 request/response pair) + rm -rf "$LOCAL_PRISTINE" && + git clone "file://$REPO" "$LOCAL_PRISTINE" && + cd "$LOCAL_PRISTINE" && + git checkout -b side && + for i in $(seq 1 33); do test_commit s$i; done && + + # Add novel commits to upstream + git checkout master && + cd "$REPO" && + git checkout -b o/foo && + test_commit b && + test_commit c && + git checkout -b o/bar b && + test_commit d && + git checkout -b baz a && + test_commit e && + git checkout master && + test_commit f + ) && + git -C "$REPO" config uploadpack.allowRefInWant true && + git -C "$LOCAL_PRISTINE" config protocol.version 2 +' + +test_expect_success 'fetching with exact OID' ' + test_when_finished "rm -f log" && + + rm -rf local && + cp -r "$LOCAL_PRISTINE" local && + GIT_TRACE_PACKET="$(pwd)/log" git -C local fetch origin \ + $(git -C "$REPO" rev-parse d):refs/heads/actual && + + git -C "$REPO" rev-parse "d" >expected && + git -C local rev-parse refs/heads/actual >actual && + test_cmp expected actual && + grep "want $(git -C "$REPO" rev-parse d)" log +' + +test_expect_success 'fetching multiple refs' ' + test_when_finished "rm -f log" && + + rm -rf local && + cp -r "$LOCAL_PRISTINE" local && + GIT_TRACE_PACKET="$(pwd)/log" git -C local fetch origin master baz && + + git -C "$REPO" rev-parse "master" "baz" >expected && + git -C local rev-parse refs/remotes/origin/master refs/remotes/origin/baz >actual && + test_cmp expected actual && + grep "want-ref refs/heads/master" log && + grep "want-ref refs/heads/baz" log +' + +test_expect_success 'fetching ref and exact OID' ' + test_when_finished "rm -f log" && + + rm -rf local && + cp -r "$LOCAL_PRISTINE" local && + GIT_TRACE_PACKET="$(pwd)/log" git -C local fetch origin \ + master $(git -C "$REPO" rev-parse b):refs/heads/actual && + + git -C "$REPO" rev-parse "master" "b" >expected && + git -C local rev-parse refs/remotes/origin/master refs/heads/actual >actual && + test_cmp expected actual && + grep "want $(git -C "$REPO" rev-parse b)" log && + grep "want-ref refs/heads/master" log +' + +test_expect_success 'fetching with wildcard that does not match any refs' ' + test_when_finished "rm -f log" && + + rm -rf local && + cp -r "$LOCAL_PRISTINE" local && + git -C local fetch origin refs/heads/none*:refs/heads/* >out && + test_must_be_empty out +' + +test_expect_success 'fetching with wildcard that matches multiple refs' ' + test_when_finished "rm -f log" && + + rm -rf local && + cp -r "$LOCAL_PRISTINE" local && + GIT_TRACE_PACKET="$(pwd)/log" git -C local fetch origin refs/heads/o*:refs/heads/o* && + + git -C "$REPO" rev-parse "o/foo" "o/bar" >expected && + git -C local rev-parse "o/foo" "o/bar" >actual && + test_cmp expected actual && + grep "want-ref refs/heads/o/foo" log && + grep "want-ref refs/heads/o/bar" log +' + test_done -- cgit v1.2.3 From cf1e7c07705eb21c30d0ee414810e7bc8fdf7d82 Mon Sep 17 00:00:00 2001 From: Jonathan Tan Date: Mon, 2 Jul 2018 15:08:43 -0700 Subject: fetch-pack: write shallow, then check connectivity When fetching, connectivity is checked after the shallow file is updated. There are 2 issues with this: (1) the connectivity check is only performed up to ancestors of existing refs (which is not thorough enough if we were deepening an existing ref in the first place), and (2) there is no rollback of the shallow file if the connectivity check fails. To solve (1), update the connectivity check to check the ancestry chain completely in the case of a deepening fetch by refraining from passing "--not --all" when invoking rev-list in connected.c. To solve (2), have fetch_pack() perform its own connectivity check before updating the shallow file. To support existing use cases in which "git fetch-pack" is used to download objects without much regard as to the connectivity of the resulting objects with respect to the existing repository, the connectivity check is only done if necessary (that is, the fetch is not a clone, and the fetch involves shallow/deepen functionality). "git fetch" still performs its own connectivity check, preserving correctness but sometimes performing redundant work. This redundancy is mitigated by the fact that fetch_pack() reports if it has performed a connectivity check itself, and if the transport supports connect or stateless-connect, it will bubble up that report so that "git fetch" knows not to perform the connectivity check in such a case. This was noticed when a user tried to deepen an existing repository by fetching with --no-shallow from a server that did not send all necessary objects - the connectivity check as run by "git fetch" succeeded, but a subsequent "git fsck" failed. Signed-off-by: Jonathan Tan Signed-off-by: Junio C Hamano --- t/t5537-fetch-shallow.sh | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) (limited to 't') diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh index df8d2f095a..a7afb66049 100755 --- a/t/t5537-fetch-shallow.sh +++ b/t/t5537-fetch-shallow.sh @@ -186,4 +186,47 @@ EOF test_cmp expect actual ' +. "$TEST_DIRECTORY"/lib-httpd.sh +start_httpd + +REPO="$HTTPD_DOCUMENT_ROOT_PATH/repo" + +test_expect_success 'shallow fetches check connectivity before writing shallow file' ' + rm -rf "$REPO" client && + + git init "$REPO" && + test_commit -C "$REPO" one && + test_commit -C "$REPO" two && + test_commit -C "$REPO" three && + + git init client && + + # Use protocol v2 to ensure that shallow information is sent exactly + # once by the server, since we are planning to manipulate it. + git -C "$REPO" config protocol.version 2 && + git -C client config protocol.version 2 && + + git -C client fetch --depth=2 "$HTTPD_URL/one_time_sed/repo" master:a_branch && + + # Craft a situation in which the server sends back an unshallow request + # with an empty packfile. This is done by refetching with a shorter + # depth (to ensure that the packfile is empty), and overwriting the + # shallow line in the response with the unshallow line we want. + printf "s/0034shallow %s/0036unshallow %s/" \ + "$(git -C "$REPO" rev-parse HEAD)" \ + "$(git -C "$REPO" rev-parse HEAD^)" \ + >"$HTTPD_ROOT_PATH/one-time-sed" && + test_must_fail git -C client fetch --depth=1 "$HTTPD_URL/one_time_sed/repo" \ + master:a_branch && + + # Ensure that the one-time-sed script was used. + ! test -e "$HTTPD_ROOT_PATH/one-time-sed" && + + # Ensure that the resulting repo is consistent, despite our failure to + # fetch. + git -C client fsck +' + +stop_httpd + test_done -- cgit v1.2.3