[2.0.x] Bug with Creating Tables in Modifications

Started by Matthew K., March 28, 2016, 10:28:17 PM

Previous topic - Next topic

Matthew K.

There is a bug in SMF 2.0.x that pertains to using db_create_table (found in DbPackages-mysql.php or the other dbms files).

When the function is parsing the size of the column that you are trying to create, it checks to see if the size is numeric (is_numeric();) which totally makes sense...in most cases. Where this becomes an issue is when you are wanting to use a column type that is more than just an integer. Right in phpMyAdmin, you can see the "size" is specified as "Length/Values", meaning it theoretically, in some cases can hold more than one value. I ran into this issue when trying to make two columns "points" and "total_points" both using float(6,2).

Code is below for reference. This is obviously just a slight oversight, but it prevents mod authors from using any column types that are beyond a single-integer, like float. To make my modification work, I also have to provide a modifications.xml file in an otherwise modification-less mod to fix SMF's bug in my mod.

./Sources/DbPackages-mysql:130-132:

// Sort out the size... and stuff...
$column['size'] = isset($column['size']) && is_numeric($column['size']) ? $column['size'] : null;
list ($type, $size) = $smcFunc['db_calculate_type']($column['type'], $column['size']);


Usage Example:

[
'name' => 'points',
'type' => 'float',
'size' => '6,2',
'unsigned' => true,
'default' => 0.00
],

Matthew K.

A proposed fixed (without testing), would be to add right below line 132 the following:

if ($type === 'float')
$size = $column['size'];

Obviously, this should also carry over to decimals, and any other fields that potentially take on multiple values. Another thought might be to just remove the check for is_numeric() on size, since more than not, it might be wrong. Or explode by commas, and (int) or floatval(); each piece.

This issue is in two functions, I also found it later in db_add_column.

Matthew K.

After looking into this with a little bit more detail...I notice the obvious, that $column['size'] is over-written on line 131. Which means my proposed fix without testing will not work. More on this soon.

Matthew K.

Okay, so the fix is actually really simple:
Code (Find) Select

$column['size'] = isset($column['size']) && is_numeric($column['size']) ? $column['size'] : null;

Code (Replace) Select

$column['size'] = isset($column['size']) && ($column['type'] === 'float' || is_numeric($column['size'])) ? $column['size'] : null;

Again...this will have to be changed to something similar to this:

$column['size'] = isset($column['size']) && (in_array($column['type'], array('float', 'decimal', 'double')) || is_numeric($column['size'])) ? $column['size'] : null;

Matthew K.

Does no one on the SMF team care to reply to this? I found a bug that's been in SMF for years, if not since SMF 2.0 at the very earliest and could possibly still be in SMF 2.1. And no reply?

Kindred

Thank you for th report. Sorry for not responding as fast as you wanted.
Слaва
Украинi

Please do not PM, IM or Email me with support questions.  You will get better and faster responses in the support boards.  Thank you.

"Loki is not evil, although he is certainly not a force for good. Loki is... complicated."

Matthew K.

There's actually another bug that is somewhat related in the same file. I'll post it later, hopefully I'm not wasting my time reporting them.

Colin

"If everybody is thinking alike, then somebody is not thinking." - Gen. George S. Patton Jr.

Colin

Matthew K.

./Sources/DbPackages-Mysql.php

Code (Find) Select

// Allow unsigned integers (mysql only)
$unsigned = in_array($type, array('int', 'tinyint', 'smallint', 'mediumint', 'bigint')) && !empty($column['unsigned']) ? 'unsigned ' : '';

Code (Replace) Select

// Allow unsigned integers (mysql only)
$unsigned = in_array($type, array('int', 'tinyint', 'smallint', 'mediumint', 'bigint', 'float')) && !empty($column['unsigned']) ? 'unsigned ' : '';

Again...this would need to encompass other column types that can be UNSIGNED. Such as decimal, and double. There are three instances of this line in this file. That are all identical.

Matthew K.

Is this going to get taken care of in the next patch of SMF 2.0.x and at the very least SMF 2.1?

Kindred

Don't know about 2.0.12... but I think it is already fixed in 2.1
Слaва
Украинi

Please do not PM, IM or Email me with support questions.  You will get better and faster responses in the support boards.  Thank you.

"Loki is not evil, although he is certainly not a force for good. Loki is... complicated."

nend


Matthew K.


Advertisement: