update menu module as SQL friendly

hswong3i - August 12, 2007 - 05:11
Project:Drupal
Version:6.x-dev
Component:database system
Category:bug report
Priority:critical
Assigned:hswong3i
Status:closed
Description

minor update about query format. P.S. we need to use db_query() correctly for cross database concern :)

AttachmentSize
drupal-6.x-dev-menu.module-0.1.diff3.62 KB

#1

hswong3i - August 18, 2007 - 08:57
Priority:normal» critical

since both Oracle/DB2/MSSQL will preform A LOT OF reserved word rewrite handling to query BODY, this patch can greatly improve the ability of cross database compatibility. This is because all user input values are escaped, and will not capture by rewrite handling.

#2

hswong3i - August 20, 2007 - 03:43

minor update about %s, thanks for dmitrig01 :)

AttachmentSize
drupal-6.x-dev-menu.module-0.2.diff 3.64 KB

#3

dmitrig01 - August 20, 2007 - 03:45
Status:needs review» reviewed & tested by the community

#4

Gábor Hojtsy - August 20, 2007 - 18:20
Status:reviewed & tested by the community» fixed

I see the general push to move literal stuff out of queries could help database compatibility, although fail to see in this case, what character might be infected by any DB escaping. Anyway, committed just to be in-line with this process.

#5

profix898 - August 20, 2007 - 19:06

Gábor wrote : "I see the general push to move literal stuff out of queries could help database compatibility"

Actually this is not documented anywhere AFAIK, at least it is not documented in the module update handbook page (http://drupal.org/update/modules). With all these SQL-compatibility patches going into core we will have core ready, but you will hardly find any contributed module that will not break your site immediately ;)

#6

Gábor Hojtsy - August 20, 2007 - 19:33

We'v discussed this in the installer issue (which AFAIR is not yet committed). This change in policy should definitely be documented.

#7

pwolanin - August 20, 2007 - 20:04

hmmm, this code may need to be cleaned up a little in addition - it assumes the 'navigation' menu- but that was written when we didn't allow the link to move outside of that.

#8

Anonymous - September 4, 2007 - 11:31
Status:fixed» closed
 
 

Drupal is a registered trademark of Dries Buytaert.