Fatal function declaration errors in a WordPress environment caused by dual usage of Safe by both third party plugins and first party theme

[SebastianAwatramani]

This is similar to #422 but I want to raise the issue because it seems to be an inherent issue with the architecture of the package that potentially prohibits its use in platforms that use third party plugins.

~The basic issue is caused by the fact that this package's internal composer's files autoloads a series of files any time that a function from the package is used. In my case, our theme use's Safe's json_encode, and a third party plugin uses Safe's hex2bin, mb_str_split, openssl_decrypt, and openssl_encrypt~ (see EDIT2 below).

The fatal error, however, is triggered by attempting to redeclare Safe's mysqli_get_client_stats. But as neither the plugin nor the theme use this function, I surmised that it was the simple inclusion of those files from files that actually triggers the error.

~That said, strictly speaking, even if Safe was re-architected such that it didn't include all those files automatically~, it would not solve the issue in toto. I.e. while it would allow two independent usages of the package, I believe it would still trigger errors if the same function was used by (in my case) the theme and a plugin.

While on the one hand, the solution in my case might be to simply not use the package in my theme, on the other hand this has the potential to lead to a much bigger issue within certain platforms. Namely, Safe is becoming popular (which it should because it's an awesome package), which means it might become common for multiple third party plugins to utilize it, which would then lead to a situation in which various plugins become incompatible simply because they use this package, which after a lot of annoyance for a lot of developers, would turn them off from using it, which would be a shame because, I reiterate, this package is awesome.

My first thought was that perhaps the easiest fix would be to wrap your function definitions in if(!function_exists('Safe\function_name)) {...} but I'm not sure if there are things about that solution that I'm not considering?

At any rate, thanks for you work so far. I think my current problem will probably have to be solved by removing Safe from my base project and hoping that another plugin does not include it.

EDIT: originally linked to wrong similar issue, changed #253 -> #422

EDIT2: reading through some older issues, I see why files is necessary, so really the only potentially actionable solution in my post regards checking if the function_exists (if that's feasible).

EDIT3: verbiage

[shish]

Do you have a minimal example of failing code so we can see what you're trying to do?

As far as I understand composer's autoload mechanism, files listed in files should be loaded once at startup, and not a second time after that (no matter how many different packages or plugins reference Safe, it should only ever be loaded once). If you're seeing errors about functions being redeclared, that means that something somewhere is bypassing the autoloader and attempting to manually include Safe's files multiple times, which is something one shouldn't do (and if I'm understanding the problem, any library would fail in this manner if it were loaded in this manner, nothing Safe-specific about it).

[agibson-godaddy]

Hi :) Just wanted to chime in here because we are also seeing the reported bug in WordPress plugins.

As far as I understand composer's autoload mechanism, files listed in files should be loaded once at startup, and not a second time after that (no matter how many different packages or plugins reference Safe, it should only ever be loaded once). If you're seeing errors about functions being redeclared, that means that something somewhere is bypassing the autoloader and attempting to manually include Safe's files multiple times, which is something one shouldn't do (and if I'm understanding the problem, any library would fail in this manner if it were loaded in this manner, nothing Safe-specific about it).

The above reply is absolutely right in a more typical composer set up where you have one main project that requires dependencies. If the main project requires thecodingmachine/safe and another one of your dependencies also requires it, then composer will sort the problem out.

However, WordPress is different in that each plugin is its own standalone project that has its own separate composer dependencies and own separate autoload file. These then all get loaded at the same time.

Now again, this isn't always a problem because if the hashes in vendor/composer/autoload_files.php do not change (based on package name + file path, and where the function is declared does not change, then composer will only load each file once. But you can run into issues if:

1. You update your package to move files around, or move a function definition from one file to another (this results in file hashes changing and thus potentially having the same contents of a file loaded twice via two different files); and
2. Two different plugins include two different versions of your package.

Now I understand that with point 2 above there are always going to be issues. It's a common thing in WordPress and I don't expect you to solve that. But IMO as per the recommended issue, one easy solution is to avoid a very nasty fatal error at the very least by adding ! function_exists() checks.

Do you have a minimal example of failing code so we can see what you're trying to do?

I've worked this up using two super simple WordPress plugins.

Plugin 1

Structure:

safetest1/
   - composer.json
   - safetest1.php

composer.json file:

{
    "name": "agibson/safetest1",
    "description": "test",
    "type": "library",
    "require": {
        "thecodingmachine/safe": "3.1.1"
    },
    "license": "MIT",
    "authors": [
        {
            "name": "Ashley",
            "email": "[email protected]"
        }
    ],
    "minimum-stability": "stable",
    "config": {
        "autoloader-suffix": "SafeTest2"
    }
}

safetest1.php file:

<?php
/**
 * Plugin Name: Safe Test 1
 */

require_once __DIR__.'/vendor/autoload.php';

Plugin 2

Structure:

safetest2/
   - composer.json
   - safetest2.php

composer.json file:

{
    "name": "agibson/safetest2",
    "description": "test",
    "type": "library",
    "require": {
        "thecodingmachine/safe": "^2.0"
    },
    "license": "MIT",
    "authors": [
        {
            "name": "Ashley",
            "email": "[email protected]"
        }
    ],
    "minimum-stability": "stable",
    "config": {
        "autoloader-suffix": "SafeTest2"
    }
}

Main thing of note here is that I'm including v2.0 of your package instead of 3.x.

safetest2.php file:

<?php
/**
 * Plugin Name: Safe Test 2
 */

require_once __DIR__.'/vendor/autoload.php';

---

If you install dependencies and activate both plugins you get this error:

PHP Fatal error: Cannot redeclare Safe\array_replace_recursive() (previously declared in /home/agibson/www/plugins/wp-content/plugins/safetest1/vendor/thecodingmachine/safe/generated/8.2/array.php:76) in /home/agibson/www/plugins/wp-content/plugins/safetest2/vendor/thecodingmachine/safe/deprecated/array.php on line 32

[SebastianAwatramani]

@agibson-godaddy's comment (thanks) basically sums it up so I don't think it would help for me to add any further examples but I am happy to if it would help.

[TheCelavi]

This also applies when using this library: https://github.com/bamarni/composer-bin-plugin, same hell occurs.

[theofidry]

I don't think the "common" (as, non WP-specific) packages should try to accommodate this setup. This is basically asking package maintainers to help getting around working in a non Composer project (technically yes, but not in the traditional sense). It's not a spit at the WP community, but this is the wrong fix.

Indeed, the issue is direct an obvious when dealing with with packages like this one as you have a hard failure about the function being declared twice, but you will run into much more subtle and difficult issue when dealing with classes. For instance you can end up with having the same class but with different code and only one will be loaded, the other one will be silently not loaded, and what kind of bugs you get from there can be pretty nasty.

Likewise bamarni/composer-bin-plugin is not meant to get around this issue, from the description:

It comes with its caveats [...] As a rule of thumb, you should limit this approach to tools which do not autoload your code.

There is tools such as PHP-Scoper to properly solve this problem, although it is not without its own hurdles either.