On Wed, 06 Feb 2019 06:54:02 -0500, Yuya Nishihara <y...@tcha.org> wrote:

On Tue, 05 Feb 2019 21:04:00 -0500, Matt Harbison wrote:
# HG changeset patch
# User Matt Harbison <matt_harbi...@yahoo.com>
# Date 1549417854 18000
#      Tue Feb 05 20:50:54 2019 -0500
# Branch stable
# Node ID 0e18c6ec895542394c0ad18c380bf3bbd4ba4d9b
# Parent  8b2892d5a9f2c06c998c977015a9ad3e3a3c9b5f
subrepo: avoid false unsafe path detection on Windows

diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -405,7 +405,7 @@ class hgsubrepo(abstractsubrepo):
         super(hgsubrepo, self).__init__(ctx, path)
         self._state = state
         r = ctx.repo()
-        root = r.wjoin(path)
+        root = os.path.realpath(r.wjoin(path))

Can we do r.wjoin(util.localpath(path)) instead? Even though symlinks and
".."s should be checked before, I want to avoid path resolution here for
extra safety.

Early indications are yes. If the rest of the tests pass, I'll resend. I'm not real sure what the reason for avoiding path resolution is, so feel free to add commentary to the patch if you queue it.

What I'm not certain is whether realpath() does normalize long/short names
and lower/upper case stuff. os.path.realpath() appears to call
GetFullPathName() on Windows, and I guess it wouldn't do such normalization,
but I'm not sure.

https://docs.microsoft.com/en-us/windows/desktop/api/fileapi/nf-fileapi-getfullpathnamea

It doesn't look like it changes either case:

C:\Users\Matt>py -2 -c "import os; print os.path.realpath('C:\PROGRAM FILES')"
C:\PROGRAM FILES

C:\Users\Matt>py -2 -c "import os; print os.path.realpath('C:\PROGRA~1')"
C:\PROGRA~1

C:\Users\Matt>py -2 -c "import os; print os.path.realpath('C:\PROGRA~1\..\PROGRA~1')"
C:\PROGRA~1

C:\Users\Matt>py -2 -c "import os; print os.path.realpath('C:\PROgRA~1\..\PROGRa~1')"
C:\PROGRa~1

C:\Users\Matt>py -2 -c "import os; print os.path.realpath('C:\WINDOWS')"
C:\WINDOWS

C:\Users\Matt>py -2 -c "import os; print os.path.realpath('C:\windows')"
C:\windows


I didn't have time to track this down, but I vaguely remember fixing some insane Windows path issue that somebody filed a bug about. It was something along the lines of:

    cd D:\path\to\repo_pool
    cd C:\Users\foo
    hg -R D:repo status

... and expecting that to expand to D:\path\to\repo_pool\repo. I don't remember if it was subrepo related, but I remember needing to use realpath() somewhere. There definitely aren't tests for it.
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to