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

Linux and Macos file opening fix #7

Closed
wants to merge 6 commits into from

Conversation

prateekmedia
Copy link

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
Author

@javaherisaber can you merge this please?

@javaherisaber
Copy link
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
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
Owner

@prateekmedia
See this comment related to your PR

@lexxxel
Copy link

lexxxel commented Jul 31, 2024

@javaherisaber any updates or is your fork dead?

@javaherisaber
Copy link
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