[PATCH] opener: forbid paths ending with directory separator (issue2507)

[Mads Kiilerich]

On 12/02/2010 11:06 AM, Jim Hague wrote:
> # HG changeset patch
> # User Jim Hague<jim.hague at acm.org>
> # Date 1291223557 0
> # Branch stable
> # Node ID b0da768292e2064dde079544806f2f464a747b1e
> # Parent  417f3c27983bcb6ab95b169801edc63c5c138dcf
> opener: forbid paths ending with directory separator (issue2507)
>
> If Linux is asked to open a filename with a trailing directory separator,
> e.g. "foo/", the open fails with EISDIR. On AIX, the open succeeds, opening
> file "foo". This causes test-mq-qnew to fail on AIX.
>
> Fix by adding 'ends with directory separator' to the conditions checked
> by the path auditor. Change test to expect auditor fail message.
>
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -488,6 +488,7 @@
>       '''ensure that a filesystem path contains no banned components.
>       the following properties of a path are checked:
>
> +    - ends with a directory separator
>       - under top-level .hg
>       - starts at the root of a windows drive
>       - contains ".."
> @@ -505,6 +506,9 @@
>       def __call__(self, path):
>           if path in self.audited:
>               return
> +        # AIX ignores "/" at end of path, others raise EISDIR.
> +        if path[-1] == os.sep or path[-1] == os.altsep:
> +            raise Abort(_("path ends in directory separator: %s") % path)

Some minor nits:

Are you sure that path isn't empty? I would expect this function to 
stable and not crash with index error in that case.

AFAIK it is best practice (and faster) to use endswith:
     path.endswith(os.sep) or path.endswith(os.altsep)

Instead of multiple tests it might be better to make one test like
     path[-1] in [os.sep, os.altsep]
or
     path[-1:] in [os.sep, os.altsep]

/Mads