Piwigo.org

plg · 2014-11-12 10:50:58

Hello,

For a few weeks we have encountered the bug Notice: unserialize(): Error at offset 0 of 311 bytes in /home/.../public_html/include/functions.inc.php on line xxx. For example in [Forum, topic 24756] Notice: unserialize(): Error at offset

I have added some traces on Piwigo.com to try to understand where it comes from. After one week the problem occured once more and now I understand what breaks ContactForm configuration.

In my example, the problem occured after a user activated theme [extension by flop25] Stripped & Columns. Here is why:

1) ContactForm is activated and its configuration is clean, stripped&columns theme is deactivated (but old version)

2) the user sets theme Stripped&Columns as default theme (config unchanged)

3) the user opens the gallery, in stripped_black_bloc/admin/upgrade.inc.php we detect less than 8 parameters, so it runs an "upgrade" and executes load_conf_from_db()

4) the load_conf_from_db() resets the $conf['ContactForm'] and when this plugin does:

Code:

if ($conf['ContactForm']['cf_must_initialize'])

the results is true, and contact_form_initialize_emails() is called and corrupts the ContactForm configuration.

I have locally fixed the problem with some changes in stripped&columns, mainly :

Code:

-  conf_update_param('stripped_black_bloc', pwg_db_real_escape_string(serialize($config)));
-  load_conf_from_db();
+
+  conf_update_param('stripped_black_bloc', $config, true);

But we could also change plugin ContactForm to use a:

Code:

$conf['ContactForm'] = safe_unserialize($conf['ContactForm']);

at the beginning of function contact_form_init().

PS : the same kind of problem occurs with [extension by Serge D] GreyDragon Theme

What do you think?

mistic100 · 2014-11-12 11:12:56

So I was right when I said a few months ago it is bad to call load_conf_from_db() anywhere else than in common.inc :-)

now more than ever after adding the third parameter of conf_update_param

plg wrote:
But we could also change plugin ContactForm to use a:
Code:
$conf['ContactForm'] = safe_unserialize($conf['ContactForm']);
at the beginning of function contact_form_init().

it was the case, but I tend to to move this kind a code to the "root" of main.inc.php for every plugin (conf ready sooner)

mistic100 · 2014-11-12 12:08:35

@flop: load_conf_from_db as an optional parameter to reload only one parameter if you really want to use it

flop25 · 2014-11-12 13:30:05

conf_update_param('stripped_black_bloc', $config, true); is ok ; don't know when that third paramter was introduced but that's indeed much cleaner
As always you can commit -that's the point of having a versionning system, the author can still revert easily, but some people still think like if it was not digital as I learnd by experience-
elegant and smartpocket should be updated too

plg · 2014-11-12 14:38:10

Stripped&Columns, elegant and SmartPocket modified.

flop25, can you release a new version of Stripped&Columns please? (and also Stripped, which I modified for other unrelated bugs)

flop25 · 2014-11-12 15:29:05

ok I check this evening and release

Serge D · 2014-11-12 19:33:39

plg wrote:
I have locally fixed the problem with some changes in stripped&columns, mainly :
Code:
-  conf_update_param('stripped_black_bloc', pwg_db_real_escape_string(serialize($config)));
-  load_conf_from_db();
+
+  conf_update_param('stripped_black_bloc', $config, true);

Let me ask you this
In my theme I do not need to "upgrade" the theme's config, but I do need to initialize the settings record within upgrade.inc.php
Currently code is

Code:

if (!isset($conf['greydragon'])):
  // Only need to ensure that there is a record in DB. Theme supports self initialization
  $query = "INSERT INTO " . CONFIG_TABLE . " (param, value, comment) VALUES ('greydragon' , '" . pwg_db_real_escape_string(serialize(array())) . "' , 'GreyDragon Theme Options');";
  pwg_query($query);
  load_conf_from_db();
else:
  // Nothing to upgrade
endif;

do I understand that code above need to be modified as

Code:

if (!isset($conf['greydragon'])):
  // Only need to ensure that there is a record in DB. Theme supports self initialization
  conf_update_param('greydragon', array(), true);
else:
  // Nothing to upgrade
endif;

PS: This would not set "Comment" field value in DB though

Last edited by Serge D (2014-11-12 19:34:38)

flop25 · 2014-11-12 19:45:02

ps: since the beginning i keep asking to get the comment registered too but...

plg · 2014-11-12 20:23:09

Yes, Serge D it is the right change to apply. And no, it doesn't fill piwigo_config.comment and it is not very useful anyway. We may add a new parameter on conf_update_param to set this useless comment.

mistic100 · 2014-11-12 21:22:30

You may also use

Code:

