gh-156210: Fix shutil.copytree() detection of dangling relative symlinks - #156214
gh-156210: Fix shutil.copytree() detection of dangling relative symlinks#156214lpyu001 wants to merge 1 commit into
shutil.copytree() detection of dangling relative symlinks#156214Conversation
shutil.copytree() detection of dangling relative symlinks
picnixz
left a comment
There was a problem hiding this comment.
I'm not sure this is correct. Please explain why you used srcname instead of linkto. AFAICT, the problem is linkto is the incorrect file (because readlink() resolves according to PWD and not to src).
Otherwise, I wonder whether this is deliberate and instead should be documented.
| else: | ||
| # ignore dangling symlink if the flag is on | ||
| if not os.path.exists(linkto) and ignore_dangling_symlinks: | ||
| if not os.path.exists(srcname) and ignore_dangling_symlinks: |
There was a problem hiding this comment.
I'm not sure this is correct though? srcname is the the symlink file. However, we want to resolve the relative symlink with respect to src, that is, we read the symlink content and then check if the file exists. The problem is therefore not here but rather the computation of "linkto". Instead, we should readlink(srcname) by interpreting srcname's content as relative to the src rather than PWD.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
shutil.copytree()used the raw result ofos.readlink()to determine whether a symbolic link was dangling whenignore_dangling_symlinks=True. Relative targets were therefore checked against the current working directory instead of the link's parent directory.This could cause valid relative symbolic links to be skipped, or dangling links to be copied when the current working directory contained a matching path.
The check now uses the source link path, allowing the target to be resolved from the correct directory. Regression tests cover both cases.
shutil.copytree()incorrectly classifies relative symlinks as dangling #156210