Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8.0.2 doesn't validate paths correctly #2088

Closed
dylwylie opened this issue Oct 9, 2021 · 2 comments
Closed

8.0.2 doesn't validate paths correctly #2088

dylwylie opened this issue Oct 9, 2021 · 2 comments
Assignees
Milestone

Comments

@dylwylie
Copy link

dylwylie commented Oct 9, 2021

Path arguments with a directory fail validation with "file not exists", despite existing, when resolve_path=True is specified

It looks like the code here concats the passed-in-path to its resolved directory, which means if the path contains a directory, the directory part gets duplicated

rv = os.path.join(dir_, rv)

e.g.

$ touch relative-dir/myfile
$ python3
Python 3.9.7 (default, Sep 28 2021, 18:41:28)
[GCC 10.2.1 20210110] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from click import Path
>>> p = Path(dir_okay=False, exists=True, writable=False, resolve_path=True)
>>> p.convert("relative-dir/myfile", None, None)
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/site-packages/click/types.py", line 855, in convert
    st = os.stat(rv)
FileNotFoundError: [Errno 2] No such file or directory: '/relative-dir/relative-dir/myfile'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.9/site-packages/click/types.py", line 859, in convert
    self.fail(
  File "/usr/local/lib/python3.9/site-packages/click/types.py", line 128, in fail
    raise BadParameter(message, ctx=ctx, param=param)
click.exceptions.BadParameter: File 'relative-dir/myfile' does not exist.

Environment:

  • Python version: 3.9.7
  • Click version: 8.0.2
@dylwylie dylwylie changed the title 8.0.2 doesn't validate paths correclty 8.0.2 doesn't validate paths correctly Oct 9, 2021
@davidism davidism added this to the 8.0.3 milestone Oct 10, 2021
@davidism
Copy link
Member

Seems that the fix for resolving across symlinks caused this. See #1813 (original discussion), #1825 (reverted), #1921 (next discussion), #2006 (currently applied), #2046.

@davidism
Copy link
Member

davidism commented Oct 10, 2021

Now that I look at the current code again, os.readlink really isn't a replacement for os.path.realpath the way it's used there, it's only being applied to one directory, not the entire path. Another suggestion was to use pathlib.Path(rv).resolve() instead, which has a different implementation than realpath which behaves the same on all supported Python versions. Pathlib has some overhead, but I think it's the best solution until we only support Python >= 3.8.

@davidism davidism self-assigned this Oct 10, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants