don't use system to open files - #156
Conversation
|
I agree, also see #150 (comment) (read the man page) and this one for solving the encoding of spaces and special chars (xdg-open supports uri's): |
|
Hi, nice PR but when I tried to open a music file using this then it made my application hang on linux until I close my music player. The solution can be to run this command asynchronously |
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.
|
Thanks for pointing this out, it should be fixed now. |
|
@crazecoder Any status on this PR? |
|
@prateekmedia @miDeb OpenFile.open('~/Downloads/flutter.png');Tested on Linux (Ubuntu) |
|
@javaherisaber your "simple use case" requires a shell to expand the path |
|
@miDeb |
|
I would argue this package's job is to open the file that is located at the path specified, not necessarily to modify the path by performing shell expansion (e.g. expanding |
|
@miDeb is right, this package will only be used when the file path is known either relative or absolute, in both the cases we will have the correct path and not the path with |
5 participants
This removes the ability to use
systemto open files. Instead we useProcess.runSync.Quoting the manpage for
system:AFAICT only spaces are replaced with
\right now. This is not enough, and invoking thesystemfunction is also completely unnecessary. Creating a new process directly is bothsimpler and better. This option was introduced in #127,
however only on linux and not as the default. This is not sufficient.
Without this PR, 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):
I haven't tested this on macOS but it should work there similarly to linux.