osdir.com


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Python-Dev] Remove tempfile.mktemp()


On Tue, Mar 19, 2019 at 6:21 PM Guido van Rossum <guido at python.org> wrote:

>> On Tue, Mar 19, 2019 at 10:14 AM Giampaolo Rodola' <g.rodola at gmail.com> wrote:
>> Technically you cannot make it 100% safe, only less likely to occur.
> That seems unnecessarily pedantic. A cryptographic random generator, like the one in the secrets module, *is* safe enough for all practical purposes (with a large margin).
> [...]
> Hm, the random sequence (implemented in tempfile._RandomNameSequence) is currently derived from the random module, which is not cryptographically secure. Maybe all we need to do is replace its source of randomness with one derived from the secrets module. That seems a one-line change.

Seems to make sense to me. Currently the random string is 8 chars
long, meaning (if I'm not mistaken) max 40320 possible combinations:

    >>> len(list(itertools.permutations('wuoa736m'))))
    40320

We may use 9 chars and get to 362880 and increase "security".
Regardless, it seems to me the original deprecation warning for
mktemp() was not really justified and should be removed. IMO it
probably makes more sense to just clarify in the doc that
NamedTemporaryFile is the right / safe way to create a file with a
unique name and avoid the theoretical, rare name collision you'd get
by using open(mktemp(), 'w') instead. Also I'm not sure I understand
what the code sample provided in the doc aims to achieve:
https://docs.python.org/3/library/tempfile.html#tempfile.mktemp
The way I see it, the main (or "correct") use case for mktemp() is
when you explicitly want a file name which does not exist, in order to
exercise legitimate scenarios like these ones:

    >>> self.assertRaises(FileNotFoundError, os.unlink, tempfile.mktemp())

    >>> dst = tempfile.mktemp()
    >>> os.rename(src, dst)
    >>> assert not os.path.exists(src) and os.path.exists(dst)

-- 
Giampaolo - http://grodola.blogspot.com