fix opening filenames with spaces on linux #127
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Noticed this problem as well, but this is not actually enough since filenames can contain quotes as well! I solved this by appending a backslash in front of every character. (I wrote this in an email to the mantainer because I figured this might be a security vulnerability. It most likely isn't) |
|
I believe the best way to implement this is is to open the right process right away instead of using the |
|
@miDeb you have a good point. TBH, I also don't understand why the system call is used here. I assumed the author had good reasons.. but maybe not as good as I hope ;) maybe @crazecoder can chime in. I'll update my PR anyway to use Process |
- xdg-open is available on all platforms, no need for other variations - removed linux.dar: no longer needed
|
The same issue seems to exist on macOS fwiw. |
miDeb
added a commit
to miDeb/open_file
that referenced
this pull request
Jul 18, 2021
This removes the ability to use `system` to open files. Instead we use `Process.runSync`. Quoting the [manpage for `system`](https://man.archlinux.org/man/system.3): > Any user input that is employed as part of command should be carefully sanitized, > to ensure that unexpected shell commands or command options are not executed. AFAICT only spaces are replaced with `\ ` right now. This is not enough, and invoking the `system` function is also completely unnecessary. Creating a new process directly is both simpler and better. This option was introduced in crazecoder#127, however only on linux and not as the default. This is not sufficient. Running the following in an unsandboxed flutter app on linux would open `gnome-calculator` (if installed, but you can place any command in there that will be executed): ```dart OpenFile.open(";gnome-calculator"); ``` I haven't tested this on macOS but it _should_ work there similarly to linux.
miDeb
added a commit
to miDeb/open_file
that referenced
this pull request
Sep 21, 2021
This removes the ability to use `system` to open files. Instead we use `Process.runSync`. Quoting the [manpage for `system`](https://man.archlinux.org/man/system.3): > Any user input that is employed as part of command should be carefully sanitized, > to ensure that unexpected shell commands or command options are not executed. AFAICT only spaces are replaced with `\ ` right now. This is not enough, and invoking the `system` function is also completely unnecessary. Creating a new process directly is both simpler and better. This option was introduced in crazecoder#127, however only on linux and not as the default. This is not sufficient. Running the following in an unsandboxed flutter app on linux would open `gnome-calculator` (if installed, but you can place any command in there that will be executed): ```dart OpenFile.open(";gnome-calculator"); ``` I haven't tested this on macOS but it _should_ work there similarly to linux.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #126
I'm not sure about the other platforms, so I stayed out of it..