[PATCH 05 of 11] cmdutil: securely rename a to A on disk

Mads Kiilerich mads at kiilerich.com
Fri Aug 7 21:20:13 UTC 2009


Simon Heimberg wrote, On 08/07/2009 09:43 PM:
> # HG changeset patch
> # User Simon Heimberg<simohe at besonet.ch>
> # Date 1249656995 -7200
> # Node ID 50e9f6b4d8878fd867a250c5be2578330b7f4769
> # Parent  76406c3720caa61cf9d466b43d2aac68b15ec362
> cmdutil: securely rename a to A on disk
>
> os.rename('a', 'A') does not change anything on hfs on linux
> extract temprename from util.rename
>
> diff -r 76406c3720ca -r 50e9f6b4d887 mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py	Mit Jul 22 12:52:02 2009 +0200
> +++ b/mercurial/cmdutil.py	Fre Aug 07 16:56:35 2009 +0200
> @@ -404,7 +404,8 @@
>                   # case change on case insensitive filesystems
>                   changecase = True
>                   if not dryrun:
> -                    os.rename(src, target)
> +                    temp = util.temprename(src)
> +                    os.rename(temp, target)
>               else:
>                   ui.warn(_('%s: not overwriting - file exists\n') %
>                           reltarget)
> diff -r 76406c3720ca -r 50e9f6b4d887 mercurial/util.py
> --- a/mercurial/util.py	Mit Jul 22 12:52:02 2009 +0200
> +++ b/mercurial/util.py	Fre Aug 07 16:56:35 2009 +0200
> @@ -416,6 +416,19 @@
>           return False
>       return True
>
> +def temprename(src):
> +    """renames a file to a temporary name
> +       the new name is returned"""
> +    for tries in xrange(10):
> +        temp = '%s-%08x' % (src, random.randint(0, 0xffffffff))
> +        try:
> +            os.rename(src, temp)
> +            return temp
> +        except IOError:
> +            continue
> +    raise IOError, (errno.EEXIST, "No usable temporary filename found")
> +
>    

Does moving a function count as touching it?

IF you are touching it anyway then it it would be more secure with an 
extra "if os.path.exists(temp): continue".

This function can also give strange errors on filenames close to the max 
length. Perhaps we could drop the basename and use a short "random" name 
in dirname...

> +
>   def rename(src, dst):
>       """forcibly rename a file"""
>       try:
> @@ -435,15 +448,7 @@
>           # to the nature of the operation however, any races will at worst
>           # lead to the rename failing and the current operation aborting.
>
> -        def tempname(prefix):
> -            for tries in xrange(10):
> -                temp = '%s-%08x' % (prefix, random.randint(0, 0xffffffff))
> -                if not os.path.exists(temp):
> -                    return temp
> -            raise IOError, (errno.EEXIST, "No usable temporary filename found")
> -
> -        temp = tempname(dst)
> -        os.rename(dst, temp)
> +        temp = temprename(dst)
>           os.unlink(temp)
>           os.rename(src, dst)
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>    




More information about the Mercurial-devel mailing list