Currently AMO->clean tends to corrupt input data unnecessarily (try to enter quotes in the homepage field). It is also a bad solution because it depends on the developers actually using it and people tend to forget. It is far better not to use dynamic SQL anywhere. The SQL queries should be fixed and only contain placeholders for parameters. The database driver will take care of all the necessary escaping once parameters are bound. See the documentation on prepared statements: http://php.net/mysqli_stmt_prepare. The catch is: it only works with PHP 5.0 and higher. Same goes for the PDO extension which has similar functionality. So that this change won't happen any time soon I guess. Btw, what is blocking the switch to PHP5? According to http://manual.cakephp.org/chapter/installing Cake works with PHP5.
I agree, using prepared statements would make this problem go away in the general case, though I'm surprised that you can't use prepared statements with php4, given a new enough version of mysql. I think there is a php5 setup on the cluster now, though there wasn't when we started on Remora, it might be worth looking at that switch once we get through this initial release.
I concur. There are probably a handful of reasons why switching to PHP5 is a good idea, and I believe this is one of the bigger ones.
> > I think there is a php5 setup on the cluster now, though there wasn't when we > started on Remora, it might be worth looking at that switch once we get through > this initial release. > Yep, php5 is available.
This may not be possible because we use memcache to return cached results...
Don't see why that would be a problem - AppModel.findAll() already serializes all function parameters into the cache key, AppModel.query() would do the same with query parameters.
CakePHP 1.2 uses prepped statements so that should solve the problem when we upgrade (timeline TBA). This change will also require a rewrite of our query caching in memcache. Should probably be dependent on 425315, or at least done at the3 same time as that bug.
When we pass unescaped data to the models (which is okay), and Cake will escape all its queries properly, we need to make sure the translation framework does so as well: The automatic saving/updating of translations currently relies on getting passed escaped data from the controller and does clean it up itself.
Zamboni to the rescue.