Invalid hook_schema() result can trash entire schema

This is true of theme registry as well. I think a little defensiveness like this is a good thing.

CNW because drupal_get_schema() move into boostrap.inc.

See: #335122: Test clean HEAD after every commit and http://pastebin.ca/1258476

If the schema is NULL, I don't think we need to do the require. How about this?

We can probably go as far as confirming that $current has a count() >= 1, just to be extra safe.

#305430: Prevent schema corruption was a duplicate.

Here's a new patch, including count()

I believe this to be a very dangerous practice. Where will this stop? How many lines of code will we add which shields us from bad modules? There are many, many ways for a broken module to mess up your Drupal. This is summed up as "we do not babysit broken code".

Can you cast it to an array?

$current = (array) module_invoke($module, 'schema');

That's not even 1 line of babysitting code - it's 7 characters.

Agreed with Robert. Casting to an array is the way to go.

Plus I don't see how the patch in #10 solves the issue described in the original post ("passing NULL to array_merge() results in NULL").

casting is fine. it's a bit of ugly but yeah ok.

Casting sounds fine to me if that's easier. Probably needs a comment line to clarify it then, too.

I don't see that as babysitting broken code as much as keeping a typo from totally hosing an entire devel site. :-)

New patch, casting to an array.

So, I intentionally hacked poll_schema(). It threw a couple of warnings but felt good about itself and said it was installed. I installed profile at the same time as my hacked poll, and profile installed just fine. In that regards, I think the patch works fine.

fwiw, I'm with chx on this one, b/c people really just shouldn't be implementing hook_schema() unless they intend it to return a valid schema array. And if for some reason a schema fails to install, I don't think the module should even be marked as installed.

This is not along a critical path, and while one module dying because its author messed up is one thing, screwing up other modules is simply not acceptable. #17 looks good to me.

Sorry but that comment could be a bit more explanatory. Setting back to 'code needs work'.

Argh, this bit me when I stuck a placeholder hook_schema() in a module. Such an easy fix.

Attached original patch with expanded comment. Also see #617546: Invalid hook_schema() result can trash entire schema for the D6 patch.

Safety from bad hooks++

Comment looks good.

In general, I am against babysitting broken code as well, but Crell in #19 makes a good point that this doesn't just screw up your module (as when you forget to return $items from hook_menu()), this screws up everything.

Committed to HEAD.

Looks like this could be backported to D6 as well?

I can't express in words how much I concur that it should be backported to D6. Patch attached.

As this issue continues to plague Drupal 6.x nearly a year after this patch was submitted, I'm resubmitting cafuego's patch so that it can go through the automated testing.

I also discuss the issue and its solution in greater detail in my post at http://echodittolabs.org/blog/2010/09/butterfly-effect

+1
Confirmed this is a straight backport of the D7 commit.

bad bot! This is a D6 patch.

I'm not fun of babysitting code either, but there is just too much support here. Committed to Drupal 6.