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

CommentFileSizeAuthor
#2 drupal-6.x-dev-menu.module-0.2.diff3.64 KBhswong3i
drupal-6.x-dev-menu.module-0.1.diff3.62 KBhswong3i

Comments

hswong3i’s picture

hswong3i commented
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.

hswong3i’s picture

hswong3i commented
StatusFileSize
new drupal-6.x-dev-menu.module-0.2.diff3.64 KB

minor update about %s, thanks for dmitrig01 :)

dmitrig01’s picture

dmitrig01 commented
Status: Needs review » Reviewed & tested by the community
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
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.

profix898’s picture

profix898 commented

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 ;)

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

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

pwolanin’s picture

pwolanin commented

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.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)