[PATCH STABLE] subrepo: avoid false unsafe path detection on Windows

[Matt Harbison]

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

> On Tue, 05 Feb 2019 21:04:00 -0500, Matt Harbison wrote:
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison at 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.