gc: use temporary file for editing crontab

While cron is specified by POSIX, there are a wide variety of implementations in use. "git maintenance" assumes that the "crontab" command can be fed from its standard input the new contents and the syntax to do so is not to have any filename argument, as POSIX describes. However, on FreeBSD, the cron implementation requires a file name argument: if the user wants to edit standard input, they must specify "-". Unfortunately, POSIX systems do not have to interpret "-" on the command line of crontab as a request to read from the standard input. Blindly adding "-" on the command line would not work as a general solution. Since POSIX tells us that cron must accept a file name argument, let's solve this problem by specifying a temporary file instead. This will ensure that we work with the vast majority of implementations. Note that because delete_tempfile closes the file for us, we should not call fclose here on the handle, since doing so will introduce a double free. Reported-by: Renato Botelho <garga@FreeBSD.org> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

brian m. carlson committed Aug 28, 2022 at 21:41 UTC ee69e7884e0cae3d2feabd5fcce8d3dfb44dda3a
2 files changed +25 -18
builtin/gc.c
+23 -16
@@ -2059,6 +2059,7 @@ static int crontab_update_schedule(int run_maintenance, int fd)
2059 struct child_process crontab_edit = CHILD_PROCESS_INIT;
2060 FILE *cron_list, *cron_in;
2061 struct strbuf line = STRBUF_INIT;
2062 + struct tempfile *tmpedit = NULL;
2063
2064 get_schedule_cmd(&cmd, NULL);
2065 strvec_split(&crontab_list.args, cmd);
@@ -2073,6 +2074,17 @@ static int crontab_update_schedule(int run_maintenance, int fd)
2074 /* Ignore exit code, as an empty crontab will return error. */
2075 finish_command(&crontab_list);
2076
2077 + tmpedit = mks_tempfile_t(".git_cron_edit_tmpXXXXXX");
2078 + if (!tmpedit) {
2079 + result = error(_("failed to create crontab temporary file"));
2080 + goto out;
2081 + }
2082 + cron_in = fdopen_tempfile(tmpedit, "w");
2083 + if (!cron_in) {
2084 + result = error(_("failed to open temporary file"));
2085 + goto out;
2086 + }
2087 +
2088 /*
2089 * Read from the .lock file, filtering out the old
2090 * schedule while appending the new schedule.
@@ -2080,19 +2092,6 @@ static int crontab_update_schedule(int run_maintenance, int fd)
2092 cron_list = fdopen(fd, "r");
2093 rewind(cron_list);
2094
2083 - strvec_split(&crontab_edit.args, cmd);
2084 - crontab_edit.in = -1;
2085 - crontab_edit.git_cmd = 0;
2086 -
2087 - if (start_command(&crontab_edit))
2088 - return error(_("failed to run 'crontab'; your system might not support 'cron'"));
2089 -
2090 - cron_in = fdopen(crontab_edit.in, "w");
2091 - if (!cron_in) {
2092 - result = error(_("failed to open stdin of 'crontab'"));
2093 - goto done_editing;
2094 - }
2095 -
2095 while (!strbuf_getline_lf(&line, cron_list)) {
2096 if (!in_old_region && !strcmp(line.buf, BEGIN_LINE))
2097 in_old_region = 1;
@@ -2126,14 +2125,22 @@ static int crontab_update_schedule(int run_maintenance, int fd)
2125 }
2126
2127 fflush(cron_in);
2129 - fclose(cron_in);
2130 - close(crontab_edit.in);
2128
2132 -done_editing:
2129 + strvec_split(&crontab_edit.args, cmd);
2130 + strvec_push(&crontab_edit.args, get_tempfile_path(tmpedit));
2131 + crontab_edit.git_cmd = 0;
2132 +
2133 + if (start_command(&crontab_edit)) {
2134 + result = error(_("failed to run 'crontab'; your system might not support 'cron'"));
2135 + goto out;
2136 + }
2137 +
2138 if (finish_command(&crontab_edit))
2139 result = error(_("'crontab' died"));
2140 else
2141 fclose(cron_list);
2142 +out:
2143 + delete_tempfile(&tmpedit);
2144 return result;
2145 }
2146
t/helper/test-crontab.c
+2 -2
@@ -17,8 +17,8 @@ int cmd__crontab(int argc, const char **argv)
17 if (!from)
18 return 0;
19 to = stdout;
20 - } else if (argc == 2) {
21 - from = stdin;
20 + } else if (argc == 3) {
21 + from = fopen(argv[2], "r");
22 to = fopen(argv[1], "w");
23 } else
24 return error("unknown arguments");