First, note that I reproduced this again HEAD, but I selected 4.7.0-beta2 as the Version, because 'cvs' appears many times in the version drop-down and I was afraid this would get lost if I selected the wrong 'cvs'.

The problem is the preg_replace on line 281 of database.inc can produce bad SQL. I inserted some code to print what the query looks like before and after the preg_replace, and here's what happens:

Query before preg_replace():

SELECT t.tid, t.*, parent FROM {term_data} t, {term_hierarchy} h WHERE t.tid = h.tid AND t.vid = %d ORDER BY weight, name

And after the preg_replace():

SELECT t.tid, t.*, parent FROM {term LEFT JOIN {term_data} catd ON t.tid = catd.tid _data} t, {term_hierarchy} h WHERE t.tid = h.tid AND t.vid = %d ORDER BY weight, name

As you can see, it inserted my join smack in the middle of {term_data}.

I made this small change to the preg_replace call, and it seems to fix the problem in my case. Not sure yet whether it breaks anything. Original code:
$query = preg_replace('|FROM[^[:upper:]/,_]+|','\0 '. $join .' ', $query);
My change eliminates ",_" from the regular expression:
$query = preg_replace('|FROM[^[:upper:]/]+|','\0 '. $join .' ', $query);

This puts my join after the list of tables and before the WHERE, which I think is where it belongs. Perhaps there is some reason for the ",_" in the regexp, but I don't see it.

It also seems to me the regexp will not work properly if "WHERE" were lower case.

CommentFileSizeAuthor
#9 db_rewrite_sql_3.patch624 byteschx
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chx’s picture

Let's delete underscore but do not delete the comma.

Dave Cohen’s picture

If you don't delete the comma, the join gets thrown in between "{term_data} t" and ", {term_hierarchy}". I think it belongs after both of those, and before the WHERE.

(Although mysql did not generate an error when the join was placed there.)

Wesley Tanaka’s picture

possibly a dupe of http://drupal.org/node/38298

Steve Ratcliffe’s picture

This bug was introduced recently in http://drupal.org/node/39257

chx’s picture

Assigned: Unassigned » chx

Assigning to myself, I am scheduling writing a new regexp. We will add the JOIN before the first JOIN. Stay tuned.

drewish’s picture

chx, while you're in there could you update the function's comment to make it more descriptive? Something like: "Rewrites node queries to enforce access control. Only nodes to which the user has access permissions will be returned."

chx’s picture

No. hooks docs live on contrib and not in code. Also it's also for taxonomy and now comments.

drewish’s picture

I wasn't refering to the hook, but to the function itself. Currently it's simply:
Rewrites node queries.
That does a pretty crappy job of explaining what it's supposed to do. And as I'm sure you know the documentation is drawn from the comments in the code.

chx’s picture

Status: Active » Needs review
FileSize
624 bytes

Totally new regexp. Very simple, instead of putting it after the table name, we put the join expression before the first JOIN or whatever. Added some comments.

Cvbge’s picture

What if there's no "JOIN or whatever" in the query?

chx’s picture

see |$ at the end of preg: "or the end of string"

Dave Cohen’s picture

+1

Seems to work for me.

chx’s picture

Seems work to me means nothing. Which modules have you tried?

Dave Cohen’s picture

Well, I mean that it fixes my problem which is the original reason I submitted this bug.

I'm using an access control module that I built myself. I'm happy to share it, but right now its not published anywhere. (I hoped to check it into drupal contrib, but because there is already taxonomy access control I was not granted a CVS account.)

chx’s picture

Status: Needs review » Reviewed & tested by the community

If you are running a node access module then, I think, it's good to go.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to HEAD. Thanks.

Dries’s picture

Status: Fixed » Closed (fixed)