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

[5.7] Insufficient permissions on packages.php #26232

Closed
wants to merge 1 commit into from
Closed

[5.7] Insufficient permissions on packages.php #26232

wants to merge 1 commit into from

Conversation

Yogarine
Copy link
Contributor

@Yogarine Yogarine commented Oct 24, 2018

Fix for #26230

tempnam() creates files with permissions set to 600, so we fix it by
just removing the temp file created by tempnam().

The alternative is to change permissions after creating the file, but that might not always be possible and also means we would have to figure out what the current umask() is to do it consistent with the umask.

`tempnam()` creates files with permissions set to 600, so we fix it by
just removing the temp file created by `tempnam()`.
@Yogarine Yogarine changed the title #26230 Insufficient permissions on packages.php [5.7 Insufficient permissions on packages.php Oct 24, 2018
@Yogarine Yogarine changed the title [5.7 Insufficient permissions on packages.php [5.7] Insufficient permissions on packages.php Oct 24, 2018
@Yogarine
Copy link
Contributor Author

@taylorotwell
Any explanation why reverting was chosen over fixing the specific regression?

@deleugpn
Copy link
Contributor

As mentioned in the revert PR, this fix should target 5.8, if accepted.

@Yogarine
Copy link
Contributor Author

@deleugpn

As mentioned in the revert PR, this fix should target 5.8, if accepted.

That was a suggestion made before it was clear that this was the actual fix. This issue was caused by an oversight when using tempnam() and not inherently an issue of the solution intended in the fix that was reverted.

@deleugpn
Copy link
Contributor

Taylor doesn't like to play trial and error. If something breaks production code, first it needs reverting and then re-examination.

@Yogarine Yogarine deleted the 26230-fix-insufficient-permissions-on-packages branch October 25, 2018 21:56
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