diff --git a/doc_src/index.rst b/doc_src/index.rst index 8f300149e..bc1d457d0 100644 --- a/doc_src/index.rst +++ b/doc_src/index.rst @@ -269,8 +269,6 @@ Any arbitrary file descriptor can used in a redirection by prefixing the redirec For example, ``echo hello 2> output.stderr`` writes the standard error (file descriptor 2) to ``output.stderr``. -It is an error to redirect a builtin, function, or block to a file descriptor above 2. However this is supported for external commands. - .. [#] Previous versions of fish also allowed specifying this as ``^DESTINATION``, but that made another character special so it was deprecated and will be removed in the future. See :ref:`feature flags`. .. _pipes: diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index 225edb494..09638b7a5 100644 --- a/src/parse_execution.cpp +++ b/src/parse_execution.cpp @@ -900,19 +900,14 @@ end_execution_reason_t parse_execution_context_t::populate_plain_process( return arg_result; } - // Determine the process type. - process_type = process_type_for_command(statement, cmd); - - // Only external commands and exec may redirect arbitrary fds. - bool allow_high_fds = - (process_type == process_type_t::external || process_type == process_type_t::exec); - // The set of IO redirections that we construct for the process. - auto reason = - this->determine_redirections(statement.args_or_redirs, allow_high_fds, &redirections); + auto reason = this->determine_redirections(statement.args_or_redirs, &redirections); if (reason != end_execution_reason_t::ok) { return reason; } + + // Determine the process type. + process_type = process_type_for_command(statement, cmd); } // Populate the process. @@ -985,8 +980,7 @@ end_execution_reason_t parse_execution_context_t::expand_arguments_from_nodes( } end_execution_reason_t parse_execution_context_t::determine_redirections( - const ast::argument_or_redirection_list_t &list, bool allow_high_fds, - redirection_spec_list_t *out_redirections) { + const ast::argument_or_redirection_list_t &list, redirection_spec_list_t *out_redirections) { // Get all redirection nodes underneath the statement. for (const ast::argument_or_redirection_t &arg_or_redir : list) { if (!arg_or_redir.is_redirection()) continue; @@ -1014,18 +1008,10 @@ end_execution_reason_t parse_execution_context_t::determine_redirections( redirection_spec_t spec{oper->fd, oper->mode, std::move(target)}; // Validate this spec. - if (spec.mode == redirection_mode_t::fd && !spec.is_close()) { - maybe_t target_fd = spec.get_target_as_fd(); - if (!target_fd.has_value()) { - // Like `cmd >&gibberish`. - const wchar_t *fmt = - _(L"Requested redirection to '%ls', which is not a valid file descriptor"); - return report_error(STATUS_INVALID_ARGS, redir_node, fmt, spec.target.c_str()); - } else if (*target_fd > STDERR_FILENO && !allow_high_fds) { - // Like `echo foo 2>&5`. Don't allow internal procs to write to internal fish fds. - const wchar_t *fmt = _(L"Redirection to fd %d is only valid for external commands"); - return report_error(STATUS_INVALID_ARGS, redir_node, fmt, *target_fd); - } + if (spec.mode == redirection_mode_t::fd && !spec.is_close() && !spec.get_target_as_fd()) { + const wchar_t *fmt = + _(L"Requested redirection to '%ls', which is not a valid file descriptor"); + return report_error(STATUS_INVALID_ARGS, redir_node, fmt, spec.target.c_str()); } out_redirections->push_back(std::move(spec)); @@ -1080,8 +1066,7 @@ end_execution_reason_t parse_execution_context_t::populate_block_process( assert(args_or_redirs && "Should have args_or_redirs"); redirection_spec_list_t redirections; - auto reason = - this->determine_redirections(*args_or_redirs, false /* no high fds */, &redirections); + auto reason = this->determine_redirections(*args_or_redirs, &redirections); if (reason == end_execution_reason_t::ok) { proc->type = process_type_t::block_node; proc->block_node_source = pstree; diff --git a/src/parse_execution.h b/src/parse_execution.h index 4d5a0369b..5220d7937 100644 --- a/src/parse_execution.h +++ b/src/parse_execution.h @@ -130,11 +130,7 @@ class parse_execution_context_t { globspec_t glob_behavior); // Determines the list of redirections for a node. - // If \p allow_high_fds is false, then report an error for an fd redirection other than to - // in/out/err. This is set when constructing an internal process and prevents writing random - // data to internal fish fds. end_execution_reason_t determine_redirections(const ast::argument_or_redirection_list_t &list, - bool allow_high_fds, redirection_spec_list_t *out_redirections); end_execution_reason_t run_1_job(const ast::job_t &job, const block_t *associated_block); diff --git a/tests/checks/fds.fish b/tests/checks/fds.fish index c2a60bb1f..c3b606a2c 100644 --- a/tests/checks/fds.fish +++ b/tests/checks/fds.fish @@ -23,9 +23,6 @@ $helper print_fds &2 -# CHECK: 0 1 2 5 - # This attempts to trip a case where the file opened in fish # has the same fd as the redirection. In this case, the dup2 # does not clear the CLO_EXEC bit. diff --git a/tests/checks/redirect.fish b/tests/checks/redirect.fish index 7c5d112cf..50222c264 100644 --- a/tests/checks/redirect.fish +++ b/tests/checks/redirect.fish @@ -100,35 +100,10 @@ echo $status read abc <&- #CHECKERR: read: stdin is closed -# This one should output nothing. -echo derp >&- -# Verify that builtins, blocks, and functions may not write to arbitrary fds. -echo derp >&12 -#CHECKERR: {{.*}} Redirection to fd 12 is only valid for external commands -#CHECKERR: echo derp >&12 -#CHECKERR: ^ - -begin - echo derp -end <&42 -#CHECKERR: {{.*}} Redirection to fd 42 is only valid for external commands -#CHECKERR: end <&42 -#CHECKERR: ^ - -outnerr 2>&7 -#CHECKERR: {{.*}} Redirection to fd 7 is only valid for external commands -#CHECKERR: outnerr 2>&7 -#CHECKERR: ^ - -# Redirection to 0, 1, 2 is allowed. We don't test 0 since writing to stdin is weird and unpredictable. -echo hooray1 >&1 -echo hooray2 >&2 -#CHECK: hooray1 -#CHECKERR: hooray2 # "Verify that pipes don't conflict with fd redirections" -# This code is very similar to eval. We go over a bunch of fds +# This code is very similar to eval. We go over a bunch of fads # to make it likely that we will nominally conflict with a pipe # fish is supposed to detect this case and dup the pipe to something else echo "/bin/echo pipe 3 <&3 3<&-" | source 3<&0 @@ -138,6 +113,9 @@ echo "/bin/echo pipe 6 <&6 6<&-" | source 6<&0 echo "/bin/echo pipe 7 <&7 7<&-" | source 7<&0 echo "/bin/echo pipe 8 <&8 8<&-" | source 8<&0 echo "/bin/echo pipe 9 <&9 9<&-" | source 9<&0 +echo "/bin/echo pipe 10 <&10 10<&-" | source 10<&0 +echo "/bin/echo pipe 11 <&11 11<&-" | source 11<&0 +echo "/bin/echo pipe 12 <&12 12<&-" | source 12<&0 #CHECK: pipe 3 #CHECK: pipe 4 #CHECK: pipe 5 @@ -145,3 +123,6 @@ echo "/bin/echo pipe 9 <&9 9<&-" | source 9<&0 #CHECK: pipe 7 #CHECK: pipe 8 #CHECK: pipe 9 +#CHECK: pipe 10 +#CHECK: pipe 11 +#CHECK: pipe 12