{"thread":{"id":"19889","subject":"[PATCH js/daemon-log] receive-pack: do not send error details to the client","startedAt":"2009-06-21T21:16:09Z","lastAt":"2009-06-21T21:16:09Z","messageCount":1,"participants":["Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"116721","messageId":"200906212316.09149.j6t@kdbg.org","threadId":"19889","inReplyTo":null,"subject":"[PATCH js/daemon-log] receive-pack: do not send error details to the client","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-06-21T21:16:09Z","receivedAt":"2009-06-21T21:16:09Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"If the objects that a client pushes to the server cannot be processed for\nany reason, an error is reported back to the client via the git protocol.\nWe used to send quite detailed information if a system call failed if\nunpack-objects is run. This can be regarded as an information leak. Now we\ndo not send any error details like we already do in the case where\nindex-pack failed.\n\nErrors in system calls as well as the exit code of unpack-objects and\nindex-pack are now reported to stderr; in the case of a local push or via\nssh these messages still go to the client, but that is OK since these forms\nof access to the server assume that the client can be trusted. If\nreceive-pack is run from git-daemon, then the daemon should put the error\nmessages into the syslog.\n\nWith this reasoning a new status report is added for the post-update-hook;\nuntrusted (i.e. daemon's) clients cannot observe its status anyway, others\nmay want to know failure details.\n\nSigned-off-by: Johannes Sixt <j6t@kdbg.org>\n---\n Does this make sense?\n\n The motivation of this change is to ultimately get rid of error codes\n for system call failures in start/finish/run_command functions and call\n error() in these functions.\n\n -- Hannes\n\n builtin-receive-pack.c |   53 +++++++++++++++++++----------------------------\n 1 files changed, 22 insertions(+), 31 deletions(-)\n\ndiff --git a/builtin-receive-pack.c b/builtin-receive-pack.c\nindex 33d345d..6ec1d05 100644\n--- a/builtin-receive-pack.c\n+++ b/builtin-receive-pack.c\n@@ -123,27 +123,27 @@ static struct command *commands;\n static const char pre_receive_hook[] = \"hooks/pre-receive\";\n static const char post_receive_hook[] = \"hooks/post-receive\";\n \n-static int hook_status(int code, const char *hook_name)\n+static int run_status(int code, const char *cmd_name)\n {\n \tswitch (code) {\n \tcase 0:\n \t\treturn 0;\n \tcase -ERR_RUN_COMMAND_FORK:\n-\t\treturn error(\"hook fork failed\");\n+\t\treturn error(\"fork of %s failed\", cmd_name);\n \tcase -ERR_RUN_COMMAND_EXEC:\n-\t\treturn error(\"hook execute failed\");\n+\t\treturn error(\"execute of %s failed\", cmd_name);\n \tcase -ERR_RUN_COMMAND_PIPE:\n-\t\treturn error(\"hook pipe failed\");\n+\t\treturn error(\"pipe failed\");\n \tcase -ERR_RUN_COMMAND_WAITPID:\n \t\treturn error(\"waitpid failed\");\n \tcase -ERR_RUN_COMMAND_WAITPID_WRONG_PID:\n \t\treturn error(\"waitpid is confused\");\n \tcase -ERR_RUN_COMMAND_WAITPID_SIGNAL:\n-\t\treturn error(\"%s died of signal\", hook_name);\n+\t\treturn error(\"%s died of signal\", cmd_name);\n \tcase -ERR_RUN_COMMAND_WAITPID_NOEXIT:\n-\t\treturn error(\"%s died strangely\", hook_name);\n+\t\treturn error(\"%s died strangely\", cmd_name);\n \tdefault:\n-\t\terror(\"%s exited with error code %d\", hook_name, -code);\n+\t\terror(\"%s exited with error code %d\", cmd_name, -code);\n \t\treturn -code;\n \t}\n }\n@@ -174,7 +174,7 @@ static int run_receive_hook(const char *hook_name)\n \n \tcode = start_command(&proc);\n \tif (code)\n-\t\treturn hook_status(code, hook_name);\n+\t\treturn run_status(code, hook_name);\n \tfor (cmd = commands; cmd; cmd = cmd->next) {\n \t\tif (!cmd->error_string) {\n \t\t\tsize_t n = snprintf(buf, sizeof(buf), \"%s %s %s\\n\",\n@@ -186,7 +186,7 @@ static int run_receive_hook(const char *hook_name)\n \t\t}\n \t}\n \tclose(proc.in);\n-\treturn hook_status(finish_command(&proc), hook_name);\n+\treturn run_status(finish_command(&proc), hook_name);\n }\n \n static int run_update_hook(struct command *cmd)\n@@ -203,7 +203,7 @@ static int run_update_hook(struct command *cmd)\n \targv[3] = sha1_to_hex(cmd->new_sha1);\n \targv[4] = NULL;\n \n-\treturn hook_status(run_command_v_opt(argv, RUN_COMMAND_NO_STDIN |\n+\treturn run_status(run_command_v_opt(argv, RUN_COMMAND_NO_STDIN |\n \t\t\t\t\tRUN_COMMAND_STDOUT_TO_STDERR),\n \t\t\tupdate_hook);\n }\n@@ -394,7 +394,7 @@ static char update_post_hook[] = \"hooks/post-update\";\n static void run_update_post_hook(struct command *cmd)\n {\n \tstruct command *cmd_p;\n-\tint argc;\n+\tint argc, status;\n \tconst char **argv;\n \n \tfor (argc = 0, cmd_p = cmd; cmd_p; cmd_p = cmd_p->next) {\n@@ -417,8 +417,9 @@ static void run_update_post_hook(struct command *cmd)\n \t\targc++;\n \t}\n \targv[argc] = NULL;\n-\trun_command_v_opt(argv, RUN_COMMAND_NO_STDIN\n-\t\t| RUN_COMMAND_STDOUT_TO_STDERR);\n+\tstatus = run_command_v_opt(argv, RUN_COMMAND_NO_STDIN\n+\t\t\t| RUN_COMMAND_STDOUT_TO_STDERR);\n+\trun_status(status, update_post_hook);\n }\n \n static void execute_commands(const char *unpacker_error)\n@@ -534,24 +535,10 @@ static const char *unpack(void)\n \t\tunpacker[i++] = hdr_arg;\n \t\tunpacker[i++] = NULL;\n \t\tcode = run_command_v_opt(unpacker, RUN_GIT_CMD);\n-\t\tswitch (code) {\n-\t\tcase 0:\n+\t\tif (!code)\n \t\t\treturn NULL;\n-\t\tcase -ERR_RUN_COMMAND_FORK:\n-\t\t\treturn \"unpack fork failed\";\n-\t\tcase -ERR_RUN_COMMAND_EXEC:\n-\t\t\treturn \"unpack execute failed\";\n-\t\tcase -ERR_RUN_COMMAND_WAITPID:\n-\t\t\treturn \"waitpid failed\";\n-\t\tcase -ERR_RUN_COMMAND_WAITPID_WRONG_PID:\n-\t\t\treturn \"waitpid is confused\";\n-\t\tcase -ERR_RUN_COMMAND_WAITPID_SIGNAL:\n-\t\t\treturn \"unpacker died of signal\";\n-\t\tcase -ERR_RUN_COMMAND_WAITPID_NOEXIT:\n-\t\t\treturn \"unpacker died strangely\";\n-\t\tdefault:\n-\t\t\treturn \"unpacker exited with error code\";\n-\t\t}\n+\t\trun_status(code, unpacker[0]);\n+\t\treturn \"unpack-objects abnormal exit\";\n \t} else {\n \t\tconst char *keeper[7];\n \t\tint s, status, i = 0;\n@@ -574,8 +561,11 @@ static const char *unpack(void)\n \t\tip.argv = keeper;\n \t\tip.out = -1;\n \t\tip.git_cmd = 1;\n-\t\tif (start_command(&ip))\n+\t\tstatus = start_command(&ip);\n+\t\tif (status) {\n+\t\t\trun_status(status, keeper[0]);\n \t\t\treturn \"index-pack fork failed\";\n+\t\t}\n \t\tpack_lockfile = index_pack_lockfile(ip.out);\n \t\tclose(ip.out);\n \t\tstatus = finish_command(&ip);\n@@ -583,6 +573,7 @@ static const char *unpack(void)\n \t\t\treprepare_packed_git();\n \t\t\treturn NULL;\n \t\t}\n+\t\trun_status(status, keeper[0]);\n \t\treturn \"index-pack abnormal exit\";\n \t}\n }\n-- \n1.6.3.17.g1665f\n"}]}