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

Add function_exists( 'add_action' ) check to initialization file #1270

Closed
salcode opened this issue May 23, 2019 · 4 comments · Fixed by #1271
Closed

Add function_exists( 'add_action' ) check to initialization file #1270

salcode opened this issue May 23, 2019 · 4 comments · Fixed by #1271

Comments

@salcode
Copy link
Collaborator

salcode commented May 23, 2019

When using CMB2 via composer and using a PHP based tool (e.g. PHP Code Sniffer), the PHP based tool throws an error.

Expected Behavior:

When installing CMB2 and PHP Code Sniffer via composer with a composer.json of

{
    "require": {
        "cmb2/cmb2": "^2.6"
    },
    "require-dev": {
        "squizlabs/php_codesniffer": "3.*"
    },
    "autoload"    : {
        "files": [
            "vendor/cmb2/cmb2/init.php"
        ]
    },
    "extra": {
        "installer-paths": {
            "vendor/cmb2/cmb2": ["cmb2/cmb2"]
        }
    }
}

I expect the following to run PHP Code Sniffer (phpcs)

$ composer install
$ ./vendor/bin/phpcs

Actual Behavior:

I get a fatal error because the function add_action() is undefined.

$ composer install
$ ./vendor/bin/phpcs
Fatal error: Uncaught Error: Call to undefined function add_action() in vendor/cmb2/cmb2/init.php:126

Possible Solution

If we add a function_exists() check for add_action() before calling it, we can prevent throwing this fatal error.

Possible Solution's Risk Level

This seems to be a relatively low risk change. Since we're checking if the function add_action() exists directly before calling it, the only behavior change we should get is avoiding throwing this fatal error.

Steps to reproduce (I have confirmed I can reproduce this issue on the develop branch):

  1. Clone https://github.com/salcode/salcode-cmb2-autoload-demo
  2. Run composer install
  3. Run ./vendor/bin/phpcs

Your Environment

Mac OS X Command Line

salcode added a commit to salcode/CMB2 that referenced this issue May 23, 2019
Add function_exists('add_action') check to initialization file. When
loading CMB2 via Composer (and autoloading init.php), the init.php will
be loaded every time. Usually this is not a problem, however if we are
also using Composer to load a PHP development tool (e.g. PHP Codesniffer
(phpcs)), when the development tool runs it is NOT in the context of
WordPress.

Running CMB2's init.php outside of the context of WordPress
results in a fatal error because the function add_action() does not
exist. By adding this function_exists() check, we can avoid this fatal
error when autoloading init.php outside of the context of WordPress.

See CMB2#1270
@MichaelHabib
Copy link

I had the same issue with the latest version of CMB2 ,
I added the following code to the top of init.php
if (!defined('ABSPATH')) exit; // Exit if accessed directly

Also, after a simple google search, it seems this issue has crashed few websites !

@tw2113
Copy link
Contributor

tw2113 commented Jul 11, 2019

Based on #1271 it's a change that will get included in a future release, we just haven't gotten to the point of actually releasing yet. :D

@salcode
Copy link
Collaborator Author

salcode commented Jul 11, 2019

Also, after a simple google search, it seems this issue has crashed few websites !

@MichaelHabib I just wanted to chime in with some clarification about the issue I originally submitted, I assume your Google search was something like "Uncaught Error: Call to undefined function add_action() in vendor/cmb2/cmb2/init.php". The results that come back for me have pretty "deep" URLs (e.g. http://example.com/wp-content/plugins/cmb2/init.php) and with the sites I checked, if I went to the root of the website instead (e.g. http://example.com/) I didn't see the error.

I agree the preferred behavior would be to show nothing, rather than the error message. Based on all of this I'm glad to see this being added in the next release but I understand it is not an high priority to get the next release out. 😀

@MichaelHabib
Copy link

Thanks for the quick updates
I'll use the work around I've applied for now, as long the fix is in the next release it's all good :D

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

Successfully merging a pull request may close this issue.

3 participants