Work at SourceForge, help us to make it a better place! We have an immediate need for a Support Technician in our San Francisco or Denver office.

Close

#2700 functions/db_prefs use wrong PostgreSQL syntax

Produces PHP errors
closed-fixed
nobody
Options (155)
5
2010-07-21
2010-02-01
bubak
No

In setKey there is wrong syntax for PostgreSQL. "user" (the default column name) is keyword in PostgreSQL and should be escaped (ie. every operation with columns should be escaped, of course :).

I am sending a very simple patch against 1.4.19 version

P.S. this could also be relevant http://sourceforge.net/tracker/index.php?func=detail&aid=521833&group_id=311&atid=300311

Discussion

  • Thank you for your contribution. You're right, it's a good idea to quote the column names in the SQL. However, if this caused some issue, it should cause an issue in fillPrefsCache() and deleteKey() as well. What problem do experience, exactly, and why is it that you only see the issue upon preference save?

    Also, please submit patches as unified diff files. Thank you.

     
  • bubak
    bubak
    2010-02-01

    apply with 'patch -p0 db_prefs.php.orig <db_prefs.php.patch''

     
    Attachments
  • bubak
    bubak
    2010-02-01

    you are completely right, the new patch handles reading, deletion and update of the records (and is in uniffied diff). i missed it just because we have 5 o'clock in the morning and i am really tired; i made the patch without testing it properly and without knowing all the consequenses. i am unable to check the behaviour on anything else then Postgres right now, which works fine. i am not a php programmer, but in for example in Java there is a special class to handle such escapes -- using sprintf to generate valid sql requires much more attention and is not a very good practice imho. anyway, time to go to bed. kind regards, martin

     
  • It should be noted that double quotes causes MySQL to not return any results at all. MySQL uses back ticks (`) as the quote character for object names (table, column, etc).

    This brings up an interesting point, why aren't we using the PEAR libraries properly to do all dirty work? I believe there is a function built into the DB classes for this, quoteIdentifier(). This would make the patch look a little like this:

    $this->dbh->quoteIdentifier($this->table),
    $this->dbh->quoteIdentifier($this->user_field),

    Of course, PHP recommends against using it, but then again, most DB documentation recommends against using table, and field, names that require using it too, like not using keywords for your field names.

     
    • status: open --> closed-fixed