Re: [PATCH v3 2/2] wrapper: use a CSPRNG to generate random file names
- From
René Scharfe <l.s.r@web.de>
- Date
- Jan 19, 2022, 17:49 UTC
- Message-ID
- <53416697-2047-67c1-145b-895f147c91da@web.de>
- In-Reply-To
- <xmqq7daxt0pk.fsf@gitster.g>
Am 18.01.22 um 18:25 schrieb Junio C Hamano:
Show 5 quoted lines
> I agree with your analysis that the "diff" tempfiles do need suffix, > we SHOULD create them in $TMPDIR (not in the working tree or > $GIT_DIR) to support operation in a read-only repository, but we can > create a unique temporary directory and place a file (even under its > original name) in it as a workaround.
Here's a proof-of-concept patch for handling just that diff use-case using mkdtemp. Indeed it's nice to see the actual filename with difftool.
diff.c | 4 ++-- t/t4020-diff-external.sh | 2 +- tempfile.c | 20 ++++++++++++++++++++ tempfile.h | 1 + wrapper.c | 12 ++++++++++++ 5 files changed, 36 insertions(+), 3 deletions(-)
diff --git a/diff.c b/diff.c index c862771a58..37db4743b0 100644 --- a/diff.c +++ b/diff.c @@ -4098,8 +4098,8 @@ static void prep_temp_blob(struct index_state *istate, init_checkout_metadata(&meta, NULL, NULL, oid); - /* Generate "XXXXXX_basename.ext" */ - strbuf_addstr(&tempfile, "XXXXXX_"); + /* Generate "git-blob-XXXXXX/basename.ext" */ + strbuf_addstr(&tempfile, "git-blob-XXXXXX/"); strbuf_addstr(&tempfile, base); temp->tempfile = mks_tempfile_ts(tempfile.buf, strlen(base) + 1); diff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh index 54bb8ef27e..e7f93f36f5 100755 --- a/t/t4020-diff-external.sh +++ b/t/t4020-diff-external.sh @@ -217,7 +217,7 @@ test_expect_success 'GIT_EXTERNAL_DIFF generates pretty paths' ' touch file.ext && git add file.ext && echo with extension > file.ext && - GIT_EXTERNAL_DIFF=echo git diff file.ext | grep ......_file\.ext && + GIT_EXTERNAL_DIFF=echo git diff file.ext | grep git-blob-....../file\.ext && git update-index --force-remove file.ext && rm file.ext ' diff --git a/tempfile.c b/tempfile.c index 94aa18f3f7..80cc9fb512 100644 --- a/tempfile.c +++ b/tempfile.c @@ -56,6 +56,21 @@ static VOLATILE_LIST_HEAD(tempfile_list); +static void remove_template_directory(struct tempfile *tempfile, + int in_signal_handler) +{ + char *end = tempfile->filename.buf + tempfile->filename.len; + char *suffix = end - tempfile->suffixlen; + if (*suffix != '/') + return; + *suffix = '\0'; + if (in_signal_handler) + rmdir(tempfile->filename.buf); + else + rmdir_or_warn(tempfile->filename.buf); + *suffix = '/'; +} + static void remove_tempfiles(int in_signal_handler) { pid_t me = getpid(); @@ -74,6 +89,7 @@ static void remove_tempfiles(int in_signal_handler) unlink(p->filename.buf); else unlink_or_warn(p->filename.buf); + remove_template_directory(p, in_signal_handler); p->active = 0; } @@ -100,6 +116,7 @@ static struct tempfile *new_tempfile(void) tempfile->owner = 0; INIT_LIST_HEAD(&tempfile->list); strbuf_init(&tempfile->filename, 0); + tempfile->suffixlen = 0; return tempfile; } @@ -170,6 +187,7 @@ struct tempfile *mks_tempfile_sm(const char *filename_template, int suffixlen, i struct tempfile *tempfile = new_tempfile(); strbuf_add_absolute_path(&tempfile->filename, filename_template); + tempfile->suffixlen = suffixlen; tempfile->fd = git_mkstemps_mode(tempfile->filename.buf, suffixlen, mode); if (tempfile->fd < 0) { deactivate_tempfile(tempfile); @@ -189,6 +207,7 @@ struct tempfile *mks_tempfile_tsm(const char *filename_template, int suffixlen, tmpdir = "/tmp"; strbuf_addf(&tempfile->filename, "%s/%s", tmpdir, filename_template); + tempfile->suffixlen = suffixlen; tempfile->fd = git_mkstemps_mode(tempfile->filename.buf, suffixlen, mode); if (tempfile->fd < 0) { deactivate_tempfile(tempfile); @@ -316,6 +335,7 @@ void delete_tempfile(struct tempfile **tempfile_p) close_tempfile_gently(tempfile); unlink_or_warn(tempfile->filename.buf); + remove_template_directory(tempfile, 0); deactivate_tempfile(tempfile); *tempfile_p = NULL; } diff --git a/tempfile.h b/tempfile.h index 4de3bc77d2..b9a60f2431 100644 --- a/tempfile.h +++ b/tempfile.h @@ -82,6 +82,7 @@ struct tempfile { FILE *volatile fp; volatile pid_t owner; struct strbuf filename; + size_t suffixlen; }; /* diff --git a/wrapper.c b/wrapper.c index 36e12119d7..358db282f9 100644 --- a/wrapper.c +++ b/wrapper.c @@ -481,6 +481,18 @@ int git_mkstemps_mode(char *pattern, int suffix_len, int mode) return -1; } + if (pattern[len - suffix_len] == '/') { + if (mode != 0600) { + errno = EINVAL; + return -1; + } + pattern[len - suffix_len] = '\0'; + if (!mkdtemp(pattern)) + return -1; + pattern[len - suffix_len] = '/'; + return open(pattern, O_CREAT | O_EXCL | O_RDWR, mode); + } + /* * Replace pattern's XXXXXX characters with randomness. * Try TMP_MAX different filenames.