> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of Ramiro Polla
> Sent: Freitag, 16. Mai 2025 00:13
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make it a
> Killer-Feature!
>
> On Fri, May 16, 2025 at 12:00 AM softworkz .
> <softworkz-at-hotmail....@ffmpeg.org> wrote:
> > > On Thu, May 15, 2025 at 11:11 PM softworkz <g...@videolan.org> wrote:
> > > [...]
> > > > diff --git a/fftools/graph/filelauncher.c b/fftools/graph/filelauncher.c
> > > > new file mode 100644
> > > > index 0000000000..45514ca599
> > > > --- /dev/null
> > > > +++ b/fftools/graph/filelauncher.c
> > > [...]
> > > > +int ff_open_html_in_browser(const char *html_path)
> > > > +{
> > > > + if (!html_path || !*html_path)
> > > > + return -1;
> > > > +
> > > > +#if defined(_WIN32)
> > > > +
> > > > + // --- Windows ---------------------------------
> > > > + {
> > > > + HINSTANCE rc = ShellExecuteA(NULL, "open", html_path, NULL,
> NULL,
> > > SW_SHOWNORMAL);
> > > > + if ((UINT_PTR)rc <= 32) {
> > > > + // Fallback: system("start ...")
> > > > + char cmd[1024];
> > > > + _snprintf_s(cmd, sizeof(cmd), _TRUNCATE, "start \"\"
> \"%s\"",
> > > html_path);
> > > > + if (system(cmd) != 0)
> > > > + return -1;
> > > > + }
> > > > + return 0;
> > > > + }
> > > > +
> > > > +#elif defined(__APPLE__)
> > > > +
> > > > + // --- macOS -----------------------------------
> > > > + {
> > > > + // "open" is the macOS command to open a file/URL with the
> default
> > > application
> > > > + char cmd[1024];
> > > > + snprintf(cmd, sizeof(cmd), "open '%s' 1>/dev/null 2>&1 &",
> > > html_path);
> > > > + if (system(cmd) != 0)
> > > > + return -1;
> > > > + return 0;
> > > > + }
> > > > +
> > > > +#else
> > > > +
> > > > + // --- Linux / Unix-like -----------------------
> > > > + // We'll try xdg-open, then gnome-open, then kfmclient
> > > > + {
> > > > + // Helper macro to try one browser command
> > > > + // Returns 0 on success, -1 on failure
> > > > + #define TRY_CMD(prog) do { \
> > > > + char buf[1024]; \
> > > > + snprintf(buf, sizeof(buf), "%s '%s' 1>/dev/null 2>&1 &", \
> > > > + (prog), html_path); \
> > > > + int ret = system(buf); \
> > > > + /* On Unix: system() returns -1 if the shell can't run. */\
> > > > + /* Otherwise, check exit code in lower 8 bits.
> */\
> > > > + if (ret != -1 && WIFEXITED(ret) && WEXITSTATUS(ret) == 0) \
> > > > + return 0; \
> > > > + } while (0)
> > > > +
> > > > + TRY_CMD("xdg-open");
> > > > + TRY_CMD("gnome-open");
> > > > + TRY_CMD("kfmclient exec");
> > > > +
> > > > + fprintf(stderr, "Could not open '%s' in a browser.\n",
> html_path);
> > > > + return -1;
> > > > + }
> > > > +
> > > > +#endif
> > > > +}
> > > [...]
> > >
> > > Sorry I didn't have a closer look at the patchset while it was under
> > > review, but system(cmd) is a big no-no. We could create a file with an
> > > explicit path passed by the user, but then it's up to the user to open
> > > it.
> >
> > What's bad about opening a file in the browser when that's the documented
> > behavior of the cli parameter?
>
> Straight out of ChatGPT:
> I understand the motivation — making the feature more user-friendly by
> launching the result directly is a nice touch. The concern isn't with
> the feature itself, but rather with the way it's implemented.
> Using system() to launch a browser introduces potential security
> risks, especially if the file path is ever constructed from untrusted
> input (e.g. future scripting, API wrapping, or unexpected shell
> expansion). It's generally discouraged in projects like FFmpeg, where
> robustness and security are critical.
Hi,
of course I understand that.
But it isn't constructed from untrusted input.
Best regards
sw
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
