shutil.copyfile mutates symlink for absolute path

[jaraco]

BPO	40654
Nosy	@pfmoore, @jaraco, @tjguk, @zware, @eryksun, @zooba

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2020-05-25.19:49:29.830>
created_at = <Date 2020-05-17.03:26:35.390>
labels = ['invalid', 'type-bug', '3.8', 'OS-windows']
title = 'shutil.copyfile mutates symlink for absolute path'
updated_at = <Date 2020-05-28.12:25:46.901>
user = 'https://github.com/jaraco'

bugs.python.org fields:

activity = <Date 2020-05-28.12:25:46.901>
actor = 'eryksun'
assignee = 'none'
closed = True
closed_date = <Date 2020-05-25.19:49:29.830>
closer = 'steve.dower'
components = ['Windows']
creation = <Date 2020-05-17.03:26:35.390>
creator = 'jaraco'
dependencies = []
files = []
hgrepos = []
issue_num = 40654
keywords = ['3.8regression']
message_count = 18.0
messages = ['369091', '369094', '369123', '369124', '369202', '369212', '369232', '369233', '369616', '369617', '369624', '369628', '369630', '369916', '369946', '369964', '370034', '370198']
nosy_count = 6.0
nosy_names = ['paul.moore', 'jaraco', 'tim.golden', 'zach.ware', 'eryksun', 'steve.dower']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue40654'
versions = ['Python 3.8']

[jaraco]

In jaraco/path#186, the Path project discovered a regression with Python 3.8. It seems that if one creates a symlink with an absolute path. I used shutil.copytree('temp', 'temp2', True) and it produced this result:

~ # dir temp
 Volume in drive C has no label.
 Volume Serial Number is B8F4-40BB

 Directory of C:\Users\jaraco\temp

2020-05-16  11:05 PM    <DIR>          .
2020-05-16  11:05 PM    <DIR>          ..
2020-05-16  11:05 PM    <SYMLINK>      bar [c:\Users\jaraco\temp\foo]
2020-05-16  11:04 PM                 0 foo
               2 File(s)              0 bytes
               2 Dir(s)  17,495,805,952 bytes free
~ # dir temp2
 Volume in drive C has no label.
 Volume Serial Number is B8F4-40BB

 Directory of C:\Users\jaraco\temp2

2020-05-16  11:05 PM    <DIR>          .
2020-05-16  11:05 PM    <DIR>          ..
2020-05-16  11:06 PM    <SYMLINK>      bar [\\?\c:\Users\jaraco\temp\foo]
2020-05-16  11:04 PM                 0 foo
               2 File(s)              0 bytes
               2 Dir(s)  17,495,846,912 bytes free

As you can see, in the copy, bar has an additional \\?\ prefix on the symlink path. On Python 3.7 and earlier, the copy was made without mutating the metadata.

[eryksun]

Copying a symlink verbatim requires copying both the print name and the substitute name in the reparse buffer 1. For a file, CopyFileExW: COPY_FILE_COPY_SYMLINK implements this by enabling the symlink privilege for the thread and copying the reparse point via FSCTL_GET_REPARSE_POINT and FSCTL_SET_REPARSE_POINT. For a directory, CreateDirectoryExW is implemented similarly when lpTemplateDirectory is a symlink or mount point. (For a "\\\\?\\Volume{GUID}\\" volume mountpoint as opposed to a bind mountpoint, CreateDirectoryExW punts to SetVolumeMountPointW, which also updates the system mountpoint manager.)

If you can only have one or the other, the substitute name is more reliable according to the wording in [MS-FSCC] 2.

symlinks:

A symbolic link has a substitute name and a print name associated 
with it. The substitute name is a pathname (section 2.1.5) 
identifying the target of the symbolic link. The print name SHOULD 
be an informative pathname, suitable for display to a user, that 
also identifies the target of the symbolic link. Either pathname 
can contain dot directory names as specified in section 2.1.5.1. 

mount points (junctions):

A mount point has a substitute name and a print name associated 
with it. The substitute name is a pathname (section 2.1.5) 
identifying the target of the mount point. The print name SHOULD 
be an informative pathname (section 2.1.5), suitable for display 
to a user, that also identifies the target of the mount point. 
Neither of these pathnames can contain dot directory names.

The operative weasel word is "should", instead of a reliable "must" (RFC2119).

An example of the power of "should" is that PowerShell doesn't even set a print name when it creates a mount point via New-Item -ItemType Junction. I don't agree that nt.readlink should read junctions, but it does, so the potentially missing print name is a problem. If it were just symlinks created by CreateSymbolicLinkW, the print name is reliable because we know that it sets the print name to whatever was passed as lpTargetFileName. I suppose nt.readlink could fall back on using the substitute name if there's no print name.

Also, if nt.readlink is used to manually resolve a broken path (e.g. ntpath._readlink_deep), and the process doesn't have long paths enabled, then the "\\?\" extended path from the substitute name is more reliable. (But one could also call _getfullpathname on the print name and convert the result to extended form if it's not already an extended path.)

If you search around, you'll find some projects using the print name and some using the substitute name to implement POSIX readlink, but using the print name is more popular.

