From bed00cd032271c14ba5675aebdee3286683a1ead Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Sun, 22 Dec 2024 23:32:23 +0100 Subject: [PATCH] smart: ignore shallow/unshallow packets during ACK processing In RPC mode (https), the client sends its list of shallow commits at the beginning of each request during packfile negotiation, so that the remote endpoint keeps context. This causes the remote to prepend appropriate shallow/unshallow packets to each response sent back to the client. However, the store_common() helper function (used in multi_ack mode) does not cater for this, returning as soon as it encounters any packet different than an ACK packet and therefore leaving the rest of the HTTP buffer unprocessed. This in turn causes subsequent iterations of the while loop processing ACK packets to process data returned by older HTTP requests instead of the current one, messing up the packfile negotiation process. Given that the wait_while_ack() helper function (called after the client signals to the remote that it is ready to receive packfile data) correctly skips over shallow/unshallow packets, packfile contents can still be received successfully in some cases (depending on message framing); in some other ones, though (particularly when git_smart__download_pack() processes an HTTP buffer starting with shallow/unshallow packets), the fetching process fails with an "incomplete pack header" error due to the flush packet terminating a set of shallow/unshallow packets being incorrectly interpreted as the flush packet indicating the end of the packfile (making the code behave as if no packfile data was sent by the remote). Fix by ignoring shallow/unshallow packets in the store_common() helper function, therefore making the ACK processing logic work on the correct HTTP buffers and ensuring that git_smart__download_pack() is not called until packfile negotiation is actually finished. --- src/libgit2/transports/smart_protocol.c | 7 ++++ tests/libgit2/online/shallow.c | 46 +++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/src/libgit2/transports/smart_protocol.c b/src/libgit2/transports/smart_protocol.c index b35b33b4347..87c1904589d 100644 --- a/src/libgit2/transports/smart_protocol.c +++ b/src/libgit2/transports/smart_protocol.c @@ -317,6 +317,13 @@ static int store_common(transport_smart *t) if ((error = recv_pkt(&pkt, NULL, t)) < 0) return error; + if (t->rpc && (pkt->type == GIT_PKT_SHALLOW || + pkt->type == GIT_PKT_UNSHALLOW || + pkt->type == GIT_PKT_FLUSH)) { + git__free(pkt); + continue; + } + if (pkt->type != GIT_PKT_ACK) { git__free(pkt); return 0; diff --git a/tests/libgit2/online/shallow.c b/tests/libgit2/online/shallow.c index d7038cdd2dc..a5508c16d45 100644 --- a/tests/libgit2/online/shallow.c +++ b/tests/libgit2/online/shallow.c @@ -165,6 +165,52 @@ void test_online_shallow__unshallow(void) git_repository_free(repo); } +void test_online_shallow__deepen_full(void) +{ + git_str path = GIT_STR_INIT; + git_repository *repo; + git_revwalk *walk; + git_clone_options clone_opts = GIT_CLONE_OPTIONS_INIT; + git_fetch_options fetch_opts = GIT_FETCH_OPTIONS_INIT; + git_remote *origin = NULL; + git_oid oid; + git_oid *roots; + size_t roots_len; + size_t num_commits = 0; + int error = 0; + + clone_opts.fetch_opts.depth = 7; + clone_opts.remote_cb = remote_single_branch; + + git_str_joinpath(&path, clar_sandbox_path(), "deepen_full"); + cl_git_pass(git_clone(&repo, "https://github.com/libgit2/TestGitRepository", git_str_cstr(&path), &clone_opts)); + cl_assert_equal_b(true, git_repository_is_shallow(repo)); + + fetch_opts.depth = 8; + cl_git_pass(git_remote_lookup(&origin, repo, "origin")); + cl_git_pass(git_remote_fetch(origin, NULL, &fetch_opts, NULL)); + cl_assert_equal_b(false, git_repository_is_shallow(repo)); + + cl_git_pass(git_repository__shallow_roots(&roots, &roots_len, repo)); + cl_assert_equal_i(0, roots_len); + + git_revwalk_new(&walk, repo); + git_revwalk_push_head(walk); + + while ((error = git_revwalk_next(&oid, walk)) == GIT_OK) { + num_commits++; + } + + cl_assert_equal_i(num_commits, 21); + cl_assert_equal_i(error, GIT_ITEROVER); + + git__free(roots); + git_remote_free(origin); + git_str_dispose(&path); + git_revwalk_free(walk); + git_repository_free(repo); +} + void test_online_shallow__deepen_six(void) { git_str path = GIT_STR_INIT;