In node_load, while going through $param, there is a possibility that an integer parameter will have a null value. When this happens, because it doesn't meet the is_numeric() test, it goes through the string path, and you can get comparisons like WHERE n.nid = '', which is of course nonsensical (nid being an integer), though I think MySQL will silently take it anyway.
The attached patch fixes the issue, tested against PostgreSQL 8.1 for the last several hours, and it doesn't seem to be causing any problems. Someone with a MySQL Drupal install should make sure that MySQL doesn't do anything particularly strange with it. Without it, pgsql throws frequent errors looking like:
pg_query() [function.pg-query]: Query failed: ERROR: invalid input syntax for integer: "" in /usr/local/share/drupal-4.7.4/includes/database.pgsql.inc on line 94.
query: SELECT n.nid, n.vid, n.type, n.status, n.created, n.changed, n.comment, n.promote, n.moderate, n.sticky, r.timestamp AS revision_timestamp, r.title, r.body, r.teaser, r.log, r.format, u.uid, u.name, u.picture, u.data FROM node n INNER JOIN users u ON u.uid = n.uid INNER JOIN node_revisions r ON r.vid = n.vid WHERE n.nid = '' in /usr/local/share/drupal-4.7.4/includes/database.pgsql.inc on line 113.
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | node_module_null_int_1.patch | 701 bytes | Zed Pobre |
| #2 | node_module_null_int_0.patch | 577 bytes | Zed Pobre |
| node_module_null_int.patch | 573 bytes | Zed Pobre |
Comments
Comment #1
ChrisKennedy commentedGreat catch. This may be related to the problem at http://drupal.org/node/100181
You need a space after the "if". Also comparing $value to '' will incorrectly use "IS NULL" if $value equals zero. So you need to compare with !== or use is_null.
Comment #2
Zed Pobre commentedI'm attaching a new patch, using
is_null(). I'd like to know how you planned to solve this with !==, though, just out of coding curiosity. It doesn't seem to be immediately applicable.Comment #3
Zed Pobre commentedActually, after thinking about this some more, I think even that change is incomplete, because you also want to catch the case where the variable is not an array, but potentially equal to
''(or zero, thanks to PHP's terrifying form of weak typing). It's okay to conflate zero and''here, because the absence of an array implies node ID, and node ID cannot be equal to zero anyway.Of course, I think it's also always supposed to be NOT NULL as well.
I'm not sure if we want to silently cover cases where someone is trying to compare n.nid to
'', or just force that error back into the logs to find a buggy module, but here's a third patch that does cover that case.Comment #4
drummThe nid can't be null in the database and the documentation for the function mentions nothing about passing in empty strings for $param because that would be wrong.
Comment #5
tstoecklerI think since NULL is not a valid nid, this should really by closed (works as designed)