[FFmpeg-devel] [RFC] av_tempfile()

[Reimar Döffinger]

On Sun, Oct 16, 2011 at 08:50:03PM +0200, Michael Niedermayer wrote:
> On Sun, Oct 16, 2011 at 07:32:55PM +0200, Reimar Döffinger wrote:
> > On Sun, Oct 16, 2011 at 03:31:07PM +0200, Michael Niedermayer wrote:
> > > Hi
> > > 
> > > I think ff_tempfile() should be in libavutil as its a generically
> > > usefull thing and i will also need it from libavformat. Currently
> > > its only compiled when we link to xvid.
> > > 
> > > Ill move it to libavutil soon, but i wont update the version yet, only
> > > once there is agreement (or lack of disagreement) on name, API and
> > > place
> > 
> > I seem to remember some issues with it, so I wouldn't like too much for
> > it become critical.
> > The issues I see on a quick look are that the non-mkstemp fallback might
> > result in overall code being unsafe, that it will always create the file
> > in "." in that case.
> > For the mkstemp code that it always tries /tmp which is not really
> > appropriate for Windows,
> > that it does not respect TEMPDIR or any of the
> > other ones like that (note: I have more than once worked on systems
> > where tmp was accessible but had performance that made it unusable),
> > and it falls back to trying to create a file in "." for which I think
> > there is more than one reason to dislike.
> > Mostly I do not like of libav* creating any kind of file at all, ever,
> > or if there is no alternative at all it should be something like mkstemp
> > except that it will at also atomically unlink the file before returning
> > its descriptor.
> 
> Some users might want to keep the temporary files for debuging or in
> case of the cache protocol as a means to download while watching.
> 
> But what do you suggest how should it be implemented?
> We have several features that depend on temporary files (the libxvid
> wraper and the caching protocol)

Honestly? Remove the insecure crap first, and just fail.
"Features over security" IMO is not an acceptable behaviour, especially
if it's not possible to disable it.
Then force the user to specify a file name. That also works far better
if you want the "download while watching" to work sanely.
If that's not good enough, then think about the requirements and then
implement a function that matches them and describe those in the
descriptions. Just taking a function that is so full of issue even
after you fixed two (three?) of them and using it just because it's
there hardly makes much sense.