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

file: Raise error on permission denied #57574

Open
wants to merge 2 commits into
base: devel
Choose a base branch
from
Open

file: Raise error on permission denied #57574

wants to merge 2 commits into from

Conversation

@Jakski
Copy link

@Jakski Jakski commented Jun 8, 2019

SUMMARY

Fixes #57573.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

file module

ADDITIONAL INFORMATION

os.path.lexists won't report error when process has no permission to access path. See: https://github.com/python/cpython/blob/e119b3d136bd94d880bce4b382096f6de3f38062/Lib/posixpath.py#L174. It means that inaccessible paths will be treated as absent.

@Jakski
Copy link
Author

@Jakski Jakski commented Sep 16, 2019

This fix is waiting for approval more than month. Should I change it somehow before it can be considered good for merging?

Loading

@Akasurde
Copy link
Member

@Akasurde Akasurde commented Dec 25, 2019

@Jakski Thanks for the PR. I added this PR in the next IRC meeting ansible/community#507. Please make sure you attend the meeting and get reviews for this PR.

Loading

Copy link
Member

@samdoran samdoran left a comment

Please create a changelog fragment and integration tests. See this fragment as an example.

Loading

@@ -296,7 +296,7 @@ def get_state(path):

b_path = to_bytes(path, errors='surrogate_or_strict')
try:
if os.path.lexists(b_path):
if os.lstat(b_path):
Copy link
Member

@samdoran samdoran Jan 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not quite the correct fix. We probably need to use something os.access() to check if the file exists and if we can access it:

if os.access(b_path, os.F_OK):  # checks that the file exists, following symlinks
    if not os.access(b_path, os.R_OK):  # check that we can read the file
        # probably warn, for now, then change to an error later

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

5 participants