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

tmp folder flooded with downloaded Font files (when using SFTP). #11

Open
gamecreature opened this issue Mar 7, 2021 · 3 comments
Open

Comments

@gamecreature
Copy link

Issue description:

When using an SFTP filesystem backend the tmp-file directory is flooded with dowloaded google font files
(When using SFTP without storing the password, Wordpress asks for password when updating plugins etc.)

Version used:

The version that's from the 'master branch'. It seems that's the latest version that is updated via Wordpress plugins.
(Develop branch is totally different)

Description:

(Copied from themeum/kirki#2375).
It seems this repository is the base of the webfont-loader

In class WPTT_WebFont_Loader (line 343)
The library tries to move the tmp file via the wp_filesystem

 // Move temp file to final destination.
 $success = $this->get_filesystem()->move( $tmp_path, $font_path, true );

This fails, because the SFTP cannot login. (and on the frontend no password-dialog is used)
This results in a tmp folder with endless numbers of files.

Hacky fix for now is to move it directly without the filesystem. (wp-content/fonts should be writable by the webserver)

 // Move temp file to final destination.
// $success = $this->get_filesystem()->move( $tmp_path, $font_path, true );
$succes = rename($tmp_path, $font_path);

This resolves my issue.
But it would be nice if it was possible to configure the library, to not use the wp-filesystem to move this temp file. Because in some cases the filesystem needs input from the administrator.

Maybe it should be configurable by a configuration switch?

@pattonwebz
Copy link
Member

Do you have a folder that you can actually write to on the filesystem? I had a similar issue when testing this as I was on a (mostly) unwritable system with just a select few locations I could store the fonts in. This was merged and was the solution for me: https://github.com/WPTT/webfont-loader/pull/6/files

@gamecreature
Copy link
Author

gamecreature commented Mar 8, 2021

@pattonwebz Problem is that the folder is writable on the system by the webserver.
But WPTT always tries the WP_Filesystem way to write the file. (Which in my case is password protected)

But you're remark inspired me.. Why not change the WPTT code like this?
(No setting required, just fallback to WP_Fileystem if no write access is available)

$success = is_writable(dirname($font_path)) ? rename($tmp_path, $font_path) : false;
if( !$success) $success = $this->get_filesystem()->move( $tmp_path, $font_path, true );

@aristath
Copy link

aristath commented Mar 8, 2021

This was done using the WP_Filesystem because at the time file operations were not (and still are not) allowed directly in themes. If I remember correctly they were even throwing errors in the theme-uploader so they could not be used.
We'll need to revisit this one and test if we can now use PHP file operations or not

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

No branches or pull requests

3 participants