Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix null dereferences on unset %(fileargs) #1159

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions src/argv.c
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ format_append_argv(struct format_context *format, const char ***dst_argv, const
int argc;

if (!src_argv)
return argv_append(dst_argv, "");
return true;

for (argc = 0; src_argv[argc]; argc++)
if (!format_append_arg(format, dst_argv, src_argv[argc]))
Expand Down Expand Up @@ -472,7 +472,17 @@ argv_format(struct argv_env *argv_env, const char ***dst_argv, const char *src_a
}
}

return src_argv[argc] == NULL;
if (src_argv[argc] != NULL)
return false;

// Make sure to return an argv-style array even if we have not formatted anything.
if (!*dst_argv) {
if (!argv_realloc(dst_argv, 1, 1))
return false;
(*dst_argv)[0] = NULL;
}

return true;
}

static inline bool
Expand Down
5 changes: 2 additions & 3 deletions src/prompt.c
Original file line number Diff line number Diff line change
Expand Up @@ -942,7 +942,7 @@ run_prompt_command(struct view *view, const char *argv[])
copied = argv_format(view->env, &next->argv, argv, false, true);
argv[0] = cmd;

if (!copied) {
if (!copied || !next->argv[0]) {
report("Argument formatting failed");
} else {
/* When running random commands, initially show the
Expand All @@ -966,7 +966,6 @@ run_prompt_command(struct view *view, const char *argv[])
char text[SIZEOF_STR] = "";

if (argv[1]
&& strlen(argv[1]) > 0
&& (!argv_format(view->env, &fmt_argv, &argv[1], false, true)
|| !argv_to_string(fmt_argv, text, sizeof(text), " ")
)) {
Expand Down Expand Up @@ -1103,7 +1102,7 @@ exec_run_request(struct view *view, struct run_request *req)
if (!argv_to_string(req->argv, cmd, sizeof(cmd), " ")
|| !argv_from_string_no_quotes(req_argv, &req_argc, cmd)
|| !argv_format(view->env, &argv, req_argv, false, true)
|| !argv) {
|| !argv[0]) {
report("Failed to format arguments");
return REQ_NONE;
}
Expand Down
45 changes: 39 additions & 6 deletions test/regressions/github-1136-test
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,54 @@ test_case bang-cmdlineargs-doesnt-crash \
--script='
:!%(cmdlineargs)
' <<EOF
On branch master
Changes to be committed:
(no files)
Changes not staged for commit:
(no files)
Untracked files:
(no files)

[status] Nothing to update 100%
Comment on lines +16 to +24
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not consistent with the comment "This runs an empty command, hence the empty pager".

EOF

test_case echo-cmdlineargs-doesnt-crash \
--args='status' \
--script='
:echo %(cmdlineargs)
' <<EOF
On branch master
Changes to be committed:
(no files)
Changes not staged for commit:
(no files)
Untracked files:
(no files)

[status] Nothing to update 100%
EOF

# This runs an empty command, hence the empty pager.
test_case bang-fileargs-doesnt-crash \
--args='status' \
--script='
:!%(fileargs)
' <<EOF
On branch master
Changes to be committed:
(no files)
Changes not staged for commit:
(no files)
Untracked files:
(no files)




[pager] 0%
[status] Nothing to update 100%
Comment on lines +49 to +57
Copy link
Collaborator

@koutcher koutcher Dec 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here. In both cases, I believe the comment is correct and we should expect an empty pager.

EOF

test_case echo-cmdlineargs-doesnt-crash \
test_case echo-fileargs-doesnt-crash \
--args='status' \
--script='
:echo %(cmdlineargs)
:echo %(fileargs)
' <<EOF
On branch master
Changes to be committed:
Expand Down