Why are custom error messages not used for Mage_Core_Exception

Question

Referring to the example code below, in the case of Mage_Core_Exception why is the error message from the exception being used directly $e->getMessage() rather than a custom message like $result['error_msg'] = $this->__('There was an error processing ...')

Doesn't this represent a security risk where the error message from the exception might be something like "unknown table user_password in the database" and this gets sent to the frontend via AJAX or other means?

Path: app/code/core/Mage/Authorizenet/controllers/Directpost/PaymentController.php

public function responseAction()
{
    $data = $this->getRequest()->getPost();
    /* @var $paymentMethod Mage_Authorizenet_Model_DirectPost */
    $paymentMethod = Mage::getModel('authorizenet/directpost');

    $result = array();
    if (!empty($data['x_invoice_num'])) {
        $result['x_invoice_num'] = $data['x_invoice_num'];
    }

    try {
        if (!empty($data['store_id'])) {
            $paymentMethod->setStore($data['store_id']);
        }
        $paymentMethod->process($data);
        $result['success'] = 1;
    }
    catch (Mage_Core_Exception $e) {
        Mage::logException($e);
        $result['success'] = 0;
        $result['error_msg'] = $e->getMessage();
    }
    catch (Exception $e) {
        Mage::logException($e);
        $result['success'] = 0;
        $result['error_msg'] = $this->__('There was an error processing your order. Please contact us or try again later.');
    }

    if (!empty($data['controller_action_name'])) {
        if (!empty($data['key'])) {
            $result['key'] = $data['key'];
        }
        $result['controller_action_name'] = $data['controller_action_name'];
        $result['is_secure'] = isset($data['is_secure']) ? $data['is_secure'] : false;
        $params['redirect'] = Mage::helper('authorizenet')->getRedirectIframeUrl($result);
    }
    $block = $this->_getIframeBlock()->setParams($params);
    $this->getResponse()->setBody($block->toHtml());
}

score 2 · Accepted Answer · answered Dec 02 '14 at 15:31

I don't think there is a Mage_Core_Exception exception thrown without knowing the exception message.

Here are some random examples:

throw new Mage_Core_Exception('Unable to find writable var_dir');
throw new Mage_Core_Exception(
    Mage::helper('sales')->__('Last status can\'t be unassigned from state.')
);
throw new Mage_Core_Exception(
    $this->__('Some of the processed products have no SKU value defined. Please fill it prior to performing operations on these products.')
);

also Mage::throwException uses the same class:

Mage::throwException(Mage::helper('sales')->__('Cannot get order instance'));
Mage::throwException(Mage::helper('catalog')->__('Attribute "%s" is required.', $category->getResource()->getAttribute($code)->getFrontend()->getLabel()));

There are 2 catch statements

one for Mage_Core_Exception because the user should see the exception message. This should be considered more as "errors in usage of the app" than exceptions
one for Exception (every other exception) where the user sees a a general message like "You cannot do that". Usually when the exception is thrown in cases like you mentioned (table missing, connection errors) the message is logged in var/log and not printed in the application.

I think this is a nice way of separating exception types.
But keep in mind 2 things.
Never show the result of $e->getMessage when $e is instance of Exception, and never throw a Mage_Core_Exception whit sensitive data in the message.
If you follow these rules you should be safe.

Why are custom error messages not used for Mage_Core_Exception

1 Answers1