pluggabl/woocommerce-jetpack

handling unreliable external resources

Opened this issue · 0 comments

Have a stage (localhost) site and a live one (VPS/Ubuntu). When switching back and forth between them, Booster complains because of not enough heavy-duty coding practices in place while handling external resources. Examples:

Issue#1:

PHP Warning: simplexml_load_file(): I/O warning : failed to load external entity "http://www.ecb.int/stats/eurofxref/eurofxref-daily.xml" in \plugins\booster-plus-for-woocommerce\includes\functions\wcj-functions-exchange-rates.php on line 387

    function wcj_ecb_get_exchange_rate( $currency_from, $currency_to ) {
        $final_rate = false;
        if ( function_exists( 'simplexml_load_file' ) ) {
            $xml = simplexml_load_file( 'http://www.ecb.int/stats/eurofxref/eurofxref-daily.xml' );

Explanation: when simplexml_load_file isn't able to load the resource a php warning is thrown.

Proposed fix: add some extra code that would handle such unexpected situation:

    function wcj_ecb_get_exchange_rate( $currency_from, $currency_to ) {
        $final_rate = false;
        if ( function_exists( 'simplexml_load_file' ) ) {
            //$xml = simplexml_load_file( 'http://www.ecb.int/stats/eurofxref/eurofxref-daily.xml' );
            // https://stackoverflow.com/questions/1917876/php-simplexml-load-file-catch-file-errors
            $xml = simplexml_load_file( 'http://www.ecb.int/stats/eurofxref/eurofxref-daily.xml', 'SimpleXMLElement', LIBXML_NOWARNING );

Issue#2:

PHP Warning: Division by zero in \plugins\booster-plus-for-woocommerce\includes\functions\wcj-functions-exchange-rates.php on line 159

    function wcj_get_exchange_rate( $currency_from, $currency_to ) {
// ...
        $return = apply_filters( 'wcj_currency_exchange_rate', $return, $exchange_rates_server, $currency_from, $currency_to );
        return ( 'yes' === $calculate_by_invert ) ? round( ( 1 / $return ), 6 ) : $return;

Explanation: $return seems to depend on functions that fetch external resources, and doesn't get checked when they return unexpected values.

Proposed fix: add some ad-hock check that would handle such improperly fetched variables:

return ( $return && 'yes' === $calculate_by_invert ) ? round( ( 1 / $return ), 6 ) : $return;