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

Tar: for directories, use a default mask that has the 'x' bit. #71760

Merged
merged 3 commits into from
Jul 8, 2022

Conversation

tmds
Copy link
Member

@tmds tmds commented Jul 7, 2022

Without the 'x'-bit, the directories that get created are not accessible.

@eerhardt @carlossanlop @dotnet/area-system-io ptal.

Without the 'x'-bit, the directories that get created are not
accessible.
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Jul 7, 2022
@ghost
Copy link

ghost commented Jul 7, 2022

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

Without the 'x'-bit, the directories that get created are not accessible.

@eerhardt @carlossanlop @dotnet/area-system-io ptal.

Author: tmds
Assignees: -
Labels:

area-System.IO

Milestone: -

@stephentoub
Copy link
Member

Without the 'x'-bit, the directories that get created are not accessible.

Do we need a test that untars something containing a directory and then tries to access it, e.g. copying something inside it, reading something from it, etc.? It sounds like that would fail without this fix?

@tmds
Copy link
Member Author

tmds commented Jul 7, 2022

Do we need a test that untars something containing a directory and then tries to access it, e.g. copying something inside it, reading something from it, etc.? It sounds like that would fail without this fix?

Yes, it would fail without the fix.
The existing tests have the expected permission hard coded, so they'll also fail if this regresses.

Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

:shipit:

@carlossanlop
Copy link
Member

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@carlossanlop carlossanlop left a comment

Choose a reason for hiding this comment

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

LGTM. I ran the runtime-extra-platforms pipeline, to make sure all the exotic platforms pass too.
If it all looks good, we can merge it.

@carlossanlop
Copy link
Member

No tar failures. :shipit:

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Formats.Tar community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants