feat: /fopen command to auto-open downloaded files#273
Conversation
| usleep(100000); | ||
| } | ||
| char command[MAX_STR_SIZE]; | ||
| snprintf(command, sizeof(command), "xdg-open %s", ft->file_path); |
There was a problem hiding this comment.
to mitigate potential command injection, i suggest we sanitize the string and make them url encoded (with the exception of '/')
eg: xdg-open file:///home/green/mpv-shot0001.jpg
or with space: xdg-open file:///home/green/mpv%20shot0001.jpg
There was a problem hiding this comment.
the url encoder could be used for other hyperlinks too.
| if (mkdir(tmp_dir, S_IRWXU) == -1) { | ||
| line_info_add(self, false, NULL, NULL, SYS_MSG, 0, 0, "Could not create tox download /tmp/ directory.\n", err); | ||
| } | ||
| // end make tmpdir if it does not exist |
There was a problem hiding this comment.
i am not sure what the reasoning behind the temp dir is.
| // make and call xdg command | ||
| while (ft->state != FILE_TRANSFER_INACTIVE) { | ||
| usleep(100000); | ||
| } |
There was a problem hiding this comment.
there are legitimate uses for opening files that are still being transferred (most streaming media types like videos and audio)
There was a problem hiding this comment.
That sleep will freeze toxic. ft->state can't update while we're in that loop because that function is holding a mutex lock. Sleeps in random functions is almost never the correct solution. They're generally only used in the main loop or in task-specific threads.
Green-Sky
changed the title
/fopen command to auto-open downloaded files/fopen command to auto-open downloaded files
| line_info_add(self, false, NULL, NULL, SYS_MSG, 0, 0, "I am about to run the xdg command."); | ||
| line_info_add(self, false, NULL, NULL, SYS_MSG, 0, 0, "The file path is '%s'", ft->file_path); | ||
|
|
||
| int open_result = popen(command, "r"); |
There was a problem hiding this comment.
popen() is a rather dangerous function that does shell interpretation, so it's prone to shell injection. Why not fork() + exec*() instead, calling /usr/bin/xdg-open directly?
There was a problem hiding this comment.
while I agree with the first statement, said path is not portable.
nixos:
$ which xdg-open
/run/current-system/sw/bin/xdg-open
There was a problem hiding this comment.
ie we would have to search for the binary first (probably once at startup)
|
superseded by #407 |
5 participants
So the xdg-open command will fire if I call it before the download, but not after. Any ideas what's going on? I can see that the code got there via the debugging line, but it doesn't seem to do anything.
This change is