WordPress.org

Ready to get started?Download WordPress

Forums

Plugin Organizer
[resolved] Errors when using WP_DEBUG (17 posts)

  1. Mike Schinkel
    Member
    Posted 1 year ago #

    When using WP_DEBUG the following errors are displayed in the admin plugins page:

    Notice: Undefined index: PO_group_view in /Users/mikeschinkel/Sites/example/public_html/wp-content/plugins/plugin-organizer/lib/PluginOrganizer.class.php on line 481
    Notice: Undefined index: PO_group_view in /Users/mikeschinkel/Sites/example/public_html/wp-content/plugins/plugin-organizer/lib/PluginOrganizer.class.php on line 497
    Notice: Undefined index: PO_group_view in /Users/mikeschinkel/Sites/example/public_html/wp-content/plugins/plugin-organizer/lib/PluginOrganizer.class.php on line 566
    Notice: Undefined index: plugin_status in /Users/mikeschinkel/Sites/example/public_html/wp-content/plugins/plugin-organizer/lib/PluginOrganizer.class.php on line 398
    Notice: Undefined offset: 0 in /Users/mikeschinkel/Sites/example/public_html/wp-content/plugins/plugin-organizer/lib/PluginOrganizer.class.php on line 381

    Using WP_DEBUG during plugin development is a best practice as you can see here:

    http://nacin.com/2010/04/23/5-ways-to-debug-wordpress/

    Would love to see you add the following like to your development system's /wp-config.php file and then test your plugin with it:

    define( 'WP_DEBUG', true );

  2. Jeff Sterup
    Member
    Plugin Author

    Posted 1 year ago #

    Undefined indexes are not errors. They are a notice. You should never have those turned on in production and I'm well aware that they are there. The undefined index may pose a problem however. I will look into that.

  3. Mike Schinkel
    Member
    Posted 1 year ago #

    Hi Jeff,

    Technically speaking you are correct. Some (all?) of those warning probably do not result in a but. The concern however is that you are not using WP_DEBUG so you might be missing warnings that actually matter.

    Here's other issues: people who develop sites with WP_DEBUG cannot use your plugin without it throwing errors, so besides being annoying while development when demoing to clients they ask "Why is it broken?". And some of the more advanced people in the WordPress community actually prefer to run their sites with WP_DEBUG being true.

    Here's what Andrew Nacin the development lead for WordPress 3.5 has to say about WP_DEBUG[1]:

    If you’re a plugin or theme developer and you don’t know what the WP_DEBUG is, you’re missing out. (If you do know what it is and you don’t use it during development, you’re crazy.)
    ...
    In a nutshell, there’s an entire API devoted to informing you of the best practices and the latest APIs.

    So, it's pretty easy to fix these, just always use WP_DEBUG when you are developing and then you'll be able to see and fix these issues.

    Hope this helps.

    [1] http://nacin.com/2010/03/22/deprecated-functions-and-wp_debug/

  4. Jeff Sterup
    Member
    Plugin Author

    Posted 1 year ago #

    If you run your site with wp_debug set to true you are opening yourself up to hackers. That level of error reporting should never be turned on in a production site. It should only be turned on for testing. I do turn my error reporting up during development but undefined indexes are not problems. They are sometimes intended. As is the case with the notices you reference. I could put an array_key_exists in the if statements to get rid of them but since they are not necessary and can become very redundant I generally don't use them.

  5. Mike Schinkel
    Member
    Posted 1 year ago #

    I'm curious how using WP_DEBUG opens someone up to hackers. Can you give a specific example?

    Anyway, I've gone ahead and made the fixes for you. It required three trivial fixes in /lib/PluginOrganizer.class.php and one in /tpl/globalPlugins.php:

    https://gist.github.com/4061683

    Hopefully you'll be willing to apply? BTW, I used isset() instead of array_key_exists() because it is more compact in the code and slightly faster.

    As an aside, some thoughts on why it's a good idea to always develop with warnings on:

    http://stackoverflow.com/a/1960588/102699

  6. Jeff Sterup
    Member
    Plugin Author

    Posted 1 year ago #

    http://php.net/manual/en/errorfunc.configuration.php

    When you turn on WP_DEBUG you are essentially turning on error_reporting and setting E_ALL. This can have the effect of server path disclosure to a hacker. You have also done the same thing above by pasting your server path for everyone to see. This is not a good idea.

  7. Mike Schinkel
    Member
    Posted 1 year ago #

    @Jeff Fair point about hackers. Of course if your site doesn't throw errors then you don't have this problem. But I demur.

    As for my pasting my server path for everyone to see, I doubt pasting the local paths to my localhost development machine is really going to open me up to hackers. If it does, so be it.

    Regardless if is it a good idea to not have WP_DEBUG on for production, as Nacin says "if you don't use WP_DEBUG during development, you’re crazy."

    Anyway, my goal of posting this was to help, not to cause you to get defensive. If you would like to apply my patch that would be nice. If not that's fine too.

  8. Dougal Campbell
    Member
    Posted 1 year ago #

    Path disclosure doesn't make a site vulnerable. Yes, it's information that could make it a little easier for a determined hacker to find things to poke. But only if you server has a vulnerability in the first place. Security through obscurity is not security, as the saying goes. Also, the path Mike posted above is obviously on his own local dev setup, and most-likely not on a public-facing IP.

    That said, eliminating all warnings and info messages is still a Best Practice. Yes, it's annoying to error-check every instance of an array index, define(), etc. But it's still a Best Practice. Clean code is happy code!

    Oh, and nobody is advocating running with WP_DEBUG in production. But even so -- many sites turn on logging for warnings or even info messages. If you leave in code that generates these unneeded messages, all those servers running your plugin will waste CPU cycles and disk space for them. I, for one, get annoyed when my logs are polluted like that.

  9. Jeff Sterup
    Member
    Plugin Author

    Posted 1 year ago #

    Here's a write up on OWASP of the problems that arise from full path disclosure. I'm not advocating security through obscurity but there are many different steps one needs to go through to achieve a secure platform. Hiding this vulnerability is one of them. WordPress is known for its security holes and there is no reason for leaving this one open. He was advocating for running with WP_DEBUG set to true in production.

    https://www.owasp.org/index.php/Full_Path_Disclosure

    Im not trying to be defensive. I'm just trying to point out that notices are not errors and that they should be ignored. There are many more areas of this plugin that throw notices and I have yet to find a plugin that doesn't give some kind of notice. The suggestion has been made before and I am well aware of the notices. The plugin is working as intended.

  10. Mike Schinkel
    Member
    Posted 1 year ago #

    @Jeff,

    He was advocating for running with WP_DEBUG set to true in production.

    Not sure how you got that impression. My initial post said (bold added):

    Using WP_DEBUG during plugin development is a best practice

    My only mention of using during production was this aside only because you should never have those turned on in production, which wasn't the point:

    And some of the more advanced people in the WordPress community actually prefer to run their sites with WP_DEBUG being true.

    Again, I wasn't trying to make you defensive. If you think it's acceptable to leave notices in your plugin then given current requirements of the plugin repository it's your prerogative. I'm now sorry that I ever brought this up.

  11. Andrew Nacin
    Lead Developer
    Posted 1 year ago #

    WP_DEBUG is indeed often run in production (including on wordpress.org itself). But it is best to not conflate the two things it does. Primarily, it sets error reporting to include notices — that's the important part. While it also forces errors to be displayed, that isn't something you'd do in production — that's why this part is overridable (via WP_DEBUG_DISPLAY).

    The important thing isn't displaying errors, it's recognizing that notices are errors. They often indicate (as you mentioned) a problem, or an assumption that could result in a problem later.

  12. Franz Josef Kaiser
    Member
    Posted 1 year ago #

    @Jeff Sterup

    To double @Nacin and add some further explanation: Without WP_DEBUG, you're not able to use WP_DEBUG_LOG. And you'll really want to use this in production too, as you might not know what happened to your site, when your server starts crashing. Log files can be accessed even after your site is long gone.

    As an addition, here's a short sneak peek from my local config setup. "Error level: Paranoia".

    // DS is a short defined as alias of DIRECTORY_SEPARATOR
    # ==================================
    # PHP errors & log
    error_reporting( E_ALL | E_STRICT | E_CORE_ERROR | E_CORE_WARNING | E_COMPILE_ERROR | E_ERROR | E_WARNING | E_PARSE | E_USER_ERROR | E_USER_WARNING | E_RECOVERABLE_ERROR );
    @ini_set( 'display_errors',     1 );
    @ini_set( 'log_errors',         1 );
    
    # ==================================
    # DEBUG
    define( 'WP_DEBUG',             true );
    // file: ~/WP_CONTENT_DIR/debug.log
    define( 'WP_DEBUG_LOG',         true );
    define( 'WP_DEBUG_DISPLAY',     true );
    define( 'SAVEQUERIES',          true );
    # DEBUG: MU
    define( 'DIEONDBERROR',         true );
    define( 'ERRORLOGFILE',         WP_CONTENT_DIR.DS.'logs'.DS.'mu_error.log' );
    
    # PHP Error log location
    @ini_set( 'error_log',          WP_CONTENT_DIR.DS.'logs'.DS.'php_error.log' );
    

    In case you're wondering why I'm doing this, here's the explanation. If you ignore Notices, Warnings or errors, then you're making the life of everyone who is coding after you pretty hard. If you deliver code that other people build upon, extend, etc. then you're responsible to throw no errors/warnings/notices at all, because else no one would be able to distinguish between their own errors and yours.

    Don't behave like a ThemeForest developer.

  13. Andrew Nacin
    Lead Developer
    Posted 1 year ago #

    I wouldn't use WP_DEBUG_LOG in production, unless you've specifically blocked wp-content/debug.log from external access. (I wished we didn't introduce that to begin with, and I don't know how long it has for this world.) Rather, you'll want to use standard PHP error logging.

    also, SAVEQUERIES is slow and should not be used in production.

  14. Franz Josef Kaiser
    Member
    Posted 1 year ago #

    @Nacin Haven't talked about this logs in production, but that clarification seems to be necessary.

    (...) here's a short sneak peek from my local config setup

    Also it's not necessary to add all those E_*, as E_ALL covers lots of them, but it's easier for me to delete only what I don't need in some specific case. As I said: Just my local config.

    Anyway, maybe it's time for another "Make" article on debug and development. The last line from my comment above may fit as title.

  15. Japh
    Member
    Posted 1 year ago #

    Using WP_DEBUG during development is a crucial part of the development process. As @Nacin says, Notices are errors.

    If your plugin is throwing any sort of error level output, you have more work to do.

    ----------

    Also, I just wanted to add a brief reply to "Don't behave like a ThemeForest developer.":

    While your point is understood, and I myself am also often frustrated by this on some older ThemeForest themes, it's completely unfair to generalise like that.

  16. Jeff Sterup
    Member
    Plugin Author

    Posted 1 year ago #

    Fine. I will go through the plugin and remove notices. But please take my advice. It is not a good idea to run with that level of error reporting on in a production site. Unless you don't get much traffic. If the notices are printed to the screen it can give a hacker clues as to how your system is set up. Plus it has to write every one of those notices to a file and can bring down your server. Believe me I've seen it happen. Best practice is to only log critical errors to your log unless it is a dev server. You can try your best but I can still create notices in your error logs to fill it up by playing around with ajax requests and other URL's.

  17. Jeff Sterup
    Member
    Plugin Author

    Posted 1 year ago #

    Version 2.5 removes the notices.

Topic Closed

This topic has been closed to new replies.

About this Plugin

About this Topic