Bugs in PEAR::DB

From: Date: Tue, 10 Jul 2001 20:03:55 +0000
Subject: Bugs in PEAR::DB
Groups: php.pear.dev 
Request: Send a blank email to pear-dev+get-627@lists.php.net to get a copy of this message
Hi, I've been using the DB module in a project and found a few glitches. First of all, prepare/execute has a bug around this part: function executeEmulateQuery($stmt, $data = false) { $p = &$this->prepare_tokens; $stmt = (int)$this->prepare_maxstmt++; if (!isset($this->prepare_tokens[$stmt]) || !is_array($this->prepare_tokens[$stmt]) || !sizeof($this->prepare_tokens[$stmt])) { return $this->raiseError(DB_ERROR_INVALID); } * $this->prepare_maxstmt is never initialized. This results in a warning. * Whatever the value of $this->prepare_maxstmt is, it blatantly overwrites the passed in $stmt value... Why? Not only does this make passing in the statement handle useless, but due to the increment, multiple calls to execute result just go thru all the prepared statements, and after those end (which happens on 2nd execute call if prepare has been only called once), errors start occuring (I assume execute returns a DB error object). Since the execute emulation code does not check the return value of 'execute' and just executes that return value, the SQL server (in my case, MySQL), ends up being asked to execute SQL statement "Object". The error is only checked after simpleQuery is executed, and so what is trapped is SQL server complaning about an invalid query and not DB::common::executeEmulateQuery about a bogus statement handle. * I failed to figure out what $this->prepare_maxstmt uses, so just commenting out the $stmt = (int)$this->prepare_maxstmt++; line fixes the problem and makes prepare/execute work. Not really an error, but why does executeEmulateQuery passes the given data thru DB::common::quoteString, while the DB::common::query doesn't? This is the code: $realquery .= "'" . $this->quoteString($pdata) . "'"; Are users of DB::common::query expected to do it by themselves? Why aren't they then expected to manually quoteString the array that they pass in to execute? Thing is, if the users do quoteString (or the equivalent) the passed in array, then executeEmulateQuery ends up quoting it again. On many installations, PHP also automatically quotes the POST and GET information. In order to store such automatically-quoted information with executeEmulateQuery, the users would be forced to unquote the data before passing it in to execute... that's silly, if you ask me. Finally, the quoteString implementation is not adequate: function quoteString($string) { return str_replace("'", "\'", $string); } If a string "I\'ve done it." is passed in to quoteString (which may happen for whatever reason, either intentionally [hack attempt?] or due to PHP magically quoting the CGI vars), then the output is "I\\'ve done it." When this string is used within execute, this results in a query that looks like "something='I\\'ve done it'". In MySQL, \ ends up escaping \ and the quote is treated as is, ending up passing something='I\' and garbage "ve done it'" immediatelly afterwards. (I encountered this error when I tried using prepare/execute w/ MySQL). There should be either a MySQL-specific or any other database-specific quoteString implementation. In fact, I don't see why just not just use addslashes: function quoteString($string) { return addslashes($string); } This should probably be database-specific (different databases may escape strings differently), so I added this function to DB::mysql and with the above prepare/execute fix (well, I hope it doesn't break anything else), prepare/execute works nicely. The extra escaping was unacceptable to me, so I had to remove that out of the executeEmulateQuery. With these three changes, prepare/execute works just right in my application. - Oleg

« previous php.pear.dev (#627) next »