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

[4.4] Fix permission issues on certain ISP #43473

Merged
merged 3 commits into from
May 16, 2024

Conversation

bembelimen
Copy link
Contributor

@nikosdion reported the following issue

Summary of Changes

I am seeing a problem with Kickstart extracting backup archives and Joomla's installation ZIP file on some cPanel hosts (e.g. Rochen). This seems to have started the last couple of days, and only affects recent versions of PHP under active support (8.1, 8.2, and 8.3).

The problem I see is that PHP reports the file we're trying to write into is not writeable, even though it most definitely can be written to. This happens around the first few files included in the ZIP file.

I traced the problem on two affected hosts and found it happens because we change the permissions of the still-empty file, right before writing into it. In theory, this shouldn't be a problem. PHP should update its file stat cache upon changing the permissions, therefore know the file exists and is writeable. On the affected hosts it wasn't the case. Removing the line changing the permissions worked (removing a useful fefature), and so did adding a clearstatcache() after that line (which is the right way to solve it).

Joomla! Update uses the same code, line for line, but I have not been able to reproduce the issue with it. Still, I think that this problem MAY occur under conditions I do not yet know, preventing people from updating their sites. Since it looks like it's very timing-sensitive it might explain why there have been some users reporting a failure to update with a message about files not being able to be written to, even though their host tells them this is not the case. Maybe their host isn't lying to them, maybe this is a bug that's been very hard to reproduce.

In any case, I would ask you to find the two lines reading

$this->setCorrectPermissions($this->fileHeader->file);

in extract.php and add

clearstatcache($this->fileHeader->file);

after them. It doesn't hurt performance in any measurable way, and it makes sure that any PHP file stat cache shenanigans won't get in the way of Joomla! Update.

Testing Instructions

Code review

@brianteeman
Copy link
Contributor

Would it be a good idea to add a comment with this code to explain its purpose especially as rthe rest of this file is very well documented?

This file is changed during the script's operation so we clear the status cache.

@bembelimen
Copy link
Contributor Author

Would it be a good idea to add a comment with this code to explain its purpose especially as rthe rest of this file is very well documented?

This file is changed during the script's operation so we clear the status cache.

Thanks, done

@MacJoom MacJoom self-assigned this May 15, 2024
@MacJoom MacJoom added this to the Joomla! 4.4.5 milestone May 15, 2024
@MacJoom MacJoom merged commit 638df97 into joomla:4.4-dev May 16, 2024
3 checks passed
@MacJoom
Copy link
Contributor

MacJoom commented May 16, 2024

Thank you!

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

Successfully merging this pull request may close these issues.

None yet

5 participants