[PATCH for default] path_auditor: check filenames for basic platform validity (issue2755)

[Matt Mackall]

On Thu, 2011-04-07 at 23:10 +0200, Adrian Buehlmann wrote:
> # HG changeset patch
> # User Adrian Buehlmann <adrian at cadifra.com>
> # Date 1302106183 -7200
> # Node ID 4dafe2d07b287fa7d79a325cc8d610d749d62c26
> # Parent  d3f90ff904b80519ee432241d359d34d75af5475
> path_auditor: check filenames for basic platform validity (issue2755)

Hmmm, the placement of this code is tricky.

We've got two related questions:

a) can I check out file X on the current OS?
b) can I check in file X on OS A and still check it out on B, C, D...?

Question (a) strongly suggests we want a per-platform function like your
checkosfilename() you've implemented here. But question (b) suggests an
checkportable() function that somehow knows about issues everywhere
else. And the two together suggest that checkportable() delegates to a
few checkXXXfilename() functions that are in the platform neutral code.

Given that Windows is the only platform with significant name
portability issues, this basically ends up looking like:

posix:
 checkosfilename = pass
 checkportable = checkwinfilename

windows:
 checkosfilename = checkwinfilename
 checkportable = checkwinfilename

Does that make sense?

> Example (on Windows):
> 
>   $ hg parents
>   $ hg manifest tip
>   con.xml
>   $ hg update
>   abort: filename contains 'con', which is reserved on Windows: con.xml
> 
> Before this patch, update produced (as explained in issue2755):
> 
>   $ hg update
>   abort: No usable temporary filename found
> 
> I've added the new function checkwinfilename to util.py and not to windows.py,
> so that we can later call it when running on posix platforms too, for when we
> decide to implement a (configurable) warning message on 'hg add'.
> 
> As per this patch, checkwinfilename is currently only used when running
> on Windwows.
> 
> path_auditor calls checkosfilename, which is a NOP on posix and an alias for
> checkwinfilename on Windows.
> 
> diff --git a/mercurial/posix.py b/mercurial/posix.py
> --- a/mercurial/posix.py
> +++ b/mercurial/posix.py
> @@ -147,6 +147,11 @@
>      except (OSError, AttributeError):
>          return False
>  
> +def checkosfilename(path):
> +    '''Check that the base-relative path is a valid filename on this platform.
> +    Returns None if the path is ok, or a UI string describing the problem.'''
> +    pass # on posix platforms, every path is ok
> +
>  def set_binary(fd):
>      pass
>  
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -493,6 +493,48 @@
>  
>      return hardlink, num
>  
> +_windows_reserved_filenames = '''con prn aux nul
> +    com1 com2 com3 com4 com5 com6 com7 com8 com9
> +    lpt1 lpt2 lpt3 lpt4 lpt5 lpt6 lpt7 lpt8 lpt9'''.split()
> +_windows_reserved_chars = ':*?"<>|'
> +def checkwinfilename(path):
> +    '''Check that the base-relative path is a valid filename on Windows.
> +    Returns None if the path is ok, or a UI string describing the problem.
> +
> +    >>> checkwinfilename("just/a/normal/path")
> +    >>> checkwinfilename("foo/bar/con.xml")
> +    "filename contains 'con', which is reserved on Windows"
> +    >>> checkwinfilename("foo/con.xml/bar")
> +    "filename contains 'con', which is reserved on Windows"
> +    >>> checkwinfilename("foo/bar/xml.con")
> +    >>> checkwinfilename("foo/bar/AUX/bla.txt")
> +    "filename contains 'AUX', which is reserved on Windows"
> +    >>> checkwinfilename("foo/bar/bla:.txt")
> +    "filename contains ':', which is reserved on Windows"
> +    >>> checkwinfilename("foo/bar/b\07la.txt")
> +    "filename contains '\\x07', which is invalid on Windows"
> +    >>> checkwinfilename("foo/bar/bla ")
> +    "filename ends with ' ', which is not allowed on Windows"
> +    '''
> +    for n in path.replace('\\', '/').split('/'):
> +        if not n:
> +            continue
> +        for c in n:
> +            if c in _windows_reserved_chars:
> +                return _("filename contains '%s', which is reserved "
> +                         "on Windows") % c
> +            if ord(c) <= 31:
> +                return _("filename contains '%s', which is invalid "
> +                         "on Windows") % c
> +        base = n.split('.')[0]
> +        if base and base.lower() in _windows_reserved_filenames:
> +            return _("filename contains '%s', which is reserved "
> +                     "on Windows") % base
> +        t = n[-1]
> +        if t in '. ':
> +            return _("filename ends with '%s', which is not allowed "
> +                     "on Windows") % t
> +
>  class path_auditor(object):
>      '''ensure that a filesystem path contains no banned components.
>      the following properties of a path are checked:
> @@ -560,6 +602,9 @@
>              prefixes.append(prefix)
>              parts.pop()
>  
> +        r = checkosfilename(path)
> +        if r:
> +            raise Abort("%s: %s" % (r, path))
>          self.audited.add(path)
>          # only add prefixes to the cache after checking everything: we don't
>          # want to add "foo/bar/baz" before checking if there's a "foo/.hg"
> @@ -577,6 +622,7 @@
>      pass
>  
>  if os.name == 'nt':
> +    checkosfilename = checkwinfilename
>      from windows import *
>  else:
>      from posix import *
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel


-- 
Mathematics is the supreme nostalgia of our time.