-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
greg@svn.wikimedia.org wrote:
- /**
* Returns true if this database uses timestamps rather than integers- */
- function realTimestamps() {
return $this->mRealTimestamps;- }
If something like this is necessary at all, why bother with a member variable? The function should simply be overridden appropriately.
- /* Offset must be an integral. */
- if ( !strlen( $options['offset'] ) || !preg_match( '/^[0-9]+$/', $options['offset'] ) )
$options['offset'] = '';
- /* Offset must be an integral, unless the db is using timestamps */
- $dbr =& wfGetDB( DB_SLAVE );
- if ( !strlen( $options['offset'] ) ||
( !$dbr->realTimestamps() && !preg_match( '/^[0-9]+$/', $options['offset'] ) ) )- $options['offset'] = '';
This looks like it may be unsafe; the code here is pretty hairy and I didn't see any _other_ validation of input.
1) Confirm you didn't introduce an SQL injection attack vector on PG systems.
2) Consider simply fixing the code to properly validate input and produce portable output, the way it's supposed to.
There should be no need to check or care what internal format the database uses when dealing with form parameters.
- -- brion vibber (brion @ pobox.com)