mirror/Arsse
1
1
Fork 0
mirror of https://code.mensbeam.com/MensBeam/Arsse.git synced 2025-01-03 14:32:40 +00:00
Code Issues Activity

Munge PostgreSQL queries instead of adding explicit casts

Browse source 
PDO does not adequately inform PostgreSQL of a parameter's type, so type
casts are required. Rather than adding these to each query manually, the
queries are instead processed to add type hints automatically.

Unfortunately the queries are processed rather naively; question-mark
characters in string constants, identifiers, regex patterns, or geometry
operators will break things spectacularly.
This commit is contained in:
J. King 2018-11-29 13:45:37 -05:00
parent 4c8d8f1a52
commit 4a1c23ba45
9 changed files with 97 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
lib/Database.php
View file

@ -385,7 +385,7 @@ class Database {
// SQL will happily accept duplicates (null is not unique), so we must do this check ourselves
$p = $this->db->prepare(
"WITH RECURSIVE
target as (select ? as userid, cast(? as bigint) as source, cast(? as bigint) as dest, ? as rename),
target as (select ? as userid, ? as source, ? as dest, ? as rename),
folders as (SELECT id from arsse_folders join target on owner = userid and coalesce(parent,0) = source union select arsse_folders.id as id from arsse_folders join folders on arsse_folders.parent=folders.id)
".
"SELECT
@ -480,7 +480,7 @@ class Database {
join arsse_feeds on feed = arsse_feeds.id
left join topmost on folder=f_id"
);
$q->setOrder("pinned desc, title collate nocase");
$q->setOrder("pinned desc, coalesce(arsse_subscriptions.title, arsse_feeds.title) collate nocase");
// define common table expressions
$q->setCTE("userdata(userid)", "SELECT ?", "str", $user); // the subject user; this way we only have to pass it to prepare() once
// topmost folders belonging to the user

4
lib/Db/PDODriver.php
View file

@ -28,9 +28,9 @@ trait PDODriver {
}
$changes = $r->rowCount();
try {
$lastId = 0;
$lastId = $this->db->lastInsertId();
$lastId = ($changes) ? $this->db->lastInsertId() : 0;
} catch (\PDOException $e) { // @codeCoverageIgnore
$lastId = 0;
}
return new PDOResult($r, [$changes, $lastId]);
}

5
lib/Db/PDOError.php
View file

@ -7,14 +7,15 @@ declare(strict_types=1);
namespace JKingWeb\Arsse\Db;
trait PDOError {
public function exceptionBuild(): array {
if ($this instanceof Statement) {
public function exceptionBuild(bool $statementError = null): array {
if ($statementError ?? ($this instanceof Statement)) {
$err = $this->st->errorInfo();
} else {
$err = $this->db->errorInfo();
}
switch ($err[0]) {
case "22P02":
case "42804":
return [ExceptionInput::class, 'engineTypeViolation', $err[2]];
case "23000":
case "23502":

8
lib/Db/PDOStatement.php
View file

@ -28,10 +28,10 @@ class PDOStatement extends AbstractStatement {
}
public function __destruct() {
unset($this->st);
unset($this->st, $this->db);
}
public function runArray(array $values = []): \JKingWeb\Arsse\Db\Result {
public function runArray(array $values = []): Result {
$this->st->closeCursor();
$this->bindValues($values);
try {
@ -42,9 +42,9 @@ class PDOStatement extends AbstractStatement {
}
$changes = $this->st->rowCount();
try {
$lastId = 0;
$lastId = $this->db->lastInsertId();
$lastId = ($changes) ? $this->db->lastInsertId() : 0;
} catch (\PDOException $e) { // @codeCoverageIgnore
$lastId = 0;
}
return new PDOResult($this->st, [$changes, $lastId]);
}

4
lib/Db/PostgreSQL/PDODriver.php
View file

@ -44,4 +44,8 @@ class PDODriver extends Driver {
public static function driverName(): string {
return Arsse::$lang->msg("Driver.Db.PostgreSQLPDO.Name");
}
public function prepareArray(string $query, array $paramTypes): \JKingWeb\Arsse\Db\Statement {
return new PDOStatement($this->db, $query, $paramTypes);
}
}

77
lib/Db/PostgreSQL/PDOStatement.php Normal file
View file

@ -0,0 +1,77 @@
<?php
/** @license MIT
* Copyright 2017 J. King, Dustin Wilson et al.
* See LICENSE and AUTHORS files for details */
declare(strict_types=1);
namespace JKingWeb\Arsse\Db\PostgreSQL;
class PDOStatement extends \JKingWeb\Arsse\Db\AbstractStatement {
use \JKingWeb\Arsse\Db\PDOError;
const BINDINGS = [
"integer" => "bigint",
"float" => "decimal",
"datetime" => "timestamp",
"binary" => "bytea",
"string" => "text",
"boolean" => "smallint", // FIXME: using boolean leads to incompatibilities with versions of SQLite bundled prior to PHP 7.3
];
protected $db;
protected $st;
protected $qOriginal;
protected $qMunged;
protected $bindings;
public function __construct(\PDO $db, string $query, array $bindings = []) {
$this->db = $db; // both db and st are the same object due to the logic of the PDOError handler
$this->qOriginal = $query;
$this->retypeArray($bindings);
}
public function __destruct() {
unset($this->db, $this->st);
}
public function retypeArray(array $bindings, bool $append = false): bool {
if ($append) {
return parent::retypeArray($bindings, $append);
} else {
$this->bindings = $bindings;
parent::retypeArray($bindings, $append);
$this->qMunged = self::mungeQuery($this->qOriginal, $this->types, false);
try {
$s = $this->db->prepare($this->qMunged);
$this->st = new \JKingWeb\Arsse\Db\PDOStatement($this->db, $s, $this->bindings);
} catch (\PDOException $e) {
list($excClass, $excMsg, $excData) = $this->exceptionBuild(true);
throw new $excClass($excMsg, $excData);
}
}
return true;
}
public static function mungeQuery(string $q, array $types, bool $mungeParamMarkers = true): string {
$q = explode("?", $q);
$out = "";
for ($b = 1; $b < sizeof($q); $b++) {
$a = $b - 1;
$mark = $mungeParamMarkers ? "\$$b" : "?";
$type = isset($types[$a]) ? "::".self::BINDINGS[$types[$a]] : "";
$out .= $q[$a].$mark.$type;
}
$out .= array_pop($q);
return $out;
}
public function runArray(array $values = []): \JKingWeb\Arsse\Db\Result {
return $this->st->runArray($values);
}
/** @codeCoverageIgnore */
protected function bindValue($value, string $type, int $position): bool {
// stub required by abstract parent, but never used
return $value;
}
}

4
tests/cases/Db/BaseStatement.php
View file

@ -59,7 +59,7 @@ abstract class BaseStatement extends \JKingWeb\Arsse\Test\AbstractTest {
/** @dataProvider provideBindings */
public function testBindATypedValue($value, string $type, string $exp) {
if ($exp=="null") {
$query = "SELECT (cast(? as text) is null) as pass";
$query = "SELECT (? is null) as pass";
} else {
$query = "SELECT ($exp = ?) as pass";
}
@ -76,7 +76,7 @@ abstract class BaseStatement extends \JKingWeb\Arsse\Test\AbstractTest {
$this->markTestSkipped("Correct handling of binary data with PostgreSQL is currently unknown");
}
if ($exp=="null") {
$query = "SELECT (cast(? as text) is null) as pass";
$query = "SELECT (? is null) as pass";
} else {
$query = "SELECT ($exp = ?) as pass";
}

2
tests/cases/Db/PostgreSQL/TestStatement.php
View file

@ -13,7 +13,7 @@ class TestStatement extends \JKingWeb\Arsse\TestCase\Db\BaseStatement {
protected static $implementation = "PDO PostgreSQL";
protected function makeStatement(string $q, array $types = []): array {
return [static::$interface, static::$interface->prepare($q), $types];
return [static::$interface, $q, $types];
}
protected function decorateTypeSyntax(string $value, string $type): string {

4
tests/lib/DatabaseInformation.php
View file

@ -161,10 +161,10 @@ class DatabaseInformation {
'PDO PostgreSQL' => [
'pdo' => true,
'backend' => "PostgreSQL",
'statementClass' => \JKingWeb\Arsse\Db\PDOStatement::class,
'statementClass' => \JKingWeb\Arsse\Db\PostgreSQL\PDOStatement::class,
'resultClass' => \JKingWeb\Arsse\Db\PDOResult::class,
'driverClass' => \JKingWeb\Arsse\Db\PostgreSQL\PDODriver::class,
'stringOutput' => true,
'stringOutput' => false,
'interfaceConstructor' => function() {
$connString = \JKingWeb\Arsse\Db\PostgreSQL\Driver::makeConnectionString(true, Arsse::$conf->dbPostgreSQLUser, Arsse::$conf->dbPostgreSQLPass, Arsse::$conf->dbPostgreSQLDb, Arsse::$conf->dbPostgreSQLHost, Arsse::$conf->dbPostgreSQLPort, "");
try {