diff --git a/src/builtin_complete.cpp b/src/builtin_complete.cpp index 934277477..de191d4d4 100644 --- a/src/builtin_complete.cpp +++ b/src/builtin_complete.cpp @@ -359,14 +359,8 @@ maybe_t builtin_complete(parser_t &parser, io_streams_t &streams, wchar_t * parser.libdata().transient_commandlines.push_back(do_complete_param); cleanup_t remove_transient([&] { parser.libdata().transient_commandlines.pop_back(); }); - if (parser.libdata().builtin_complete_current_commandline) { - // Prevent accidental recursion (see #6171). - } else if (parser.libdata().builtin_complete_recursion_level >= 24) { - // Allow a limited number of recursive calls to complete (#3474). - streams.err.append_format(L"%ls: maximum recursion depth reached\n", cmd); - } else { - parser.libdata().builtin_complete_recursion_level++; - assert(!parser.libdata().builtin_complete_current_commandline); + // Prevent accidental recursion (see #6171). + if (!parser.libdata().builtin_complete_current_commandline) { if (!have_do_complete_param) parser.libdata().builtin_complete_current_commandline = true; @@ -403,7 +397,6 @@ maybe_t builtin_complete(parser_t &parser, io_streams_t &streams, wchar_t * streams.out.push_back(L'\n'); } - parser.libdata().builtin_complete_recursion_level--; parser.libdata().builtin_complete_current_commandline = false; } } else if (path.empty() && gnu_opt.empty() diff --git a/src/complete.cpp b/src/complete.cpp index 8b10f1df7..a7327c97c 100644 --- a/src/complete.cpp +++ b/src/complete.cpp @@ -1379,13 +1379,7 @@ void completer_t::complete_custom(const wcstring &cmd, const wcstring &cmdline, ctx.parser->libdata().transient_commandlines.push_back(unaliased_cmd); cleanup_t remove_transient( [&] { ctx.parser->libdata().transient_commandlines.pop_back(); }); - // Prevent infinite recursion when the completion for x wraps "A=B x" (#7344). - // Don't report an error since this could be a legitimate alias. - static uint32_t complete_assignment_recursion_count; - if (complete_assignment_recursion_count++ < 24) { - run_on_command(std::move(unaliased_cmd)); - } - complete_assignment_recursion_count--; + perform_for_command(std::move(unaliased_cmd)); *do_file = false; } else if (!complete_param(cmd, previous_argument, current_argument, !had_ddash)) { // Invoke any custom completions for this command. @@ -1511,6 +1505,21 @@ void completer_t::mark_completions_duplicating_arguments(const wcstring &cmd, } void completer_t::perform_for_command(wcstring cmd) { + // Limit recursion, in case a user-defined completion has cycles, or the completion for "x" + // wraps "A=B x" (#3474, #7344). No need to do that when there is no parser: this happens only + // for autosuggestions where we don't evaluate command substitutions or variable assignments. + if (ctx.parser) { + if (ctx.parser->libdata().complete_recursion_level >= 24) { + debug(0, _(L"completion reached maximum recursion depth, possible cycle?"), + cmd.c_str()); + return; + } + ++ctx.parser->libdata().complete_recursion_level; + }; + cleanup_t decrement{[this]() { + if (ctx.parser) --ctx.parser->libdata().complete_recursion_level; + }}; + const size_t cursor_pos = cmd.size(); // Find the plain statement to operate on. The cursor may be past it (#1261), so backtrack diff --git a/src/parser.h b/src/parser.h index 7c8b37b01..aec1e934c 100644 --- a/src/parser.h +++ b/src/parser.h @@ -148,8 +148,8 @@ struct library_data_t { /// Last reader run count. uint64_t last_exec_run_counter{UINT64_MAX}; - /// Number of recursive calls to builtin_complete(). - uint32_t builtin_complete_recursion_level{0}; + /// Number of recursive calls to the internal completion function. + uint32_t complete_recursion_level{0}; /// If we're currently repainting the commandline. /// Useful to stop infinite loops. diff --git a/tests/checks/complete.fish b/tests/checks/complete.fish index e463490ed..7f5583a34 100644 --- a/tests/checks/complete.fish +++ b/tests/checks/complete.fish @@ -65,7 +65,7 @@ complete # Recursive calls to complete (see #3474) complete -c complete_test_recurse1 -xa '(echo recursing 1>&2; complete -C"complete_test_recurse1 ")' -complete -C'complete_test_recurse1 ' +LANG=C complete -C'complete_test_recurse1 ' # CHECKERR: recursing # CHECKERR: recursing # CHECKERR: recursing @@ -90,7 +90,7 @@ complete -C'complete_test_recurse1 ' # CHECKERR: recursing # CHECKERR: recursing # CHECKERR: recursing -# CHECKERR: complete: maximum recursion depth reached +# CHECKERR: fish: completion reached maximum recursion depth, possible cycle? # short options complete -c foo -f -a non-option-argument diff --git a/tests/checks/wraps.fish b/tests/checks/wraps.fish index cd85051b3..98c014ba5 100644 --- a/tests/checks/wraps.fish +++ b/tests/checks/wraps.fish @@ -1,6 +1,8 @@ #RUN: %fish %s # Validate some things about command wrapping. +set -g LANG C # For predictable error messages. + # This tests that we do not trigger a combinatorial explosion - see #5638. # Ensure it completes successfully. complete -c testcommand --wraps "testcommand x " @@ -30,3 +32,4 @@ complete -C'testcommand2 explicit ' complete -c recvar --wraps 'A=B recvar' complete -C 'recvar ' +# CHECKERR: fish: completion reached maximum recursion depth, possible cycle?