Improve handling of mode bits #250
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- "/" should have mode `os.ModeDir|0755`, not `0000`. Among other things, this had resulted in `mode.IsDir()` returning false for root prior to this patch. - `Mkdir`, `MkdirAll`, and `OpenFile` shouldn't be allowed to set permissions that are otherwise illegal through `Chmod`. This mirrors what Go's `os` package does: it calls `syscallMode(mode)`, which effectively clears out the same bits that are disallowed by `Chmod`. - `MkdirAll` should use the given permissions for all intermediate directories that are created, not just for the final directory. Prior to this patch, intermediate directories were created with mode bits `0000`. Besides the permission bits being wrong, `mode.IsDir()` would return false for these directories prior to this patch.
|
|
|
Nice, this is a great follow-up to the illegal chmod fix. (I'd approve if I could, so poking @Kargakis) Interesting tidbit, Go itself doesn't support some of those bits for |
0xmichalis
approved these changes
Aug 2, 2020
|
Released in v1.3.3 |
anishathalye
added a commit
to anishathalye/periscope
that referenced
this pull request
Aug 2, 2020
Afero 1.3.3 includes a patch (spf13/afero#250) that fixes some bugs that we were previously working around in test-related code.
anishathalye
added a commit
to anishathalye/periscope
that referenced
this pull request
Aug 2, 2020
Afero 1.3.3 includes a patch (spf13/afero#250) that fixes some bugs that we were previously working around in test-related code.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
os.ModeDir|0755, not0000. Among other things, this had resulted inmode.IsDir()returning false for root prior to this patch.Mkdir,MkdirAll, andOpenFileshouldn't be allowed to set permissions that are otherwise illegal throughChmod. This mirrors what Go'sospackage does: it callssyscallMode(mode), which effectively clears out the same bits that are disallowed byChmod.MkdirAllshould use the given permissions for all intermediate directories that are created, not just for the final directory. Prior to this patch, intermediate directories were created with mode bits0000. Besides the permission bits being wrong,mode.IsDir()would return false for these directories prior to this patch.A potentially amusing side note: I was working around these issues in my code by abusing the buggy
fs.Chmod(), but that was fixed in #249.