Do you want 3.8 to revert to using the print name, at least for symlinks? (ntpath._readlink_deep would need to be modified to support long target paths.) Or would you rather that shutil used a more reliable way to copy symlinks verbatim on Windows? For example, use CopyFileExW for a file and CreateDirectoryEx for a directory.

[jaraco]

Thank you Eryk for the thorough and informative investigation. Seriously, wow.

Do you want 3.8 to revert to using the print name, at least for symlinks? (ntpath._readlink_deep would need to be modified to support long target paths.) Or would you rather that shutil used a more reliable way to copy symlinks verbatim on Windows? For example, use CopyFileExW for a file and CreateDirectoryEx for a directory.

In this case, my instinct is that shutil should devise a more reliable way to copy symlinks. It claims that it does to the extent the platform allows 1.

symbolic links in the source tree are represented as symbolic links in the new tree and the metadata of the original links will be copied as far as the platform allows

It hadn't occurred to me that the effect may be manifest in readlink, but I see that now too:

~ # py -3.7 -c "import os; print(os.readlink('temp/bar'))"
c:\Users\jaraco\temp\foo
~ # py -3.8 -c "import os; print(os.readlink('temp/bar'))"
\\?\c:\Users\jaraco\temp\foo

So even if shutil.copyfile were to copy the symlinks exactly, the expectation would still be missed that the output of readlink doesn't match the input to symlink, so the expectation would still be missed. A user/programmer should be able to predict the output of 'readlink' given the input to 'symlink' (or cmd /c mklink).

Based on that reasoning, my answer would be "both", but I'd put a priority on restoring the use of "print name" for readlink, as that may be sufficient to satisfy most (if not all) use-cases.

[jaraco]

I strongly suspect bpo-37834 and #59436 is where the difference was introduced.

[zooba]

This is only an issue for broken symlinks, right? (More precisely, those that cannot be resolved, which may include very long paths on some machines.)

Fixing copy is my preferred option.

Also Jason, what do you need to be able to test against prerelease? It would be nice to find these impacts during alpha/beta rather than three bug fix releases later.

[jaraco]

This is only an issue for broken symlinks, right? (More precisely, those that cannot be resolved, which may include very long paths on some machines.)

Unfortunately, no. In the original post above, you can see temp/bar points to C:\Users\jaraco\temp\foo, which exists. Yet os.readlink returns \\?\C:\Users\jaraco\temp\foo. That's the root cause of the issue with copytree.

what do you need to be able to test against prerelease? It would be nice to find these impacts during alpha/beta rather than three bug fix releases later.

I need a mechanically-reproducible technique to test dozens of projects against pre-releases on Windows. Historically, I've had Travis as my prime CI (non-existent Windows support) and AppVeyor for select projects that demanded some Windows testing. I've recently started migrating to Azure Pipelines, which has nice multi-platform support and that's why I've started catching these issues. Presumably, I'll be able to add a pre-release of Python 3.9 to the pipelines config at jaraco/skeleton - that will ensure that all projects I maintain going forward will get tested on the pre-release.

[zooba]

Ah right, it's realpath() that has the final step to remove an unnecessary \\?\ prefix.

Firstly, I still think fixing copy is more important and more valuable.

Given the possibility that the print name won't point to the real target, I'd much prefer to keep reading the substitute name. In this case, this looks like a change between versions (3.7->3.8) rather than a bug (assuming the symlink works, which hasn't been mentioned yet), and while it does result in needing a test update in Path, I don't think it makes sense to push that fix upstream. Changing readlink to always return the correct path was deliberate.

Unless you're specifically testing single steps through symlink chains, you probably want to just use realpath anyway. Presumably you weren't using that beforehand because it didn't work, but the same fix fixed that, and we do extra work to retain the existing behaviour of the higher-level function (including rejecting junctions, unlike readlink).

[zooba]

Presumably, I'll be able to add a pre-release of Python 3.9 to the pipelines config at jaraco/skeleton - that will ensure that all projects I maintain going forward will get tested on the pre-release.

I don't have a straightforward task for it, but these should give you a Python 3.9 prerelease version (or any version you like if you replace "-Prerelease" with "-Version x.y.z", IIRC):

nuget install python -Prerelease -OutputDirectory . -ExcludeVersion
.\python\tools\python.exe ...

nuget install pythonx86 -Prerelease -OutputDirectory . -ExcludeVersion
.\pythonx86\tools\python.exe ...

[jaraco]

Changing readlink to always return the correct path was deliberate.

Understood. However, this statement assumes the "correct path" is the most precise path to resolve the target. If you instead define "correct path" as the one that would be most friendly to the user who created the path, readlink no longer honors that expectation. With this change, the following invariant holds on every platform except Python 3.8 on Windows (at least in the general case):

>> os.symlink(x, y)
>> assert os.readlink(y) == x

More importantly, AFAIK, Python provides no function to transform x into what one can expect as the result of os.readlink(y). In other words, what value of f would make this invariant pass?

>> os.symlink(x, y)
>> assert os.readlink(y) == f(x)

Or put another way, is "C:\Users\jaraco\temp" an "incorrect path"?