All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/3] Redirect update hook stdout to stderr.
       [not found] <a0aecffe21074288c911c396f92901bfb558d591.1167533707.git.spearce@spearce.org>
@ 2006-12-31  2:55 ` Shawn O. Pearce
  2006-12-31  2:55 ` [PATCH 3/3] Use /dev/null for update hook stdin Shawn O. Pearce
  1 sibling, 0 replies; 3+ messages in thread
From: Shawn O. Pearce @ 2006-12-31  2:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

If an update hook outputs to stdout then that output will be sent
back over the wire to the push client as though it were part of
the git protocol.  This tends to cause protocol errors on the
client end of the connection, as the hook output is not expected
in that context.  Most hook developers work around this by making
sure their hook outputs everything to stderr.

But hooks shouldn't need to perform such special behavior.  Instead
we can just dup stderr to stdout prior to invoking the update hook.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 receive-pack.c |    3 ++-
 run-command.c  |   32 ++++++++++++++++++++++++++------
 run-command.h  |    2 ++
 3 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/receive-pack.c b/receive-pack.c
index af05a61..64289e9 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -73,7 +73,8 @@ static int run_update_hook(const char *refname,
 
 	if (access(update_hook, X_OK) < 0)
 		return 0;
-	code = run_command(update_hook, refname, old_hex, new_hex, NULL);
+	code = run_command_opt(RUN_COMMAND_STDOUT_TO_STDERR,
+		update_hook, refname, old_hex, new_hex, NULL);
 	switch (code) {
 	case 0:
 		return 0;
diff --git a/run-command.c b/run-command.c
index d0330c3..7e4ca43 100644
--- a/run-command.c
+++ b/run-command.c
@@ -14,7 +14,8 @@ int run_command_v_opt(const char **argv, int flags)
 			dup2(fd, 0);
 			dup2(fd, 1);
 			close(fd);
-		}
+		} else if (flags & RUN_COMMAND_STDOUT_TO_STDERR)
+			dup2(2, 1);
 		if (flags & RUN_GIT_CMD) {
 			execv_git_cmd(argv);
 		} else {
@@ -51,14 +52,12 @@ int run_command_v(const char **argv)
 	return run_command_v_opt(argv, 0);
 }
 
-int run_command(const char *cmd, ...)
+static int run_command_va_opt(int opt, const char *cmd, va_list param)
 {
 	int argc;
 	const char *argv[MAX_RUN_COMMAND_ARGS];
 	const char *arg;
-	va_list param;
 
-	va_start(param, cmd);
 	argv[0] = (char*) cmd;
 	argc = 1;
 	while (argc < MAX_RUN_COMMAND_ARGS) {
@@ -66,8 +65,29 @@ int run_command(const char *cmd, ...)
 		if (!arg)
 			break;
 	}
-	va_end(param);
 	if (MAX_RUN_COMMAND_ARGS <= argc)
 		return error("too many args to run %s", cmd);
-	return run_command_v_opt(argv, 0);
+	return run_command_v_opt(argv, opt);
+}
+
+int run_command_opt(int opt, const char *cmd, ...)
+{
+	va_list params;
+	int r;
+
+	va_start(params, cmd);
+	r = run_command_va_opt(opt, cmd, params);
+	va_end(params);
+	return r;
+}
+
+int run_command(const char *cmd, ...)
+{
+	va_list params;
+	int r;
+
+	va_start(params, cmd);
+	r = run_command_va_opt(0, cmd, params);
+	va_end(params);
+	return r;
 }
