fzf history not filtered by what user has already typed before invoking (Ctrl + R)

[kevinfiol]

Hi! Amazing work on clink-fzf and clink. I discovered your work yesterday and think it's all empowering as a Windows user. 🙏

While this plugin works great, it seems to not initialize the history command with what the user has typed so far in their terminal. Here are examples comparing my Powershell setup with Clink.

Here is Powershell using PSFzf:

https://user-images.githubusercontent.com/4752973/220920970-5c1f9fe9-34e1-40ff-a2c8-0dbba18c23bd.mp4

Here is clink v1.4.19.57e404:

https://user-images.githubusercontent.com/4752973/220921453-9abe05b6-31f3-4e7b-8bff-9fe9eb0eb841.mp4

Not sure if this is a bug or not implemented yet. I noticed this line, but I believe this is intended to capture what the user has typed in the fzf prompt:

local r = io.popen(history..' 2>nul | '..get_fzf('FZF_CTRL_R_OPTS')..' -i --tac')

EDIT: I just now understood this comment. If I'm reading this correctly, this functionality is intentional:

This intentionally does not use '--query' because that isn't safe:
    -- Depending on what the user has typed so far, passing it as an argument
    -- may cause the command to interpreted differently than expected.

Doing this does seem to get the result I want, but at the risk of what you mention in the comment. I noticed this also breaks if I do something like node - and then hit Ctrl + R:

local line = line_state:getline()
local r = io.popen(history..' 2>nul | '..get_fzf('FZF_CTRL_R_OPTS')..' -i --tac --query ' .. line)

[chrisant996]

@kevinfiol Correct. It can result in what is effectively an accidental "friendly fire" injection attack. Various severe unexpected side effects are possible with flags like --query, so I'm a little surprised fzf uses that approach (a safer approach could be to read the query from an input file).

Certain kinds of input line text can be escaped and made possible to pass to --query, but some kinds of text cannot be escaped successfully. And in some cases the act of escaping text can result in fzf doing a lookup on a different string than the user intended, resulting in a different set of results than the user was trying to find.

I could make it so that when the text doesn't need to be escaped, then clink-fzf could be pass it to --query. But then the behavior will be inconsistent depending on the input line (and so far, I had preferred to have consistent behavior).

Is that something you'd like?

Some especially troublesome characters are ", -, <, >, |, &, and some others.

[kevinfiol]

Thank you for the clarification. It's a tough one, but I agree with you that it's better to keep the behavior consistent. May be worth just noting the caveat in the README.

[chrisant996]

I pushed an update that should be a safe way to use the --query flag.

However, as of this writing, fzf with bash and other shells has a code injection bug in how --query is used.

[chrisant996]

Opened https://github.com/junegunn/fzf/issues/3185 to alert in the fzf repo about this code injection issue, and a potential approach for fixing it.

UPDATE: On *nix most shells automatically escape the contents of a quoted shell variable, so there is not a code injection issue in the shells that fzf includes native support for. Other shells need to do whatever is appropriate to sufficiently escape the query string before passing it to --query. The fzf.lua script now has a full implementation for escaping.

[chrisant996]

The latest fzf.lua (in both clink-fzf and clink-gizmos) now performs as much escaping as possible of the query string, while following the 2N rule for backslashes when escaping quotes. Now the only thing that can't be passed in the query string is just % because there's no way to prevent cmd from parsing % as part of environment variable expansion.