Had a bug in some module where theme_links was called but '$links' was not an array.
Ofcourse the real bug was in the module, but I theme_links should maybe not produce ugly errors if this happens?

--- theme.inc.orig      2005-03-13 14:51:45.829133700 +0100
+++ theme.inc   2005-03-13 14:46:55.000000000 +0100
@@ -432,7 +432,9 @@
  *   A string containing the themed links.
  */
 function theme_links($links, $delimiter = ' | ') {
-  return implode($delimiter, $links);
+  if (is_array($links)) {
+    return implode($delimiter, $links);
+  }
 }

 /**

Comments

chx’s picture

I do not think it is the task of core to correct the incorrect work of non-core modules.

Uwe Hermann’s picture

IMHO core should be programmed defensively and robust, and not fail on errors which it could detect and react upon properly.

danielc’s picture

Uwe makes a very good point.

jvandyk’s picture

Status: Needs review » Closed (works as designed)

Are we going to put checks in every function for every parameter, to make sure it is what the docs say it should be? I don't think so.

Andreas Wolf’s picture

I do ABSOLUTELY NOT agree with jvandyk.

Are we going to put checks in every function for every parameter, to make sure it is what the docs say it should be? I don't think so

I think you should for the following reasons:

1. The function theme_links uses the PHP function implode which is defined string implode ( string glue, array pieces ).
The inappropriate use (eg. $links is not an array) of a core function should never raise a PHP error.
Therefore all functions should test variables before using them in a special context (here: array). If the test fails the function has to return a meaningful return value, as this is written down in the Drupal Coding Standards.

2. The docs may be out of date or a function may be changed in the future. No (badly written or old) module should be allowed to crash the whole system. Therefore functions (especially core functions) have to perform these checks.

3. Generally, not checking what you could check is usually the reason for security issues.
Would you tell your customers: Hey, there is no problem. That was no hacker. Just someone who did not read the manual and used my function in a way I did not intend ?

Following the Drupal Coding Standards (http://drupal.org/node/318) :
Always attempt to return a meaningful value from a function if one is appropriate.

I suggest the following extension to the patch:

function theme_links($links, $delimiter = ' | ') {
  if (is_array($links)) {
     return implode($delimiter, $links);
  }
  else
  {
      return $links;
   }
}