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

[SOAP] Temporary WSDL cache files not being deleted #12838

Closed
Lyrkan opened this issue Nov 30, 2023 · 2 comments
Closed

[SOAP] Temporary WSDL cache files not being deleted #12838

Lyrkan opened this issue Nov 30, 2023 · 2 comments

Comments

@Lyrkan
Copy link

Lyrkan commented Nov 30, 2023

Description

Hi there,

After upgrading some servers from PHP 8.1.25 to 8.1.26 I started receiving alerts that their disk usage was increasing unusually fast due to thousands of tmp.wsdl.* files being created in the WSDL cache folder.

From what I've seen those temporary files were added in #12469 but I think it may be related to another issue I had before.

PHP scripts from my app are owned by an user called admin and may be executed by two different users :

  • admin (CLI)
  • www-data (PHP-FPM).

In both cases SOAP APIs may be called which will result in the WSDL cache being populated.
From my understanding, the filenames for those files depend on a php_get_current_user() call which doesn't return the process owner but the owner of the script, which means that in both cases (CLI or PHP-FPM) the files will have the same name.

Since - at least in my case - they are created in the /tmp directory with 600 permissions, only the owner of the process that was used to create them can also read them. So, I basically end-up with files called wsdl-admin-* owned by the first user that populated the WSDL cache (ex: admin) and not readable by the other one (ex: www-data). I'm not sure why it worked before 8.1.26, I'm guessing that it probably didn't but failed silently.

What I think happens next is that the new code from #12469 tries to move the tmp.wsdl.* files to wsdl-admin-<hash> but does not have the permission to do so.

So, two questions:

  • is there a reason the cache files are named based on the script file owner and not the PHP process owner?
  • shouldn't the tmp.wsdl.* files be removed if they could not be renamed?

PHP Version

PHP 8.1.26

Operating System

Ubuntu 20.04

@nielsdos
Copy link
Member

Thanks for the report! Your analysis seems right.

From my understanding, the filenames for those files depend on a php_get_current_user() call which doesn't return the process owner but the owner of the script, which means that in both cases (CLI or PHP-FPM) the files will have the same name.

It's actually even worse. In case the SAPI has set up a custom user to run under then it will return that user... So it's quite inconsistent. (except on Windows where it will always return the current user).

is there a reason the cache files are named based on the script file owner and not the PHP process owner?

Seems like an oversight to me. I'm not going to touch this for stable branches, but I'll take a look to fix this on master.

shouldn't the tmp.wsdl.* files be removed if they could not be renamed?

Yeah that should fix your issue.

I'm not sure why it worked before 8.1.26, I'm guessing that it probably didn't but failed silently.

Yeah the caching just didn't work before. This should be fixable by taking the right username, something that I'll look into for master. For stable branches I'll make sure the temp files are removed.

nielsdos added a commit to nielsdos/php-src that referenced this issue Nov 30, 2023
If there are two users that can execute the script that caches a WSDL,
but the script is owned by a single user, then the caching code will
name the cached file with the file owner username and a hash of the uri.
When one of the two tries to rename the file created by the other
process, this does not work because it has no permission to do so.
This then leaves temporary files floating in the temp directory.

To fix the immediate problem, unlink the file after rename has failed.
On the long term, this has to be fixed by taking the username of the
process instead of the username of the file owner.
@Lyrkan
Copy link
Author

Lyrkan commented Dec 1, 2023

Hi @nielsdos,

Thanks for taking care of it.

For now as a workaround I moved cache files to their own directory and handle permissions on them using ACLs, seems to do the trick :)

nielsdos added a commit that referenced this issue Dec 1, 2023
* PHP-8.2:
  Fix GH-12838: [SOAP] Temporary WSDL cache files not being deleted
nielsdos added a commit that referenced this issue Dec 1, 2023
* PHP-8.3:
  Fix GH-12826: Weird pointers issue in nested loops
  Fix GH-12838: [SOAP] Temporary WSDL cache files not being deleted
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants