View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0019267 | mantisbt | upgrade | public | 2015-01-26 08:34 | 2016-08-03 10:37 |
Reporter | vboctor | Assigned To | vboctor | ||
Priority | normal | Severity | block | Reproducibility | always |
Status | closed | Resolution | won't fix | ||
Product Version | 1.3.0-beta.1 | ||||
Summary | 0019267: Upgrade converts complex db configs to NULLs | ||||
Description | If the mantisbt 1.2.x database has complex config options, such options are converted to NULLs as part of the upgrade causing the application to break in random places and the configuration to be lost. | ||||
Tags | mantishub | ||||
Deleting the null configs makes MantisBT work again, but the configuration is lost. It is also hard for users to discover the issue. |
|
I looked into this further and here are the observations:
Having such invalid configs generates all sort of random errors in the apps, since other APIs gets a null value for config thresholds or workflows and passes these to APIs that don't expect nulls. My recommendation here is to follow the model:
If we want beta1 users to benefit from this, then we can add this as another step that detects complex fields with value starting with "a:" (for array) and upgrade them. If unserialize() fails, then delete. To reproduce this create a 1.2.x instance with some of these complex configs, corrupt one of them and then upgrade to 1.3. |
|
I'm curious as to how you got there in the first place, considering that the install_check_config_serialization() upgrade function should fail with an error if it detects an invalid config (leaving you with an incomplete upgrade). Continuing after an error is incorrect, and IMO having the installer deleting an offending (corrupted) value may be a problem, so I think it's best to leave fixing such issues to manual correction by the admin. What we should do is make it clear to the admin what why the error is occuring, and give them options (what they can do to fix it). I'm not sure how this could be done in the installer, as by nature the options are fairly limited in terms of what we can display back to the admin in case of error. Maybe an admin check ? |
|
The upgrade was done using upgrade_unattended script. I'm not sure whether the script stopped on first error or just ignore that this step is failing and continued other steps. If we assume that the customer has a backup and that configs is not really the end of the world, then we may delete the bad items and provide the user with a list of what we have deleted after the fact. That wouldn't help with the upgrade_unattended script though. The other issue that I'm not sure about is why unserialize() is sometimes not able to deserialize its own data. And why such configs didn't cause an issue before the upgrade. i.e. the blob was invalid. Then even with old 1.2.x code, the app should have got some bad value (e.g. false/null, etc) that would have impacted the logic of the app. |
|
Ahem. Well, you know... I thought I had one, but... Joke aside, do you have some specific example(s) of corrupted entries ? I'd like to have a look at the upgrade_unattended script with a real test case (I never actually used it myself). It'd also be interesting to see what happens in 1.2.x with such corrupted data. |
|
Here is a sample corrupt filter: Corruption caused by special characters next to "+educa" - remove the "v8#" prefix and then test the result with unserialize().
|
|
Just tested this a bit more in depth:
At this stage, the adm_config_report.php page will show the config as corrupted, and depending on error settings, a PHP notice will be displayed on some screens:
SYSTEM NOTICE: 'unserialize(): Error at offset 312 of 548 bytes' in '/home/dregad/dev/mantisbt/core/config_api.php' line 161
Schema step 193: UpdateFunction ( check_config_serialization )
Based on the above, as far as I can tell the system works as designed, and no change is required. Let me know your thoughts. |
|