Simetrical wrote:
On Thu, May 8, 2008 at 10:28 AM, DanTMan dan_the_man@telus.net wrote:
Ok, then how about we draft up a short list of symbols to test, or not test against. Simplifying down to word breaks or such leaves with to much that could break.
Such as? Nothing adhering to anything resembling normal table-naming practices will be affected.
As for spaces inside of table names, I'm pretty sure anyone insane enough to use a space inside of a name is already going to be having issues with SQL. If you're using ` on ` as a table name, you're pretty sure things are going to break if you don't go and quote that yourself. Underscores or dashes are always preferred over spaces.
It's just as valid as `-on-`, and probably just as likely. For that matter, `on` is a valid table name, which your regex would fail on too (as would mine).
Of course, you could just be more complete, and exclude only a case like
/\s*$tablenameregex\s+((LEFT|INNER|OUTER|CROSS|LEFT\s+OUTER|RIGHT|RIGHT\s+OUTER|FULL\s+OUTER)\s+)?JOIN\s+$tablenameregex\s*/
where $tablenameregex is something like
([a-z0-9_]*|`[^`]*`|"[^`]"|[[^]]])
In other words, you could parse the SQL. Just make sure you do it correctly. :)
`table` is already handled before the test for tablename stuff. We're just trying to see if anything that is part of a SQL query shows up inside. So completeness isn't what's being tested. You noted the use of () next to ON, rather than just whitespace. Any similar types of characters? If not all that would be needed is to add in () and it should be fine: "/(^|\s|))(JOIN|ON|AS)((|\s|$)/i" Technically... = should never really show up outside either... Or maybe we're overcomplicating this...
/((\S{,64}|`[^`]{,64}`).)?(\S{,64}|`[^`]{,64}`)/iS All we really want is to know if the input is valid. And all we're trying to catch is: - table - database.table And the variants of them with some amount of backticks. Yes " is usable if ANSI_QUOTES is enabled, but that is in no way widely compatible with MySQL itself. So none of the code should ever output that. Additionally tables and databases are not allowed any more than 64 characters, not to mention we shouldn't be using ones that long.
That regex should match any valid input, and exclude any invalid input which would simply be outputted.
What I find horribly disturbing is this:
|$res = $db->query( "SELECT title FROM $tablename", 'SMW::getQueryResult:DEBUG');
|
That's a blatant select statement which should be sent through $db->select.
Yes, there are a fair few of those in core too.
But I'll look into improvements into Database methods sometime later. I'd honestly like to abstract queries away from raw SQL into using methods. We're doing PHP, not SQL, let the SQL portion of the code handle SQL while we write in PHP. Doing it right could also potentially allow the complex things like SMW, API, and DPL which build complex queries to abstract those as well with the helper functions, or heck... We could come up with some sort of actual Query builder class.
Personally I would caution against getting too abstract. The current database abstraction layer uses familiar PHP constructions like arrays in a very natural way, on the whole. Adding tons of methods like join() is not necessarily a great way to go.
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
~Daniel Friesen(Dantman) of: -The Gaiapedia (http://gaia.wikia.com) -Wikia ACG on Wikia.com (http://wikia.com/wiki/Wikia_ACG) -and Wiki-Tools.com (http://wiki-tools.com)