[patch] Fix g10/exec.c (make_tempdir) [_WIN32] not properly handling GetTempPath

[David Shaw]

On Wed, May 24, 2006 at 10:50:23PM +0100, Israel G. Lugo wrote:
> David Shaw wrote:
> > I think the proposed fix is a bit of overkill.  I'd rather do
> > something simpler, like (not tested yet):
> That is still enforcing a hardcoded limit, though, of MAX_PATH
> characters (which is defined to 260 in the 2003 SDK). If we can behave
> elegantly and support arbitrarily long paths with ease, why would we
> want to impose an unnecessary artificial limit on ourselves?
> 
> If the user's temporary directory happens to be longer than 260 bytes
> (think localized Windows versions, or large usernames, or multibyte
> locales or whatever), the alternative solution you quoted will reject
> it (and fall back to the non-portable c:\windows\temp thing), when it
> wasn't really necessary to do so.

So MAX_PATH is not, in fact, the maximum path on Windows?  Wow, that's
a good idea.  Is there a MAX_PATH_WE_REALLY_MEAN_IT or similar that I
can use here?  What is the real maximum path?

> > There is no need to get the Windows directory specially, as
> > GetTempPath will return the Windows directory as a last resort.  It
> >  tries $TMP, $TEMP, $USERPROFILE, and finally the Windows
> > directory.
> That is true; I suppose I was only trying to make the program a bit
> more resilient. We don't really know for what reasons GetTempPath
> might fail (some internal error, etc). It could fail for a reason
> other than not being able to detect the Windows directory; in such a
> case, GPG might still be able to get the Windows directory, by using
> GetWindowsDirectory (if that fails too then we could always fall back
> to the hardcoded thing).

To what end?  We could write a dozen fallbacks: "well, if GetTempPath
fails, we'll figure out the localization of the particular system and
try a few directories, if that fails we'll figure out what version of
Windows it is and try a few directories..."

No, therein lies madness.  The OS provides GetTempPath.  If it fails,
then the answer is to tell the user so and they can fix it.  It's
rather like how GnuPG uses a wrapper around malloc.  If malloc fails,
it aborts the process.  It doesn't try and use a file on disk as
temporary swap space so it can keep going.  If /dev/random doesn't
exist, we tell the user that and exit.  We don't call mknod.
Similarly, if we're fetching a key from a keyserver on port 11371 (the
HKP port) and get a connection refused, we don't try port 80 just in
case it'll work.  If the user wanted port 80, they'd have asked for
port 80.

You're quite right that the existing code uses GetTempPath
incorrectly, and that the code won't work (probably crash) if the
temporary path is longer than 256 bytes.  Give me a number and I'll
raise the size, but layers of fallbacks strikes me as unnecessary
complication in a relatively unimportant place in the code.  Note also
that this incorrect code has been in GnuPG for around 4 years now, and
nobody noticed, meaning nobody used a temporary directory path larger
than 256 bytes...

> I understand it may perhaps be difficult for some of the GPG
> developers to be motivated to develop for the Windows platform, or
> even to test the program on it. I myself use GNU/Linux for most of my
> computing tasks, but I do regrettably have to use Windows as well.

This is not a "Windows=Bad, Unix=Good" thing!  Both Windows and Unix
provide an API for getting a temporary directory.  If that API fails,
then that's it, on whatever platform.  In the Unix code, if you set
$TMPDIR to garbage, then it won't work.  In neither case do I think
that trying many different directories to try and find a "good" one is
advisable.

David