Simple Machines Community Forum

Hello!

There is something wrong in the upgrade package from 2.0.8 to 2.0.9 (smf_patch_1.1.20_2.0.9.zip). It's about the second operation in "$sourcedir/ManageServer.php" (also shown here (http://custom.simplemachines.org/upgrades/index.php?action=upgrade;file=smf_patch_1.1.20_2.0.9.zip;smf_version=2.0.8#sources_manageserver-php_2)):

<operation>
<search position="before"><![CDATA[
$context['config_vars'][$config_var[1]]['value'] = unserialize($context['config_vars'][$config_var[1]]['value']);
]]></search>
<add><![CDATA[
$context['config_vars'][$config_var[1]]['value'] = !empty($context['config_vars'][$config_var[1]]['value']) ? unserialize($context['config_vars'][$config_var[1]]['value']) : array();
]]></add>
</operation>

It should be position="replace" instead of position="before". I saw some errors in my forum log after upgrading, and after manual fixing as I describe below, they are gone.

The fact is, we have the following at line 1875 of ManageServer.php in SMF 2.0.8 install package:

$context['config_vars'][$config_var[1]]['value'] = unserialize($context['config_vars'][$config_var[1]]['value']);

After patching to 2.0.9 as described above, we would have this:

$context['config_vars'][$config_var[1]]['value'] = unserialize($context['config_vars'][$config_var[1]]['value']);
$context['config_vars'][$config_var[1]]['value'] = !empty($context['config_vars'][$config_var[1]]['value']) ? unserialize($context['config_vars'][$config_var[1]]['value']) : array();

So the second unserialize() call returns an error, something like "string expected as parameter, but array found" (sorry I've fixed this in my forum already but I didn't copy the exact message), this is because the first unserialize() call has already done the job.

If you see ManageServer.php line 1875 in SMF 2.0.9 install package, you will see only this:

$context['config_vars'][$config_var[1]]['value'] = !empty($context['config_vars'][$config_var[1]]['value']) ? unserialize($context['config_vars'][$config_var[1]]['value']) : array();

So it looks like the upgrade package is wrong, and the intention was to use position="replace" instead of position="before" for that particular change. That, for sure, would make the patch to 2.0.8 to be in line with 2.0.9 source.

Just to clarify: the ManageServer.php file is OK in the install and upgrade full packages for 2.0.9, just the upgrade package from 2.0.8 is wrong.

Regards :)

PS FYI: it's related to one custom mod that uses a "<select>" with multiple options in the Admin panel, that makes that part of SMF code to execute. In theory, you can reproduce the error by displaying any "<select>" with multiple options in the Admin panel (I don't know if SMF original source uses any, if not you can try any mod that uses it).

http://www.simplemachines.org/community/index.php?topic=528448.msg3750911#msg3750911

And on. :)

For those who missed it. ;)

Hmmm...  BK, why the link? Pall of the necessary information was listed here already.  Unneeded.

Actually, I meant to link to the answer. This is what I should have linked to....lol

Quote from: Oldiesmann on October 05, 2014, 01:42:21 PM
That should indeed be a replace. Just remove the original line and keep the new one there.

Not knowing diddly about coding, which of these lines in ManageServer.php do I need to remove?

{
$context['config_vars'][$config_var[1]]['name'] .= '[]';
$context['config_vars'][$config_var[1]]['value'] = unserialize($context['config_vars'][$config_var[1]]['value']);

$context['config_vars'][$config_var[1]]['value'] = !empty($context['config_vars'][$config_var[1]]['value']) ? unserialize($context['config_vars'][$config_var[1]]['value']) : array();
}

$context['config_vars'][$config_var[1]]['value'] = unserialize($context['config_vars'][$config_var[1]]['value']);

Thanks BK. :)

Has this been fixed in the package manager upgrade yet?

No, and it won't be in this version. We need to release a next version patch to fix these issues. We can't have 2 different versions of the same...version :P

Ok, so dumb question; if we were to go ahead and manually fix any of the errors ourselves, won't that cause more errors when the next patch is released; as it would be looking for the fixes to this patch?

It will allow it to be skipped ;)
If the error is there, fix it. If not, skip

So, what should you do if you applied the patch before the bug was discovered? (I think I did it before the bug was discovered. D;)

@NekoJonez

Search "unserialize" in the ManageServer.php
You will find something like this:

Quote$context['config_vars'][$config_var[1]]['name'] .= '[]';
               $context['config_vars'][$config_var[1]]['value'] = unserialize($context['config_vars'][$config_var[1]]['value']);

               $context['config_vars'][$config_var[1]]['value'] = !empty($context['config_vars'][$config_var[1]]['value']) ? unserialize($context['config_vars'][$config_var[1]]['value']) : array();

And remove the red line


By the way, the functional impact is very limited and in many cases it should not cause any issue:
- Only affects configuration options in the admin panel that are managed using a <select> with multiple options (I don't know if the default SMF installation uses that, but some Mods do...)
- The configuration in the <select> multiple, is saved correctly in the database, so it's correctly taken into account for the forum behavior.
- The configuration is not displayed after reloading the page in the Admin panel. So it's impossible to know the current configuration unless you fix this. This is the only impact that I noticed (appart from the errors appearing in the log)

Thanks, the red line was indeed still there.

neko...   there was not update - so the line will be there for all versions of 2.0.9 made from the patch

I tried two times to install from the ACP. I get the following error.

Code: (Find) [Select] * @version 2.0.8Code: (Replace) [Select] * @version 2.0.9


Code: (Find) [Select] if (strpos($db_string, 'GROUP BY') !== false && strpos($db_string, 'ORDER BY') === false && strpos($db_string, 'INSERT INTO') === false)Code: (Replace) [Select] if (strpos($db_string, 'GROUP BY') !== false && strpos($db_string, 'ORDER BY') === false && preg_match('~^\s+SELECT~i', $db_string))

Ideas?

Thanks

Ron..

Quote from: Kindred on October 07, 2014, 07:58:06 PM
neko...   there was not update - so the line will be there for all versions of 2.0.9 made from the patch

Yay for derpiness.

I tried the patch using the Package Manager and got a "Test Failed" message on one file, .sources/display.php. In the "Replace" text on the failed line is says "Find: $context['show_view_results_button'] = $context['allow_vote'] && (!$context['allow_poll_view'] || !$context['poll']['show_results'] || !$context['poll']['has_voted']);
"and "Replace: $context['show_view_results_button'] = $context['allow_vote'] && $context['allow_poll_view'] && !$context['poll']['show_results'];
"The issue is the "Find" line is not there. There is a line that is almost exactly the same with the exception it has ['poll'] in front of each segment, as follows: $context['poll']['show_view_results_button'] = $context['poll']['allow_vote'] && (!$context['poll']['allow_poll_view'] || !$context['poll']['show_results'] || !$context['poll']['has_voted']);

Should I go ahead and run the install and then manually remove the line with the "poll" references and replace it with the "Replace" line, or insert the "poll" bit in front of each segment in the replacement line?

I guess I should also say I have the mod Additional Polls installed. Could that be what placed the "poll" bits in the code?

Thanks...

I will take a wild guess and say that you should install the patch ignoring that error.
The mod will probably not care about the changes introduced by the update patch, so... ;)