https://github.com/git/git
Revision a5bb10fd5e74101e7c07da93e7c32bbe60f6173a authored by Taylor Blau on 06 April 2023, 18:07:58 UTC, committed by Johannes Schindelin on 17 April 2023, 19:15:40 UTC
When renaming (or deleting) a section of configuration, Git uses the
function `git_config_copy_or_rename_section_in_file()` to rewrite the
configuration file after applying the rename or deletion to the given
section.
To do this, Git repeatedly calls `fgets()` to read the existing
configuration data into a fixed size buffer.
When the configuration value under `old_name` exceeds the size of the
buffer, we will call `fgets()` an additional time even if there is no
newline in the configuration file, since our read length is capped at
`sizeof(buf)`.
If the first character of the buffer (after zero or more characters
satisfying `isspace()`) is a '[', Git will incorrectly treat it as
beginning a new section when the original section is being removed. In
other words, a configuration value satisfying this criteria can
incorrectly be considered as a new secftion instead of a variable in the
original section.
Avoid this issue by using a variable-width buffer in the form of a
strbuf rather than a fixed-with region on the stack. A couple of small
points worth noting:
- Using a strbuf will cause us to allocate arbitrary sizes to match
the length of each line. In practice, we don't expect any
reasonable configuration files to have lines that long, and a
bandaid will be introduced in a later patch to ensure that this is
the case.
- We are using strbuf_getwholeline() here instead of strbuf_getline()
in order to match `fgets()`'s behavior of leaving the trailing LF
character on the buffer (as well as a trailing NUL).
This could be changed later, but using strbuf_getwholeline() changes
the least about this function's implementation, so it is picked as
the safest path.
- It is temping to want to replace the loop to skip over characters
matching isspace() at the beginning of the buffer with a convenience
function like `strbuf_ltrim()`. But this is the wrong approach for a
couple of reasons:
First, it involves a potentially large and expensive `memmove()`
which we would like to avoid. Second, and more importantly, we also
*do* want to preserve those spaces to avoid changing the output of
other sections.
In all, this patch is a minimal replacement of the fixed-width buffer in
`git_config_copy_or_rename_section_in_file()` to instead use a `struct
strbuf`.
Reported-by: André Baptista <andre@ethiack.com>
Reported-by: Vítor Pinho <vitor@ethiack.com>
Helped-by: Patrick Steinhardt <ps@pks.im>
Co-authored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
1 parent 2919821
Tip revision: a5bb10fd5e74101e7c07da93e7c32bbe60f6173a authored by Taylor Blau on 06 April 2023, 18:07:58 UTC
config: avoid fixed-sized buffer when renaming/deleting a section
config: avoid fixed-sized buffer when renaming/deleting a section
Tip revision: a5bb10f
editor.c
#include "cache.h"
#include "config.h"
#include "strbuf.h"
#include "run-command.h"
#include "sigchain.h"
#ifndef DEFAULT_EDITOR
#define DEFAULT_EDITOR "vi"
#endif
int is_terminal_dumb(void)
{
const char *terminal = getenv("TERM");
return !terminal || !strcmp(terminal, "dumb");
}
const char *git_editor(void)
{
const char *editor = getenv("GIT_EDITOR");
int terminal_is_dumb = is_terminal_dumb();
if (!editor && editor_program)
editor = editor_program;
if (!editor && !terminal_is_dumb)
editor = getenv("VISUAL");
if (!editor)
editor = getenv("EDITOR");
if (!editor && terminal_is_dumb)
return NULL;
if (!editor)
editor = DEFAULT_EDITOR;
return editor;
}
const char *git_sequence_editor(void)
{
const char *editor = getenv("GIT_SEQUENCE_EDITOR");
if (!editor)
git_config_get_string_tmp("sequence.editor", &editor);
if (!editor)
editor = git_editor();
return editor;
}
static int launch_specified_editor(const char *editor, const char *path,
struct strbuf *buffer, const char *const *env)
{
if (!editor)
return error("Terminal is dumb, but EDITOR unset");
if (strcmp(editor, ":")) {
struct strbuf realpath = STRBUF_INIT;
const char *args[] = { editor, NULL, NULL };
struct child_process p = CHILD_PROCESS_INIT;
int ret, sig;
int print_waiting_for_editor = advice_waiting_for_editor && isatty(2);
if (print_waiting_for_editor) {
/*
* A dumb terminal cannot erase the line later on. Add a
* newline to separate the hint from subsequent output.
*
* Make sure that our message is separated with a whitespace
* from further cruft that may be written by the editor.
*/
const char term = is_terminal_dumb() ? '\n' : ' ';
fprintf(stderr,
_("hint: Waiting for your editor to close the file...%c"),
term);
fflush(stderr);
}
strbuf_realpath(&realpath, path, 1);
args[1] = realpath.buf;
p.argv = args;
p.env = env;
p.use_shell = 1;
p.trace2_child_class = "editor";
if (start_command(&p) < 0) {
strbuf_release(&realpath);
return error("unable to start editor '%s'", editor);
}
sigchain_push(SIGINT, SIG_IGN);
sigchain_push(SIGQUIT, SIG_IGN);
ret = finish_command(&p);
strbuf_release(&realpath);
sig = ret - 128;
sigchain_pop(SIGINT);
sigchain_pop(SIGQUIT);
if (sig == SIGINT || sig == SIGQUIT)
raise(sig);
if (ret)
return error("There was a problem with the editor '%s'.",
editor);
if (print_waiting_for_editor && !is_terminal_dumb())
/*
* Erase the entire line to avoid wasting the
* vertical space.
*/
term_clear_line();
}
if (!buffer)
return 0;
if (strbuf_read_file(buffer, path, 0) < 0)
return error_errno("could not read file '%s'", path);
return 0;
}
int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
{
return launch_specified_editor(git_editor(), path, buffer, env);
}
int launch_sequence_editor(const char *path, struct strbuf *buffer,
const char *const *env)
{
return launch_specified_editor(git_sequence_editor(), path, buffer, env);
}
Computing file changes ...
