Forbid subcommand keywords in variables-as-commands (#10249)

This stops you from doing e.g.

```fish
set pager command less
echo foo | $pager
```

Currently, it would run the command *builtin*, which can only do
`--search` and similar, and would most likely end up printing its own
help.

That means it very very likely won't work, and the code is misguided -
it is trying to defeat function resolution in a way that won't do what
the author wants it to.

The alternative would be to make the command *builtin* execute the
command, *but*

1. That would require rearchitecting and rewriting a bunch of it and
the parser
2. It would be a large footgun, in that `set EDITOR command foo` will
only ever work inside fish, but $EDITOR is also used outside.

I don't want to add a feature that we would immediately have to discourage.
This commit is contained in:
Fabian Boehm 2024-02-06 22:12:55 +01:00 committed by GitHub
parent 70a5267682
commit bdfbdaafcc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 21 additions and 0 deletions

View file

@ -34,6 +34,7 @@ use crate::parse_constants::{
use crate::parse_tree::{NodeRef, ParsedSourceRef}; use crate::parse_tree::{NodeRef, ParsedSourceRef};
use crate::parse_util::parse_util_unescape_wildcards; use crate::parse_util::parse_util_unescape_wildcards;
use crate::parser::{Block, BlockId, BlockType, LoopStatus, Parser, ProfileItem}; use crate::parser::{Block, BlockId, BlockType, LoopStatus, Parser, ProfileItem};
use crate::parser_keywords::parser_keywords_is_subcommand;
use crate::path::{path_as_implicit_cd, path_try_get_path}; use crate::path::{path_as_implicit_cd, path_try_get_path};
use crate::pointer::ConstPointer; use crate::pointer::ConstPointer;
use crate::proc::{ use crate::proc::{
@ -567,6 +568,20 @@ impl<'a> ParseExecutionContext {
"The expanded command was empty." "The expanded command was empty."
); );
} }
// Complain if we've expanded to a subcommand keyword like "command" or "if",
// because these will call the corresponding *builtin*,
// which won't be doing what the user asks for
//
// (skipping in no-exec because we don't have the actual variable value)
if !no_exec() && parser_keywords_is_subcommand(out_cmd) && &unexp_cmd != out_cmd {
return report_error!(
self,
ctx,
STATUS_ILLEGAL_CMD.unwrap(),
&statement.command,
"The expanded command is a keyword."
);
}
EndExecutionReason::ok EndExecutionReason::ok
} }

View file

@ -334,3 +334,9 @@ printf '<%s>\n' ($fish -c 'echo "$abc["' 2>&1)
#CHECK: <fish: Invalid index value> #CHECK: <fish: Invalid index value>
#CHECK: <echo "$abc["> #CHECK: <echo "$abc[">
#CHECK: < ^> #CHECK: < ^>
set -l pager command less
echo foo | $pager
#CHECKERR: checks/expansion.fish (line 339): The expanded command is a keyword.
#CHECKERR: echo foo | $pager
#CHECKERR: ^~~~~^