Jonathan Ernst wrote:
Hi,
I followed your discussions about sql/html injection and had a look at what you are doing in order to fix the situation and I don't want to offense anybody but I feel that what you are trying to do is a bit hacky ;-).
Here is a cleaned up version of the comments handling that better fixes (imho) all problems related to sql and html injection.
The code I made is not dependent on magic_quote value and should'nt allow anybody to mess with the values read or written from/to the database. Also it doesn't require to make copies of every single string the user passes to us.
It is based on php manual's best practices for avoiding injection.
I'd be very glad if we'll use such mechanism for the rest of the queries and get rid of compile_whatever, makeSafe & co.
This patch follows the Coding Standards we set up a little bit more as well.
Awaiting your comments.
I am more in favor of this approach than using makeSafe() and do the same thing as Chris's query_parameters() patch. However I am hard pressed to say whether this method is really better or safer than query_parameters().
I have had real issues with makeSafe() the primary one is that IMO the place to make sure that we are safe from sql injection is where we create the SQL. The makeSafe() did not do that.
The changes to that are only cosmetic and make it harder to see the actual changes. Formating changes should be in a separate patch.
Also I would really appreciate a "Files Changed:" section that lists the files changed/added/removed by this patch. I find that it really helps in reviewing patches.
Please resubmit with the formating changes in a separate patch.
--
Tony Lambregts