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

Prevent arguments from being interpreted as options #80

Merged
merged 2 commits into from
Jan 16, 2021
Merged

Prevent arguments from being interpreted as options #80

merged 2 commits into from
Jan 16, 2021

Conversation

kidonng
Copy link
Contributor

@kidonng kidonng commented Jan 14, 2021

User inputs, file names and commands can contain leading hypens (--), which may cause errors or the function not working.

Examples where fzf.fish would error:

  • the cursor is over the token --something when the user executes find file
  • the selected changed path from git status is -folder
  • the selected command is --function

The solution is to delineate the end of options with --, which is a POSIX specification defined in https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html#tag_12_02 (guideline 10), which is respected by Fish builtins. After --, the command will always interpret the arguments as positional arguments.

Copy link
Owner

@PatrickF1 PatrickF1 left a comment

Choose a reason for hiding this comment

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

Wow some SQL-injection-like bugs you're fixing here. Amazing, didn't even consider these edge cases!

@PatrickF1 PatrickF1 changed the title Prevent mangled arguments Prevent user's selection from being interpreted as arguments Jan 14, 2021
@PatrickF1 PatrickF1 changed the title Prevent user's selection from being interpreted as arguments Prevent arguments from being interpreted as options Jan 14, 2021
@PatrickF1
Copy link
Owner

PatrickF1 commented Jan 14, 2021

Can you link me to documentation that describes how -- fixes it?

EDIT: nvm didn't realize this is a POSIX thing which is easy to Google https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html#tag_12_02 is where it's described

@PatrickF1
Copy link
Owner

Doesn't __fzf_search_shell_variables.fish have the same problem?

@kidonng
Copy link
Contributor Author

kidonng commented Jan 15, 2021

It's included.

@PatrickF1
Copy link
Owner

I really don't see it...

@PatrickF1
Copy link
Owner

@kidonng Oh LOL I typoed. You probably thought I was crazy. I meant __fzf_search_git_status.fish

@kidonng
Copy link
Contributor Author

kidonng commented Jan 16, 2021

Right, I forgot that it will insert paths eventually.

@PatrickF1
Copy link
Owner

Nice, thanks! I was about to ask why you didn't add -- to all the string commands but then I realized all the lines will always start with a space and a letter (e.g. M functions/file.fish so there's no need. Good thing you wrote the code first!

@PatrickF1 PatrickF1 merged commit 89685b8 into PatrickF1:main Jan 16, 2021
@PatrickF1 PatrickF1 deleted the argv branch January 16, 2021 07:07
PatrickF1 pushed a commit that referenced this pull request Mar 6, 2021
hrshtst pushed a commit to hrshtst/fzf.fish that referenced this pull request Apr 22, 2021
Prevent errors that can happen if the current token, selected path, or selected command, beings with a `-`. 
Examples where fzf.fish would error:
- the cursor is over the token --something when the user executes search files
- the cursor is over -z when the user executes search variables
- the selected changed path from git status is -folder
- the selected command is --function

The solution is to delineate the end of options with --, which is a POSIX specification defined in https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html#tag_12_02 (guideline 10), which is respected by Fish builtins. After --, the command will always interpret the arguments as positional arguments.
hrshtst pushed a commit to hrshtst/fzf.fish that referenced this pull request Apr 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants