Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.$menu_link = db_fetch_array(db_query("SELECT * FROM {menu_links} WHERE customized = 0 AND link_path = '%s'", $path));
menu_link_save($menu_link);
this serializes options again, thus l breaks. This is definitely not something I would expect. But the fix is downright trivial.
| Comment | File | Size | Author |
|---|---|---|---|
| options_double_serialized.patch | 1.34 KB | chx |
Comments
Comment #1
pwolanin commentedsubscribe - a couple thoughts. cache_set uses the opposite kind of check: is_array().
Also, if we want to be really safe, we should check that any such string unserializes to a valid array. If you pass in options that are not an array, l() breaks in an unfortunate WSOD fashion.
Comment #2
chx commentedWe do not really babysit broken code and if you pass in options which is a random string then you are hosed. There are many ways to break Drupal with broken code. However, the example code is hardly broken so I would say the patch is good.
Comment #3
dries commentedIsn't it odd that the options can be either serialized or not? One would expect that these would be in a consistent state (unserialized).
Comment #4
pwolanin commented@Dries - I'm not necessarily in favor of this, but there is an argument for it in terms of efficiency. Karoly wants this for cases where he wants to make small updates (like the title) for several menu links.
The alternative is to do one query to find the mlid values, and then call the API function menu_link_load() for each mlid, before changing the title and calling menu_link_save().
Comment #5
gábor hojtsyThis awaits on Dries' decision.
Comment #6
dries commentedI don't think we've researched this enough to make a decision.
pwolanin: I'd think that re-using our own APIs is a good thing? Is this code in the "critical path" (i.e. performance bottleneck)? If the answer is "no", I'd favor to use our own APIs. If the answer is "yes", that might be a valid reason not to use our own APIs.
Comment #7
chx commented