Skip to content

Commit

Permalink
Merge branch 'jk/fix-proto-downgrade-to-v0'
Browse files Browse the repository at this point in the history
Transports that do not support protocol v2 did not correctly fall
back to protocol v0 under certain conditions, which has been
corrected.

* jk/fix-proto-downgrade-to-v0:
  git_connect(): fix corner cases in downgrading v2 to v0
  • Loading branch information
gitster committed Mar 28, 2023
2 parents 8069aa0 + eaa0fd6 commit f879501
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 13 deletions.
4 changes: 2 additions & 2 deletions builtin/fetch-pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,8 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
int flags = args.verbose ? CONNECT_VERBOSE : 0;
if (args.diag_url)
flags |= CONNECT_DIAG_URL;
conn = git_connect(fd, dest, args.uploadpack,
flags);
conn = git_connect(fd, dest, "git-upload-pack",
args.uploadpack, flags);
if (!conn)
return args.diag_url ? 0 : 1;
}
Expand Down
2 changes: 1 addition & 1 deletion builtin/send-pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
fd[0] = 0;
fd[1] = 1;
} else {
conn = git_connect(fd, dest, receivepack,
conn = git_connect(fd, dest, "git-receive-pack", receivepack,
args.verbose ? CONNECT_VERBOSE : 0);
}

Expand Down
8 changes: 5 additions & 3 deletions connect.c
Original file line number Diff line number Diff line change
Expand Up @@ -1408,6 +1408,7 @@ static void fill_ssh_args(struct child_process *conn, const char *ssh_host,
* the connection failed).
*/
struct child_process *git_connect(int fd[2], const char *url,
const char *name,
const char *prog, int flags)
{
char *hostandport, *path;
Expand All @@ -1417,10 +1418,11 @@ struct child_process *git_connect(int fd[2], const char *url,

/*
* NEEDSWORK: If we are trying to use protocol v2 and we are planning
* to perform a push, then fallback to v0 since the client doesn't know
* how to push yet using v2.
* to perform any operation that doesn't involve upload-pack (i.e., a
* fetch, ls-remote, etc), then fallback to v0 since we don't know how
* to do anything else (like push or remote archive) via v2.
*/
if (version == protocol_v2 && !strcmp("git-receive-pack", prog))
if (version == protocol_v2 && strcmp("git-upload-pack", name))
version = protocol_v0;

/* Without this we cannot rely on waitpid() to tell
Expand Down
2 changes: 1 addition & 1 deletion connect.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#define CONNECT_DIAG_URL (1u << 1)
#define CONNECT_IPV4 (1u << 2)
#define CONNECT_IPV6 (1u << 3)
struct child_process *git_connect(int fd[2], const char *url, const char *prog, int flags);
struct child_process *git_connect(int fd[2], const char *url, const char *name, const char *prog, int flags);
int finish_connect(struct child_process *conn);
int git_connection_is_socket(struct child_process *conn);
int server_supports(const char *feature);
Expand Down
7 changes: 4 additions & 3 deletions remote-curl.c
Original file line number Diff line number Diff line change
Expand Up @@ -474,10 +474,11 @@ static struct discovery *discover_refs(const char *service, int for_push)

/*
* NEEDSWORK: If we are trying to use protocol v2 and we are planning
* to perform a push, then fallback to v0 since the client doesn't know
* how to push yet using v2.
* to perform any operation that doesn't involve upload-pack (i.e., a
* fetch, ls-remote, etc), then fallback to v0 since we don't know how
* to do anything else (like push or remote archive) via v2.
*/
if (version == protocol_v2 && !strcmp("git-receive-pack", service))
if (version == protocol_v2 && strcmp("git-upload-pack", service))
version = protocol_v0;

/* Add the extra Git-Protocol header */
Expand Down
27 changes: 27 additions & 0 deletions t/t5702-protocol-v2.sh
Original file line number Diff line number Diff line change
Expand Up @@ -728,6 +728,33 @@ test_expect_success 'file:// --negotiate-only with protocol v0' '
test_i18ngrep "negotiate-only requires protocol v2" err
'

test_expect_success 'push with custom path does not request v2' '
rm -f env.trace &&
git -C client push \
--receive-pack="env >../env.trace; git-receive-pack" \
origin HEAD:refs/heads/custom-push-test &&
test_path_is_file env.trace &&
! grep ^GIT_PROTOCOL env.trace
'

test_expect_success 'fetch with custom path does request v2' '
rm -f env.trace &&
git -C client fetch \
--upload-pack="env >../env.trace; git-upload-pack" \
origin HEAD &&
grep ^GIT_PROTOCOL=version=2 env.trace
'

test_expect_success 'archive with custom path does not request v2' '
rm -f env.trace &&
git -C client archive \
--exec="env >../env.trace; git-upload-archive" \
--remote=origin \
HEAD >/dev/null &&
test_path_is_file env.trace &&
! grep ^GIT_PROTOCOL env.trace
'

# Test protocol v2 with 'http://' transport
#
. "$TEST_DIRECTORY"/lib-httpd.sh
Expand Down
10 changes: 7 additions & 3 deletions transport.c
Original file line number Diff line number Diff line change
Expand Up @@ -279,8 +279,12 @@ static int connect_setup(struct transport *transport, int for_push)
}

data->conn = git_connect(data->fd, transport->url,
for_push ? data->options.receivepack :
data->options.uploadpack,
for_push ?
"git-receive-pack" :
"git-upload-pack",
for_push ?
data->options.receivepack :
data->options.uploadpack,
flags);

return 0;
Expand Down Expand Up @@ -914,7 +918,7 @@ static int connect_git(struct transport *transport, const char *name,
{
struct git_transport_data *data = transport->data;
data->conn = git_connect(data->fd, transport->url,
executable, 0);
name, executable, 0);
fd[0] = data->fd[0];
fd[1] = data->fd[1];
return 0;
Expand Down

0 comments on commit f879501

Please sign in to comment.