Skip to content

Conversation

@niw
Copy link
Contributor

@niw niw commented Aug 10, 2020

Problem

Data.write(to:) is a only method in the Foundation that can create a
regular file.
However, it ignores umask and always set 0600 permission unlike
macOS Foundation, which respects process umask.

Solution

  1. With .atomic write option

    It uses mkstemp(3) in _NSCreateTemporaryFile, which is always
    creating a file with 0600 permission, if the system follows
    the latest POSIX specification or the permission is undefined.

    On macOS Foundation, therefore _NSCreateTemporaryFile uses
    mktemp(3) and open(2) instead to respect umask.

  2. Without .atomic write option

    It uses 0o600 even if it uses open(2) that respects umask.
    Simply gives 0o666 instead.

This is a bug caused by previous commit in
#1876.

Swift JIRA is https://bugs.swift.org/browse/SR-13307.

**Problem**

`Data.write(to:)` is a only method in the Foundation that can create a
regular file.
However, it ignores `uamask` and always set 0600 permission unlike
macOS Foundation, which respects process `umask`.

**Solution**

1. With `.atomic` write option

It uses `mkstemp(3)` in `_NSCreateTemporaryFile`, which is always
creating a file with 0600 permission, if the system follows
the latest POSIX specification or the permission is undefined.

On macOS Foundation, therefore `_NSCreateTemporaryFile` uses
`mktemp(3)` and `open(2)` instead to respect `umask`.

2. Without `.atomic` write option

It uses `0o600` even if it uses `open(2)` that respects `umask`.
Simply gives `0o666` instead.

This is a bug caused by previous commit in
swiftlang#1876.

JIRA: https://bugs.swift.org/browse/SR-13307
@spevans
Copy link
Contributor

spevans commented Aug 10, 2020

Could you add a regression test that validates that a file is created with the correct permissions, thanks

@spevans
Copy link
Contributor

spevans commented Aug 10, 2020

@swift-ci test

@niw
Copy link
Contributor Author

niw commented Aug 10, 2020

@spevans Sure! the test need to change process’s umask which is global therefore it need to not be reentrant.

@millenomi
Copy link
Contributor

@swift-ci please test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants