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.

Sign up for GitHub

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

Ignore sticky bit when validating permissions. #177

Merged
merged 3 commits into from
Nov 16, 2019
Merged

Ignore sticky bit when validating permissions. #177

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1168fa6
Ignore sticky bit when validating permissions.
WibbletheDuck Nov 1, 2019
5d623f1
Fix and add to comment, fix error message.
WibbletheDuck Nov 1, 2019
5ca11a7
Change 'octet' to 'octal digit' in comments
WibbletheDuck Nov 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion jupyter_core/paths.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ def get_file_mode(fname):
# Some filesystems (e.g., CIFS) auto-enable the execute bit on files. As a result, we
# should tolerate the execute bit on the file's owner when validating permissions - thus
# the missing one's bit on the third octet.
return stat.S_IMODE(os.stat(fname).st_mode) & 0o7677 # Use 4 octets since S_IMODE does the same
return stat.S_IMODE(os.stat(fname).st_mode) & 0o6677 # Use 4 octets since S_IMODE does the same
Copy link
Member

Choose a reason for hiding this comment

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

I agree we should ignore the sticky bit. Wondering if we shouldn't just ignore them all in that octet, but that's a larger discussion.

If we're only removing the sticky bit from the mask, then the comment should be updated to state why. I also had a brain cramp when I added the previous comment and we should change the wording of "one's bit" to "least significant bit" while we're here. So perhaps something like...

...thus the missing least significant bit on the third octet. In addition, we also tolerate the sticky bit being set, so the lsb from the fourth octet is also removed.

The other "repair" ask I have (sorry), is to fix the error message at the end of file. Could you please add some spacing before "Got" and put a period at the end of its sentence as well? That's been bugging me in the recent logs I've seen. 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 ?

Copy link
Member

Choose a reason for hiding this comment

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

I'm going to go super nitpicky about the comments: I think we're using 'octet' inaccurately. An octet is 8 bits, aka a byte. Digits in octal represent 3 bits each. So I think we should be using the term 'octal digit' instead.

(I know this was already there, but I think it's worth fixing while we're looking at this bit of code anyway)

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, you're right - my bad. @WibbletheDuck - would you mind fixing up the occurrences of octet as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kevin-bates Done!

Copy link
Member

Choose a reason for hiding this comment

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

Thank you!



@contextmanager
Expand Down