init() failures are opaque and hard to diagnose #461

Open
schmod opened this Issue Aug 3, 2016 · 1 comment

Projects

None yet

2 participants

@schmod
schmod commented Aug 3, 2016

SimplePie::init() method simply gives up and returns false if a variety of conditions are not met, which leads to conditions that can be very difficult to diagnose.

Significantly, if PHP's XML (or PCRE) extensions aren't loaded (which frustratingly appears to now be the new default in Ubuntu 16.04), SimplePie::init() returns false, and offers no indication about why it did so.

This error is particularly hard to diagnose, because phpinfo() still prints a bunch of stuff about XML even when the extension isn't installed.

Given that there's no possible way that SimplePie can work on a system under these conditions, I'd suggest that we should throw an exception or call trigger_error() instead of returning false if we encounter any of these conditions.

Similarly, SimplePie is not shy about producing errors in other situations, so it seems particularly baffling that we choose to fail silently in the most egregious case...


The relevant code snippet:

    public function init()
    {
        // Check absolute bare minimum requirements.
        if (!extension_loaded('xml') || !extension_loaded('pcre'))
        {
            return false;
        }
        // Then check the xml extension is sane (i.e., libxml 2.7.x issue on PHP < 5.2.9 and libxml 2.7.0 to 2.7.2 on any version) if we don't have xmlreader.
        elseif (!extension_loaded('xmlreader'))
        {
            static $xml_is_sane = null;
            if ($xml_is_sane === null)
            {
                $parser_check = xml_parser_create();
                xml_parse_into_struct($parser_check, '<foo>&amp;</foo>', $values);
                xml_parser_free($parser_check);
                $xml_is_sane = isset($values[0]['value']);
            }
            if (!$xml_is_sane)
            {
                return false;
            }
        }

I also find it somewhat counterintuitive that methods like SimplePie::get_items() will happily return empty arrays or null values if SimplePie::init() hasn't been called (or was unsuccessful).

@mblaney
Member
mblaney commented Aug 3, 2016

hi @schmod, thanks that sounds good. Happy to accept a pull request if you want to add the code to the section you've described above? By writing it up you've done most of the work already. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment