View Issue Details

IDProjectCategoryView StatusLast Update
0020168mantisbtdb schemapublic2017-05-20 16:10
Reportercproensa Assigned Todregad  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version1.3.0-beta.3 
Target Version1.3.11Fixed in Version1.3.11 
Summary0020168: Use of 'mantis' as plugin table prefix prevents plugin's installation
Description

If configuration option for plugin table prefix starts with the string "mantis"
Example:
$g_db_table_plugin_prefix = 'mantis_plugin';

table names are not formed correctly, because it detects that is passing a legacy style 'mantis_XXX_table' to db_get_table function (database_api.php)

my proposed table name would be:
"mantis_plugin_PLUGINNAME_TABLE"

the function is returning
"plugin_PLUGINNAM"

Additional Information

This is the checking code

    if( strpos( $p_name, 'mantis_' ) === 0 ) {
        $t_table = substr( $p_name, 7, strpos( $p_name, '_table' ) - 7 );
        error_parameters(
            __FUNCTION__ . "( '$p_name' )",
            __FUNCTION__ . "( '$t_table' )"
        );
        trigger_error( ERROR_DEPRECATED_SUPERSEDED, DEPRECATED );
TagsNo tags attached.
Attached Files

Relationships

has duplicate 0022715 closeddregad Use of 'mantis' as plugin table prefix prevents plugin's installation 
has duplicate 0022811 closeddregad useing Source Control Integration 2.0.0 update error 
related to 0022851 closeddregad Installer should display sample table names based on table prefix/suffix settings 

Activities

dregad

dregad

2015-10-03 20:06

developer   ~0051592

Is this a real-life scenario, or just academic ?

Using 'mantis_plugin' as a plugin table prefix sounds a bit silly. Who would truly want their plugin tables to be called 'mantis_mantis_plugin_MYTABLE_table' (assuming $g_db_table_prefix is left to default value 'mantis') ?

I would be tempted to address this by adding an admin check to notify the administrator that this setting is not valid.

cproensa

cproensa

2015-10-05 04:05

developer   ~0051598

Last edited: 2017-05-09 10:09

Using 'mantis_plugin' as a plugin table prefix sounds a bit silly. Who would truly want their plugin tables to be called 'mantis_mantis_plugin_MYTABLE_table' (assuming $g_db_table_prefix is left to default value 'mantis') ?

yes, I actually put this configuration...

The proper thing would be to make it check for string starting with 'mantis_' AND ending with '_table'
However, this is a corner case

I would be tempted to address this by adding an admin check to notify the administrator that this setting is not valid

My confussion came from not seeing clear that "plugin prefix" is added after "table prefix", and thinking they were independent.
Probably also addressed by showing a short text in install script making clear how a table name would result.

dregad

dregad

2015-10-05 05:16

developer   ~0051600

The proper thing would be to make it check for string starting with 'mantis_' AND ending with '_table'

OK, easy enough to do.

Probably also addressed by showing a short text in install script making clear how a table name would result.

Makes sense, too.

cproensa

cproensa

2017-04-12 10:03

developer   ~0056504

Considering this is a legacy naming convention from... 1.2?
maybe it's time to finally remove it

dregad

dregad

2017-04-12 12:13

developer   ~0056508

maybe it's time to finally remove it

Indeed that's a good point.
It was left in to ensure backwards-compatibility of legacy plugins in 1.3.

dregad

dregad

2017-04-12 12:28

developer   ~0056510

showing a short text in install script making clear how a table name would result.

Something like this ?

screenshot-20170412-1827.png (7,114 bytes)   
screenshot-20170412-1827.png (7,114 bytes)   
cproensa

cproensa

2017-04-12 12:54

developer   ~0056511

Something like this ?

That would imply to modify the sample dynamically from user input...
a static text with an example would be enough

Maybe relevant:
Include a warning (if oracle) that maximum table names are 30 chars, which includes prefixes, suffixes and plugin name

dregad

dregad

2017-04-13 19:40

developer   ~0056538

That would imply to modify the sample dynamically from user input...

That was my idea actually, easy to do with a few lines of JavaScript. Although I'll concede it's a bit overkill ;-)

Include a warning (if oracle) that maximum table names are 30 chars, which includes prefixes, suffixes and plugin name

Impossible to do an actual check in real life, since 1. we can't control what plugins authors do, and 2. do not know which plugins the admin will install. So the warning could only be a generic text.
For the record, in Oracle 12c the limit has been increased to 128 chars [1]

