[3405] [2.0 RC1] No Errors and No Table

[onepiece]

When using $smcFunc['db_create_table'] if you have an incorrect table definition, like defining a size value for a TEXT field, the table is not created but there are no errors either -even when you are on debug mode-.

This gives mod authors a hard time debugging the issue and figuring out the problem. Here some examples of it.

http://www.simplemachines.org/community/index.php?topic=224166.msg1814902#msg1814902
http://www.simplemachines.org/community/index.php?topic=276008.0
http://www.simplemachines.org/community/index.php?topic=299177.0

[SleePy]

Thanks,
Bug #3405: Table creation does not return success of failure

You can fix this by modifying DbPackages-mysq.php

Find:

Code Select Expand


    // Create the table!
    $smcFunc['db_query']('', $table_query,
        'security_override'
    );


Add After:

Code Select Expand


    // Does it exist?
    $request = $smcFunc['db_query']('', '
        SHOW TABLES LIKE {string:table}',
        array(
            'table' => $table_name,
    ));
    // Oh no!
    if ($smcFunc['db_num_rows']($request) < 1)
    {
        loadLanguage('Errors');
        log_error($txt['database_error'] . ':' . sprintf($txt['db_table_creation'], $table_name), $database, null, null);
        $smcFunc['db_free_result']($request);

        return false;
    }
    $smcFunc['db_free_result']($request);

    return true;


Then add to Errors.english.php

Code Select Expand


$txt['db_table_creation'] = 'Unable to create %1s table in database';


[onepiece]

I don't think that's a neat solution. In my opinion, it should return the real error occurring. Here is the actual problem:

Subs-Db-mysql.php

Code (Line:355-60) Select Expand


if (empty($db_unbuffered))
$ret = @mysql_query($db_string, $connection);
else
$ret = @mysql_unbuffered_query($db_string, $connection);
if ($ret === false && empty($db_values['db_error_skip']))
$ret = smf_db_error($db_string, $connection);

It won't return an error for $smcFunc['db_query'] if $db_values['db_error_skip'] is not empty. When $db_values is set as the string 'security_override' (which is how it is done for most of the package functions) $db_values['db_error_skip'] will be equal to 's' and it won't return an error.

My suggested fixes: (just for MySQL as an example)

Sources/Subs-Db-mysql.php

Quick and easy way:

Code (Find) Select Expand

if ($ret === false && empty($db_values['db_error_skip']))

Code (Replace) Select Expand

if ($ret === false && (empty($db_values['db_error_skip']) || $db_values === 'security_override'))

Maybe a cleaner way:

Code (Find) Select Expand

// Overriding security? This is evil!
$security_override = $db_values === 'security_override' || !empty($db_values['security_override']);

Code (Replace) Select Expand

// It is better to have db values as an array here.
$db_values = $db_values === 'security_override' ? array('security_override' => true) : $db_values;

// Overriding security? This is evil!
$security_override = !empty($db_values['security_override']);

[karlbenson]

As i pointe out to sleepy,
We need to find out why security_override is used when creating the table.
For alot of things it has been used INTENTIONALLY.

[onepiece]

Well, I don't have a problem with security_override parameter. I just say that, regardless of the value of security_override parameter, it should return an error if there are any.

[SleePy]

I assumed that it was done that way on purpose to prevent SMF from trying to throw security measures at something we hand worked out ourselves.
In any case then. We just add => true, after 'security_override' and this should still work.

[onepiece]

I assumed that it was done that way on purpose to prevent SMF from trying to throw security measures at something we hand worked out ourselves.

I'm not sure if I got what you mean correctly, but SMF doesn't show the actual error to user, unless s/he has the 'admin_forum' permission.

In any case then. We just add => true, after 'security_override' and this should still work.

Yeah, with the array() it would work. However, there are quite a lot of queries using it as a string. Also, if that is to make only the desired queries to display errors, I don't think that's a clean way either.

[[SiNaN]]

Fixed with Revision 9883.