[PATCH v2] receive-pack: add option to interrupt pre-receive when client exits
- From
- Robin Jarry <robin.jarry@6wind.com>
- Date
- Jan 26, 2022, 21:44 UTC
- Message-ID
- <20220126214438.3066132-1-robin.jarry@6wind.com>
- In-Reply-To
- <20220125095445.1796938-1-robin.jarry@6wind.com>
When hitting ctrl-c on the client while a remote pre-receive hook is running, receive-pack is not killed by SIGPIPE because the signal is ignored. This is a side effect of commit ec7dbd145bd8 (receive-pack: allow hooks to ignore its standard input stream).
The pre-receive hook itself is not interrupted and does not receive any error since its stdout is a pipe which is read in an async thread and output back to the client socket in a side band channel.
After the pre-receive has exited the SIGPIPE default handler is restored and if the hook did not report any error, objects are migrated from temporary to permanent storage.
This can be confusing for most people and may even be considered a bug.
Add a new receive.strictPreReceiveImpl config option to *not* ignore SIGPIPE when running pre-receive. If set to true, and the hook output cannot be forwarded to the client, receive-pack will be killed via SIGPIPE and the push will be aborted. Add a signal handler to kill and reap the hook process before exiting. This option only affects pre-receive.
This does not guarantee that all client disconnections will abort a push. If there is no pre-receive hook or if it does not produce any output, receive-pack will not be killed via SIGPIPE and the push will complete.
Signed-off-by: Robin Jarry <robin.jarry@6wind.com> --- v1 -> v2: Changed approach following Jiang Xin advice. Adding an option makes more sense and also makes a much simpler patch.
Documentation/config/receive.txt | 13 +++++++++++++ builtin/receive-pack.c | 26 +++++++++++++++++++++++++- 2 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/Documentation/config/receive.txt b/Documentation/config/receive.txt index 85d5b5a3d2d8..7174168541dc 100644 --- a/Documentation/config/receive.txt +++ b/Documentation/config/receive.txt @@ -143,3 +143,16 @@ receive.updateServerInfo:: receive.shallowUpdate:: If set to true, .git/shallow can be updated when new refs require new shallow roots. Otherwise those refs are rejected. + +receive.strictPreReceiveImpl:: + If a pre-receive hook does not consume its standard input fully, it may + kill receive-pack via SIGPIPE. This can lead to obscure push failures. + To avoid potential death-by-SIGPIPE due to poorly written hooks, + receive-pack ignores SIGPIPE while running the pre-receive hook. ++ +If this option is set to true, SIGPIPE will `not` be ignored by receive-pack +while running the "pre-receive" hook. This has a side-effect: If the hook +outputs something and the client has disconnected, receive-pack will be killed +and the push will be aborted. ++ +SIGPIPE is always ignored while running "post-receive". diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 9f4a0b816cf9..8718a6dd91b4 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -74,6 +74,7 @@ static const char *head_name; static void *head_name_to_free; static int sent_capabilities; static int shallow_update; +static int strict_pre_receive_impl; static const char *alt_shallow_file; static struct strbuf push_cert = STRBUF_INIT; static struct object_id push_cert_oid; @@ -219,6 +220,11 @@ static int receive_pack_config(const char *var, const char *value, void *cb) return 0; } + if (strcmp(var, "receive.strictprereceiveimpl") == 0) { + strict_pre_receive_impl = git_config_bool(var, value); + return 0; + } + if (strcmp(var, "receive.certnonceseed") == 0) return git_config_string(&cert_nonce_seed, var, value); @@ -800,6 +806,19 @@ static void prepare_push_cert_sha1(struct child_process *proc) } } +static volatile pid_t hook_pid; + +static void kill_hook(int signum) +{ + if (hook_pid != 0) { + kill(hook_pid, signum); + waitpid(hook_pid, NULL, 0); + hook_pid = 0; + } + sigchain_pop(signum); + raise(signum); +} + struct receive_hook_feed_state { struct command *cmd; struct ref_push_report *report; @@ -858,7 +877,11 @@ static int run_and_feed_hook(const char *hook_name, feed_fn feed, return code; } - sigchain_push(SIGPIPE, SIG_IGN); + hook_pid = proc.pid; + if (strict_pre_receive_impl && strcmp(hook_name, "pre-receive") == 0) + sigchain_push(SIGPIPE, kill_hook); + else + sigchain_push(SIGPIPE, SIG_IGN); while (1) { const char *buf; @@ -872,6 +895,7 @@ static int run_and_feed_hook(const char *hook_name, feed_fn feed, if (use_sideband) finish_async(&muxer); + hook_pid = 0; sigchain_pop(SIGPIPE); return finish_command(&proc);
-- 2.35.0.1.g8273a50afc47