ahmedkaludi / wp-multilang

16 stars 3 forks source link

Out of memory error after update to 2.4.11 #111

Open flack opened 1 week ago

flack commented 1 week ago

Since updating to 2.4.11, I get out of memory errors in my dev environment, I have temporarily removed my memory limit and profiled this shortly with XDebug, which showed that the function wpm_local_file_get_contents is called 314820 times during a single request.

I've added a var_dump in WPM_Config::parse_config_files and that shows that the list of config files is growing with each call, because it keeps adding the same file over and over again, e.g.:


array (size=40)
  0 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  'wordpress-seo' => string '/wp-content/plugins/wp-multilang/configs/plugins/wordpress-seo.json' (length=86)
  1 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  2 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  3 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  4 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  5 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  6 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  7 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  8 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  9 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  10 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  11 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  12 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  13 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  14 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  15 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  16 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  17 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  18 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  19 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  20 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  21 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  22 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  23 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  24 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  25 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  26 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  27 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  28 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  29 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  30 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  31 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  32 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  33 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  34 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  35 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  36 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  37 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)
  38 => string '/wp-content/plugins/wp-multilang/core-config.json' (length=68)

Not sure how this is actually supposed to work (there's not really any useful commit messages...), but something seems broken.

(Tested on PHP 8.3.6)

saddy001 commented 1 week ago

Hello @flack ,

do you know of a simple check from the command line if I'm affected too?

flack commented 1 week ago

So I've debugged this a bit further, the problem comes from here:

https://github.com/ahmedkaludi/wp-multilang/blame/ae57e486239cc6524103eb6ad75b93bfc3a53a28/includes/wpm-config-functions.php#L109

WP_Filesystem() returns false, because there are no FTP settings. I have define('FS_METHOD', 'direct'); in my wp-config.php, but I had it at the end of the file. When I move the line to be above require_once( ABSPATH . 'wp-settings.php' ), the problem disappears.

I haven't checked too deeply, but it seems wrong to use $wp_filesystem here, because why would you want to load local config files over FTP? $file_path is a complete local path at this point, so you might as well just use file_get_contents.

flack commented 1 week ago

@saddy001 so I guess you would be affected if your wp-config is missing the define('FS_METHOD', 'direct'); line or has it in the wrong place

Sanjeevsetu commented 1 week ago

Hi @flack & @saddy001

Thanks for the bug report. Actually, We are force to use WordPress file system. As per WordPress coding standards we must avoid use of file_get_content that's why we made changes.

The issue you are facing We will try to re create it and get back to you.

flack commented 1 week ago

@Sanjeevsetu well, it uses file_get_contents either way, see

https://github.com/WordPress/wordpress-develop/blob/0b8b80449fb25e0242ad53262fcbabc08ea3ecb9/src/wp-admin/includes/class-wp-filesystem-direct.php#L39

(and in their version it's even a bit unsafer I'd say, because they suppress errors)

flack commented 1 week ago

or if you want to use WP API, you could try this:

$filesystem = new WP_Filesystem_Direct( true );

$filesystem->get_contents($path)
flack commented 1 week ago

also, see

https://github.com/WordPress/WordPress-Coding-Standards/pull/1374

WP coding standard allows file_get_contents for local files, only for URLs it's not allowed. So if you get a warning, it's probably a bug in the code analyzer