Issues

ZF2-563: Zend\Log\Write\Db::mapEventIntoColumn creates non-existing database table column due to PHP type juggling

Description

Zend\Log\Writer\Db::mapEventIntoColumn creates new non-existing database table column due to PHP type juggling:


protected function mapEventIntoColumn(array $event, array $columnMap = null)
{
    if (empty($event)) {
        return array();
    }

    $data = array();
    foreach ($event as $name => $value) {
        if (is_array($value)) {
            foreach ($value as $key => $subvalue) {
                if (isset($columnMap[$name][$key])) { // bad check!
                    $data[$columnMap[$name][$key]] = $subvalue;
                }
            }
        } elseif (isset($columnMap[$name])) {
            $data[$columnMap[$name]] = $value;
        }
    }
    return $data;
}

The isset($columnMap[$name][$key]) is not good, because a non-existing string value for $key enables PHP type juggling and thus making it 0. This occurs when $value is an array and $columnMap['extra'] contains a string.

A small example to make it more clear what is happening here:


<?php
$columnMap = array( 'extra' => 'extra' );

var_dump( isset( $columnMap['extra']['something'] ) ); // Output: boolean true

var_dump( $columnMap['extra']['something'] ); // Output: string 'e' (length=1)

Also it is impossible to store an array in a database table column without converting it to a string, so that must be done before the data can be used in a database query.

Comments

Typo

Can you add your use case of extra as a string? The Logger can only send an array as extra.

$extra is an array, that is right, but I am referring to $colomnMap['extra']. That is a string.

When $extra is an array, let's say: ``` The script now checks if $columnMap['extra']['line'] exists with isset() and this always returns true. Because of type juggling the string 'line' is converted to 0 (see my example). This way you get the first letter of 'extra', the 'e' and thus a non-existing database table column.

When Zend\Logger\Logger is used as an exception handler, in Zend\Logger\Logger::registerExceptionHandler() the following happens:


set_exception_handler(function ($exception) use ($logger) {
    $extra = array(
        'file'  => $exception->getFile(),
        'line'  => $exception->getLine(),
        'trace' => $exception->getTrace()
    );
    if (isset($exception->xdebug_message)) {
        $extra['xdebug'] = $exception->xdebug_message;
    }
    $logger->log(Logger::ERR, $exception->getMessage(), $extra);
});

So $extra is now an array with data from the exception. When I throw an exception the type juggling happens in the above mentioned methods.

Use case:


<?php
namespace Bug;

use Zend\Mvc\MvcEvent as MvcEvent;

class Module
{
    /**
     * On bootstrap event
     *
     * @access public
     * @param MvcEvent $event
     * @return void
     */
    public function onBootstrap( MvcEvent $event )
    {
        $application = $event->getApplication();
        $sm = $application->getServiceManager();

        $dbAdapter = $sm->get( 'dbAdapter' );
        $columnMapping = array(
            'extra' => 'extra'
        );
        $writer = new \Zend\Log\Writer\Db( $dbAdapter, 'bug', $columnMapping );

        $logger = new \Zend\Log\Logger();
        $logger->addWriter( $writer );
        $logger->registerExceptionHandler( $logger );

        throw new \Exception( 'My message' );
        return;
    }
}

EDIT: Changed database table name, namespace and added create table definition for easier reproducing:


CREATE TABLE `bug` (
  `extra` text
) ENGINE=InnoDB DEFAULT CHARSET=utf8

EDIT 2: Sorry, offcourse the error might be useful too: {quote}Fatal error: Uncaught exception 'PDOException' with message 'SQLSTATE[42S22]: Column not found: 1054 Unknown column 'e' in 'field list'' in some\folder\vendor\Zend\library\Zend\Db\Adapter\Driver\Pdo\Statement.php on line 220{quote}

Looking at Zend\Log\Writer\Db::eventIntoColumn it seems the intended usage is this:


-                        $data$columnMap[$name][$key]] = $subvalue;
+                        $data[$columnMap[$name] . $this->separator . $key] = $subvalue;

This still does not make any sense, because this creates random database table columns depending on the array keys of the extra array (see issue [#ZF2-562).

When you add the needed columns for logging the exception:


Create Table: CREATE TABLE `bug` (
  `extra` text,
  `extra_file` text,
  `extra_line` text,
  `extra_trace` text,
  `extra_xdebug` text
) ENGINE=InnoDB DEFAULT CHARSET=utf8

you also still get a notice and an unintended array to string conversion and thus data loss: {quote}Notice: Array to string conversion in some\folder\vendor\Zend\library\Zend\Db\Adapter\Driver\Pdo\Statement.php on line 258{quote}

This issue has been closed on Jira and moved to GitHub for issue tracking. To continue following the resolution of this issues, please visit: https://github.com/zendframework/zf2/issues/2589