Bug in upgrade package from 2.0.8 to 2.0.9 (smf_patch_1.1.20_2.0.9.zip)

Started by Ferny, October 05, 2014, 01:01:37 PM

Previous topic - Next topic

Ferny

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):

<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).
Digital Video & Audio:
www.mundodivx.com


Kindred

Hmmm...  BK, why the link? Pall of the necessary information was listed here already.  Unneeded.
Сл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."

Burke ♞ Knight

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.

Steve

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();
}
DO NOT pm me for support!

Burke ♞ Knight

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

Steve

DO NOT pm me for support!

SabreOfParadise


margarett

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
Se forem conduzir, não bebam. Se forem beber... CHAMEM-ME!!!! :D

QuoteOver 90% of all computer problems can be traced back to the interface between the keyboard and the chair

br360

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?

margarett

It will allow it to be skipped ;)
If the error is there, fix it. If not, skip
Se forem conduzir, não bebam. Se forem beber... CHAMEM-ME!!!! :D

QuoteOver 90% of all computer problems can be traced back to the interface between the keyboard and the chair

NekoJonez

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;)
Retro video game blogger, writer, actor, podcaster and general amazing dude.

Twitter
My Blog

Ferny

@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)
Digital Video & Audio:
www.mundodivx.com

NekoJonez

Retro video game blogger, writer, actor, podcaster and general amazing dude.

Twitter
My Blog

Kindred

neko...   there was not update - so the line will be there for all versions of 2.0.9 made from the patch
Сл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."

Ronald_1938

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..

NekoJonez

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.
Retro video game blogger, writer, actor, podcaster and general amazing dude.

Twitter
My Blog

nivlac

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...

margarett

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... ;)
Se forem conduzir, não bebam. Se forem beber... CHAMEM-ME!!!! :D

QuoteOver 90% of all computer problems can be traced back to the interface between the keyboard and the chair

Advertisement: