Skip to content

Linux and Macos file opening fix - #7

Closed
prateekmedia wants to merge 6 commits into
javaherisaber:masterfrom
miDeb:master
Closed

Linux and Macos file opening fix#7
prateekmedia wants to merge 6 commits into
javaherisaber:masterfrom
miDeb:master

Conversation

@prateekmedia

Copy link
Copy Markdown

This PR reduces the code from two files to 4 lines and also fixes the error in linux and mac on opening of files with special characters.

More info from on crazecoder#156:

This removes the ability to use system to open files. Instead we use Process.runSync.

Quoting the manpage for system:

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.

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):

  OpenFile.open(";gnome-calculator");

I haven't tested this on macOS but it should work there similarly to linux.

@prateekmedia

Copy link
Copy Markdown
Author

@javaherisaber can you merge this please?

@javaherisaber

Copy link
Copy Markdown
Owner

@prateekmedia
Thanks for your contribution to the repo and sorry for the delay, I've been busy with my projects.
I'll try to verify the changes and will keep you posted on this thread

@prateekmedia

Copy link
Copy Markdown
Author

It may have some merge conflicts as it is not my code, but from my side it is verified to be legitimate and working.

@javaherisaber

Copy link
Copy Markdown
Owner

@prateekmedia
See this comment related to your PR

@lexxxel

lexxxel commented Jul 31, 2024

Copy link
Copy Markdown

@javaherisaber any updates or is your fork dead?

@javaherisaber

Copy link
Copy Markdown
Owner

@prateekmedia @lexxxel @miDeb
A new version including your fix is now published to the pub.dev
Sorry for the delay, I've been busy last year and couldn't make it
Thanks for your efforts on this PR

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.

4 participants