cproensa

cproensa

2017-04-13 19:58

developer   ~0056539

Impossible to do an actual check in real life, since 1. we can't control what plugins authors do, and 2. do not know which plugins the admin will install. So the warning could only be a generic text.

Sure. My point is to make clear that to the user, before the damage is done. I had the experience of having a plugin creating tables that exceeded the length limit. Renaming the database tables after installation was a real pita...
At least you would know that all you can do is minimize the prefix/suffix.
IIRC, when installing with oracle, the default prefixes were set as "m" and "p" anyway

For the record, in Oracle 12c the limit has been increased to 128 chars [1]

about time. The express version that is available for development is 11g, i think

dregad

dregad

2017-04-14 05:01

developer   ~0056547

make clear that to the user, before the damage is done

I know people don't read it, but it's quite well documented in admin guide ;-)
http://mantisbt.org/docs/master/en-US/Admin_Guide/html-desktop/#admin.config.database

Anyway, I guess it does not cost much to add this extra warning.

The express version that is available for development is 11g, i think

Correct, 12c XE has not yet been announced even though Oracle apparently said that it's planned. I found some musings about dates in https://oracle-base.com/blog/2016/01/07/oracle-xe-12c/; 12.2 is out, but not 12.2.0.2 yet, I don't think we'll see XE 12 before end of 2017, probably even later.

dregad

dregad

2017-05-09 10:07

developer   ~0056819

Changing summary to make it more descriptive of the actual problem (copied from 0022715)

dregad

dregad

2017-05-10 08:44

developer   ~0056827

See 0022851 for follow up on 0020168:0051598:

showing a short text in install script making clear how a table name would result

dregad

dregad

2017-05-10 08:54

developer   ~0056828

PR https://github.com/mantisbt/mantisbt/pull/1103

Related Changesets

MantisBT: master 4917320b

2017-04-12 04:29

dregad


Details Diff
db_get_table() also check suffix for legacy-style names

If $db_table_plugin_prefix contains 'mantis', db_get_table() function
will incorrectly process a plugin's tables as if they were legacy tables
(i.e. 'Mantis 1.2 style', with a 'mantis_' prefix).

If $db_table_suffix also is not set to '_table', then the generated table
name will be incorrect (e.g. with the Source Integration plugin,
changeset table will be generated as 'mantis_Source_ch_suffix' instead
of 'mantis_mantis_Source_changeset_suffix'), causing the installation by
plugin_upgrade() to fail and invalid tables to be created.

We now use a regex to check for legacy tables, verifying that it ends
with '_table' in addition to beginning with 'mantis_'. This basically
reverts commit 38bc02483eb58a3708e4f419bfa7c02b6c6900db (issue 0016038).

Note on performance: while preg_match() is slower than strpos(), it is
actually more efficient to use that, considering that we would need a
second function call to check for the suffix, as well as a substr() call
to extract the table name, while preg_match() does it all at once.

Fixes 0020168
Affected Issues
0016038, 0020168
mod - core/database_api.php Diff File

MantisBT: master-1.3.x b933abcb

2017-04-12 04:29

dregad


Details Diff
db_get_table() also check suffix for legacy-style names

If $db_table_plugin_prefix contains 'mantis', db_get_table() function
will incorrectly process a plugin's tables as if they were legacy tables
(i.e. 'Mantis 1.2 style', with a 'mantis_' prefix).

If $db_table_suffix also is not set to '_table', then the generated table
name will be incorrect (e.g. with the Source Integration plugin,
changeset table will be generated as 'mantis_Source_ch_suffix' instead
of 'mantis_mantis_Source_changeset_suffix'), causing the installation by
plugin_upgrade() to fail and invalid tables to be created.

We now use a regex to check for legacy tables, verifying that it ends
with '_table' in addition to beginning with 'mantis_'. This basically
reverts commit 38bc02483eb58a3708e4f419bfa7c02b6c6900db (issue 0016038).

Note on performance: while preg_match() is slower than strpos(), it is
actually more efficient to use that, considering that we would need a
second function call to check for the suffix, as well as a substr() call
to extract the table name, while preg_match() does it all at once.

Fixes 0020168

Backported from master 4917320b2067f5797aa87e5772a820e6b5a177cc.
Affected Issues
0016038, 0020168
mod - core/database_api.php Diff File