[FFmpeg-devel] The "bad" Patch

[softworkz .]

Hello everybody,

it's about 14 days ago, on Thursday, 2025-05-15, when I had applied 
my "Execution Graph Printing" patchset after 12 revisions and 3 
reminder e-mails sent to the ML.
The patch 15/15 had a controversial reception from other FFmpeg
developers, ranging from friendly concerns up to aggressive blown-up 
comments, some with elements of defamation, false accusations which
were blindly repeated even after I had rectified them.


It has been difficult to navigate for me, as there were very 
different kinds of arguments, and not all of them were of a type
that would have justified immediate reversion. Also, I cannot 
consider people as trustworthy while they are going crazy. I had
hoped for entering a discussion to explain the details, but once 
the mob had formed and I had realized that a reasonable discussion 
is not possible, I chose to revert that patch.

All-in-all, what remains from that situation is a very wrong 
picture, that I cannot leave unresolved. I had only decided to
wait those 14 days, in order to allow for some cooldown. But 
now it's really time for clearing a few things up.

I had noticed a number of misunderstandings about the patch (many 
don't even seem to have looked at it). I don't mean to say that 
everybody misunderstood everything, it was rather mixed and there
has also been one very valid comment regarding the way how the temp
folder was determined. 



Misunderstanding 1: What is being launched?
===========================================

It appears that many had only read the headlines and assumed the code 
would launch the executable of a browser in a new process.
Those who said that system() is not the right way to launch such a 
process, are absolutely right. But that's not what's happening.

What's really happening: It is invoking a common shell script/command
(named xdg-open) which is handling the request and opens the configured
application when required, or when already running, it causes it to open
the provided file or url. xdg-open does that for many file types, it's
not just for opening urls and is a de-facto standard on Linux, similar
to ShellExecute() on Windows.


Misunderstanding 2: Opening URLs is a big issue and taboo for CLI tools
=======================================================================

In fact, so many applications are doing it, that it's almost pointless
to name any. Almost all GUI applications are showing URLs somewhere on
which you can click to open in the browser, if not part of the actual
functionality, then at least in some Help>About dialog.
Many text editors are detecting URLs and allow launching them in the 
browser in some way. The same applies to text in terminal application
windows quite often.
All these are launching the URLs through xdg-open. It's not a big
thing and in no way special.

Neither is it unusual for Linux CLI tools to launch URLs in a browser,
some prominent examples are docker (Docker), aws (Amazon) or gh (GitHub).

Some had reacted as if a CLI that would do browser-launching
would be so toxic and unacceptable that they would never use it.
Well - think again, because then you'd need to stop using: Git

Even Git does it - and you're all using it.


Misunderstanding 3: I would not be knowing what I'm doing
=========================================================

Some had alluded that I would not be knowing about possible dangers
when invoking a shell, like injection attacks. But that's really far 
from the truth. One of the first things I did when I started working
with FFmpeg more than a decade ago was creating a huge .net lib,
partially auto-created with about 2k classes, precisely reflecting
ffmpeg's codecs, muxers, filters, protocols, etc - with all their
options replicated in detail with proper types, value constraints
and enums. That lib has a single purpose only: Building valid 
and safe FFmpeg command lines on all platforms. A simple shell 
escape is not the only form of injection attacks and - without
giving any recipes - FFmpeg command lines are an attractive
target for such attacks. Bottom line: No matter how you are 
launching something - via shell or by starting a process - it is
always important to make sure the parameters that you are passing
anywhere are valid and sane.


"Bad" API?
==========

As far as I'm seeing it, an API can hardly be intrinsically bad.
It depends on the case and the parameters that are passed.
With the wrong parameters, bad things can happen with many APIs.

So, let's take a look at the command string that is actually run:

xdg-open '...path...' </dev/null 1>/dev/null 2>&1 &


The path is built from two parts combined, temp path and generated
file name, e.g.

/tmp/ffmpeg_graph_2025-05-28_06-53-23_550.html


There's nothing in the resulting command that is depending on 
user input and could be manipulated in some way.

So, what's actually bad about this invocation? Where's the risk?

While searching for an answer, I've seen many uses of system()
in Linux tools (less, Vim, linuxmint; Python's os.system() maps
directly to glibc's system() and is used everywhere) - and I've 
seen even more which are doing the exact same things as glibc
does internally (launching sh).

I fully understand that care needs to be taken when invoking a
shell. It should raise a flag - absolutely. But the next step
must be careful evaluation rather than going crazy.
I really don't see why it should be categorized as "forbidden",
especially not for a very clear and narrow case like here.


So far, the system() API in glibc has only been talked about 
as a black box - a "bad" black box. Making claims about
something unknown is always easy - but not useful for 
discussion, and hence I've moved that out of the way:
I have updated and reposted the "bad" patch, alongside a 
second patch which substitutes the system() call with the 
actual source code from glibc 2.31 that is used on Linux 
systems. Now, it's no longer a black box.


And so, here's my question to all those who had told me how
forbidden, unacceptable and 'no-no' the use of the system()
API would be:

Could you please explain to me in detail what exactly is 
bad about using that API in THIS WAY and in THIS CONTEXT?

Which bad things can happen and how exactly?

Thanks
sw


Please note:

I have no intentions towards getting this merged.

It's a valid opinion when you think that "ffmpeg shouldn't
launch a browser" or similar - but that's not the topic.
This is merely a technical question.