if (!isset($conf['greydragon'])):
  $conf['greydragon'] = serialize(array());
  // Only need to ensure that there is a record in DB. Theme supports self initialization
  $query = "INSERT INTO " . CONFIG_TABLE . " (param, value, comment) VALUES ('greydragon' , '" . pwg_db_real_escape_string($conf['greydragon']) . "' , 'GreyDragon Theme Options');";
  pwg_query($query);
else:
  // Nothing to upgrade
endif;

you save the full reload of config, and have a way nicer semantic

Serge D · 2014-11-13 01:29:03

mistic100 wrote:

You may also use

Code:

if (!isset($conf['greydragon'])):
  $conf['greydragon'] = serialize(array());
  // Only need to ensure that there is a record in DB. Theme supports self initialization
  $query = "INSERT INTO " . CONFIG_TABLE . " (param, value, comment) VALUES ('greydragon' , '" . pwg_db_real_escape_string($conf['greydragon']) . "' , 'GreyDragon Theme Options');";
  pwg_query($query);
else:
  // Nothing to upgrade
endif;

you save the full reload of config, and have a way nicer semantic

I thought all discussion above was about to standardize on the code.
if there is a function which is doing db options manipulation, it should be used

plg · 2014-11-13 08:36:42

I think the point of mistic100 is not to avoid using conf_update_param but to initialize $conf['greydragon'] instead of relying on the "magic" of conf_update_param which will fill the same variable, so maybe the best solution is :

Code:

$conf['greydragon'] = array();
conf_update_param('greydragon', $conf['greydragon'], true);

plg · 2014-11-13 08:38:53

Serge D wrote:
if there is a function which is doing db options manipulation, it should be used

I'm sure mistic100 likes conf_update_param :-) see how he improved the function in Piwigo 2.7 [Subversion] r28567

mistic100 · 2014-11-13 09:16:50

plg wrote:
I think the point of mistic100 is not to avoid using conf_update_param but to initialize $conf['greydragon'] instead of relying on the "magic" of conf_update_param which will fill the same variable, so maybe the best solution is :
Code:
$conf['greydragon'] = array();
conf_update_param('greydragon', $conf['greydragon'], true);

no that's not my point

my point is to not rely on the magic of load_conf_from_db to update the $conf object with the value we just inserted in base.

--

otherwise I would not had improved conf_update_param
I suggested you to keep the manual insert if you want to have a comment in the database

--

IMHO, the best solution in 2.7 is, everywhere, anytime

Code:

conf_update_param('greydragon', $my_new_conf, true);
// or if you init empty
conf_update_param('greydragon', array(), true);

but no comment

Serge D · 2014-11-14 04:43:57

plg wrote:
I think the point of mistic100 is not to avoid using conf_update_param but to initialize $conf['greydragon'] instead of relying on the "magic" of conf_update_param which will fill the same variable, so maybe the best solution is :
Code:
$conf['greydragon'] = array();
conf_update_param('greydragon', $conf['greydragon'], true);

it was not clear,
Done

Piwigo.org

Announcement

#1 2014-11-12 10:50:58

load_conf_from_db in theme may break ContactForm

Code:

Code:

Code:

#2 2014-11-12 11:12:56

Re: load_conf_from_db in theme may break ContactForm

plg wrote:

Code:

#3 2014-11-12 12:08:35

Re: load_conf_from_db in theme may break ContactForm

#4 2014-11-12 13:30:05

Re: load_conf_from_db in theme may break ContactForm

#5 2014-11-12 14:38:10

Re: load_conf_from_db in theme may break ContactForm

#6 2014-11-12 15:29:05

Re: load_conf_from_db in theme may break ContactForm

#7 2014-11-12 19:33:39

Re: load_conf_from_db in theme may break ContactForm

plg wrote:

Code:

Code:

Code:

#8 2014-11-12 19:45:02

Re: load_conf_from_db in theme may break ContactForm

#9 2014-11-12 20:23:09

Re: load_conf_from_db in theme may break ContactForm

#10 2014-11-12 21:22:30

Re: load_conf_from_db in theme may break ContactForm

Code:

#11 2014-11-13 01:29:03

Re: load_conf_from_db in theme may break ContactForm

mistic100 wrote:

Code:

#12 2014-11-13 08:36:42

Re: load_conf_from_db in theme may break ContactForm

Code:

#13 2014-11-13 08:38:53

Re: load_conf_from_db in theme may break ContactForm

Serge D wrote:

#14 2014-11-13 09:16:50

Re: load_conf_from_db in theme may break ContactForm

plg wrote:

Code:

Code:

#15 2014-11-14 04:43:57

Re: load_conf_from_db in theme may break ContactForm

plg wrote:

Code:

Board footer