diff --git a/run-command.h b/run-command.h
index 82a0861..8156eac 100644
--- a/run-command.h
+++ b/run-command.h
@@ -13,8 +13,10 @@ enum {
 
 #define RUN_COMMAND_NO_STDIO 1
 #define RUN_GIT_CMD	     2	/*If this is to be git sub-command */
+#define RUN_COMMAND_STDOUT_TO_STDERR 4
 int run_command_v_opt(const char **argv, int opt);
 int run_command_v(const char **argv);
+int run_command_opt(int opt, const char *cmd, ...);
 int run_command(const char *cmd, ...);
 
 #endif
-- 
1.5.0.rc0.g6bb1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH 3/3] Use /dev/null for update hook stdin.
       [not found] <a0aecffe21074288c911c396f92901bfb558d591.1167533707.git.spearce@spearce.org>
  2006-12-31  2:55 ` [PATCH 2/3] Redirect update hook stdout to stderr Shawn O. Pearce
@ 2006-12-31  2:55 ` Shawn O. Pearce
  2006-12-31  6:03   ` Junio C Hamano
  1 sibling, 1 reply; 3+ messages in thread
From: Shawn O. Pearce @ 2006-12-31  2:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Currently the update hook invoked by receive-pack has its stdin
connected to the pushing client.  The hook shouldn't attempt to
read from this stream, and doing so may consume data that was
meant for receive-pack.  Instead we should give the update hook
/dev/null as its stdin, ensuring that it always receives EOF and
doesn't disrupt the protocol if it attempts to read any data.

The post-update hook is similar, as it gets invoked with /dev/null
on stdin to prevent the hook from reading data from the client.
Previously we had invoked it with stdout also connected to /dev/null,
throwing away anything on stdout, to prevent client protocol errors.
Instead we should redirect stdout to stderr, like we do with the
update hook.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 receive-pack.c |    6 ++++--
 run-command.c  |    6 +++---
 run-command.h  |    2 +-
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/receive-pack.c b/receive-pack.c
index 64289e9..cf83109 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -73,7 +73,8 @@ static int run_update_hook(const char *refname,
 
 	if (access(update_hook, X_OK) < 0)
 		return 0;
-	code = run_command_opt(RUN_COMMAND_STDOUT_TO_STDERR,
+	code = run_command_opt(RUN_COMMAND_NO_STDIN
+		| RUN_COMMAND_STDOUT_TO_STDERR,
 		update_hook, refname, old_hex, new_hex, NULL);
 	switch (code) {
 	case 0:
@@ -188,7 +189,8 @@ static void run_update_post_hook(struct command *cmd)
 		argc++;
 	}
 	argv[argc] = NULL;
-	run_command_v_opt(argv, RUN_COMMAND_NO_STDIO);
+	run_command_v_opt(argv, RUN_COMMAND_NO_STDIN
+		| RUN_COMMAND_STDOUT_TO_STDERR);
 }
 
 /*
diff --git a/run-command.c b/run-command.c
index 7e4ca43..cfbad74 100644
--- a/run-command.c
+++ b/run-command.c
@@ -9,12 +9,12 @@ int run_command_v_opt(const char **argv, int flags)
 	if (pid < 0)
 		return -ERR_RUN_COMMAND_FORK;
 	if (!pid) {
-		if (flags & RUN_COMMAND_NO_STDIO) {
+		if (flags & RUN_COMMAND_NO_STDIN) {
 			int fd = open("/dev/null", O_RDWR);
 			dup2(fd, 0);
-			dup2(fd, 1);
 			close(fd);
-		} else if (flags & RUN_COMMAND_STDOUT_TO_STDERR)
+		}
+		if (flags & RUN_COMMAND_STDOUT_TO_STDERR)
 			dup2(2, 1);
 		if (flags & RUN_GIT_CMD) {
 			execv_git_cmd(argv);
diff --git a/run-command.h b/run-command.h
index 8156eac..59c4476 100644
--- a/run-command.h
+++ b/run-command.h
@@ -11,7 +11,7 @@ enum {
 	ERR_RUN_COMMAND_WAITPID_NOEXIT,
 };
 
-#define RUN_COMMAND_NO_STDIO 1
+#define RUN_COMMAND_NO_STDIN 1
 #define RUN_GIT_CMD	     2	/*If this is to be git sub-command */
 #define RUN_COMMAND_STDOUT_TO_STDERR 4
 int run_command_v_opt(const char **argv, int opt);
-- 
1.5.0.rc0.g6bb1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 3/3] Use /dev/null for update hook stdin.
  2006-12-31  2:55 ` [PATCH 3/3] Use /dev/null for update hook stdin Shawn O. Pearce
@ 2006-12-31  6:03   ` Junio C Hamano
  0 siblings, 0 replies; 3+ messages in thread
From: Junio C Hamano @ 2006-12-31  6:03 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git

"Shawn O. Pearce" <spearce@spearce.org> writes:

> Currently the update hook invoked by receive-pack has its stdin
> connected to the pushing client.  The hook shouldn't attempt to
> read from this stream, and doing so may consume data that was
> meant for receive-pack.  Instead we should give the update hook
> /dev/null as its stdin, ensuring that it always receives EOF and
> doesn't disrupt the protocol if it attempts to read any data.

Both 2 and 3 look sane, but can we have a few new tests too?

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2006-12-31  6:03 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <a0aecffe21074288c911c396f92901bfb558d591.1167533707.git.spearce@spearce.org>
2006-12-31  2:55 ` [PATCH 2/3] Redirect update hook stdout to stderr Shawn O. Pearce
2006-12-31  2:55 ` [PATCH 3/3] Use /dev/null for update hook stdin Shawn O. Pearce
2006-12-31  6:03   ` Junio C Hamano

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.