From c21ae3eca990269d41f1d5e6117e7a32dcc32cf7 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Mon, 2 Nov 2020 15:21:04 -0500 Subject: [PATCH 1/3] Correctly send binary data to PostgreSQL This finally brings PostgreSQL to parity with SQLite and MySQL. Two tests casting binary data to text were removed since behaviour here should in fact be undefined Accountinf for any encoding when retrieving data will be addressed by a later commit --- lib/Db/PostgreSQL/Statement.php | 3 +++ tests/cases/Db/BaseStatement.php | 7 ------- tests/cases/Db/PostgreSQL/TestStatement.php | 5 +++++ tests/cases/Db/PostgreSQLPDO/TestStatement.php | 5 +++++ 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/lib/Db/PostgreSQL/Statement.php b/lib/Db/PostgreSQL/Statement.php index 8c89053..4472e8e 100644 --- a/lib/Db/PostgreSQL/Statement.php +++ b/lib/Db/PostgreSQL/Statement.php @@ -44,6 +44,9 @@ class Statement extends \JKingWeb\Arsse\Db\AbstractStatement { } protected function bindValue($value, int $type, int $position): bool { + if ($value !== null && ($this->types[$position - 1] % self::T_NOT_NULL) === self::T_BINARY) { + $value = "\\x".bin2hex($value); + } $this->in[] = $value; return true; } diff --git a/tests/cases/Db/BaseStatement.php b/tests/cases/Db/BaseStatement.php index 206aed7..ba86269 100644 --- a/tests/cases/Db/BaseStatement.php +++ b/tests/cases/Db/BaseStatement.php @@ -57,7 +57,6 @@ abstract class BaseStatement extends \JKingWeb\Arsse\Test\AbstractTest { } else { $query = "SELECT ($exp = ?) as pass"; } - $typeStr = "'".str_replace("'", "''", $type)."'"; $s = new $this->statementClass(...$this->makeStatement($query)); $s->retype(...[$type]); $act = $s->run(...[$value])->getValue(); @@ -66,15 +65,11 @@ abstract class BaseStatement extends \JKingWeb\Arsse\Test\AbstractTest { /** @dataProvider provideBinaryBindings */ public function testHandleBinaryData($value, string $type, string $exp): void { - if (in_array(static::$implementation, ["PostgreSQL", "PDO PostgreSQL"])) { - $this->markTestIncomplete("Correct handling of binary data with PostgreSQL is not currently implemented"); - } if ($exp === "null") { $query = "SELECT (? is null) as pass"; } else { $query = "SELECT ($exp = ?) as pass"; } - $typeStr = "'".str_replace("'", "''", $type)."'"; $s = new $this->statementClass(...$this->makeStatement($query)); $s->retype(...[$type]); $act = $s->run(...[$value])->getValue(); @@ -297,13 +292,11 @@ abstract class BaseStatement extends \JKingWeb\Arsse\Test\AbstractTest { 'UTF-8 string as strict binary' => ["\u{e9}", "strict binary", "x'c3a9'"], 'Binary string as integer' => [chr(233).chr(233), "integer", "0"], 'Binary string as float' => [chr(233).chr(233), "float", "0.0"], - 'Binary string as string' => [chr(233).chr(233), "string", "'".chr(233).chr(233)."'"], 'Binary string as binary' => [chr(233).chr(233), "binary", "x'e9e9'"], 'Binary string as datetime' => [chr(233).chr(233), "datetime", "null"], 'Binary string as boolean' => [chr(233).chr(233), "boolean", "1"], 'Binary string as strict integer' => [chr(233).chr(233), "strict integer", "0"], 'Binary string as strict float' => [chr(233).chr(233), "strict float", "0.0"], - 'Binary string as strict string' => [chr(233).chr(233), "strict string", "'".chr(233).chr(233)."'"], 'Binary string as strict binary' => [chr(233).chr(233), "strict binary", "x'e9e9'"], 'Binary string as strict datetime' => [chr(233).chr(233), "strict datetime", "'0001-01-01 00:00:00'"], 'Binary string as strict boolean' => [chr(233).chr(233), "strict boolean", "1"], diff --git a/tests/cases/Db/PostgreSQL/TestStatement.php b/tests/cases/Db/PostgreSQL/TestStatement.php index 7b44ec1..a7f776a 100644 --- a/tests/cases/Db/PostgreSQL/TestStatement.php +++ b/tests/cases/Db/PostgreSQL/TestStatement.php @@ -27,6 +27,11 @@ class TestStatement extends \JKingWeb\Arsse\TestCase\Db\BaseStatement { return "U&'\\+".str_pad(dechex((int) $match[1]), 6, "0", \STR_PAD_LEFT)."'"; } return $value; + case "binary": + if ($value[0] === "x") { + return "'\\x".substr($value, 2)."::bytea"; + } + // no break; default: return $value; } diff --git a/tests/cases/Db/PostgreSQLPDO/TestStatement.php b/tests/cases/Db/PostgreSQLPDO/TestStatement.php index 926df76..8878d42 100644 --- a/tests/cases/Db/PostgreSQLPDO/TestStatement.php +++ b/tests/cases/Db/PostgreSQLPDO/TestStatement.php @@ -27,6 +27,11 @@ class TestStatement extends \JKingWeb\Arsse\TestCase\Db\BaseStatement { return "U&'\\+".str_pad(dechex((int) $match[1]), 6, "0", \STR_PAD_LEFT)."'"; } return $value; + case "binary": + if ($value[0] === "x") { + return "'\\x".substr($value, 2)."::bytea"; + } + // no break; default: return $value; } From 41bcffd6fb530c1f104658be12d0cdd6603f2e8f Mon Sep 17 00:00:00 2001 From: "J. King" Date: Tue, 3 Nov 2020 17:52:20 -0500 Subject: [PATCH 2/3] Correctly query PostgreSQL byte arrays This required different workarouynd for the native and PDO interfaces --- lib/Db/PostgreSQL/PDOResult.php | 26 +++++++++++++++++++++ lib/Db/PostgreSQL/PDOStatement.php | 14 +++++++++++ lib/Db/PostgreSQL/Result.php | 14 ++++++++++- tests/cases/Db/BaseResult.php | 13 +++++++++++ tests/cases/Db/PostgreSQL/TestResult.php | 13 +++++++++++ tests/cases/Db/PostgreSQLPDO/TestResult.php | 15 +++++++++++- tests/lib/DatabaseDrivers/PostgreSQLPDO.php | 2 +- 7 files changed, 94 insertions(+), 3 deletions(-) create mode 100644 lib/Db/PostgreSQL/PDOResult.php diff --git a/lib/Db/PostgreSQL/PDOResult.php b/lib/Db/PostgreSQL/PDOResult.php new file mode 100644 index 0000000..91fe4c0 --- /dev/null +++ b/lib/Db/PostgreSQL/PDOResult.php @@ -0,0 +1,26 @@ +cur = $this->set->fetch(\PDO::FETCH_ASSOC); + if ($this->cur !== false) { + foreach($this->cur as $k => $v) { + if (is_resource($v)) { + $this->cur[$k] = stream_get_contents($v); + fclose($v); + } + } + return true; + } + return false; + } +} diff --git a/lib/Db/PostgreSQL/PDOStatement.php b/lib/Db/PostgreSQL/PDOStatement.php index c9b7b82..9929579 100644 --- a/lib/Db/PostgreSQL/PDOStatement.php +++ b/lib/Db/PostgreSQL/PDOStatement.php @@ -6,6 +6,8 @@ declare(strict_types=1); namespace JKingWeb\Arsse\Db\PostgreSQL; +use JKingWeb\Arsse\Db\Result; + class PDOStatement extends \JKingWeb\Arsse\Db\PDOStatement { public static function mungeQuery(string $query, array $types, ...$extraData): string { return Statement::mungeQuery($query, $types, false); @@ -16,4 +18,16 @@ class PDOStatement extends \JKingWeb\Arsse\Db\PDOStatement { // PostgreSQL uses SQLSTATE exclusively, so this is not used return []; } + + public function runArray(array $values = []): Result { + $this->st->closeCursor(); + $this->bindValues($values); + try { + $this->st->execute(); + } catch (\PDOException $e) { + [$excClass, $excMsg, $excData] = $this->buildPDOException(true); + throw new $excClass($excMsg, $excData); + } + return new PDOResult($this->db, $this->st); + } } diff --git a/lib/Db/PostgreSQL/Result.php b/lib/Db/PostgreSQL/Result.php index 03dba17..67a7352 100644 --- a/lib/Db/PostgreSQL/Result.php +++ b/lib/Db/PostgreSQL/Result.php @@ -10,6 +10,7 @@ class Result extends \JKingWeb\Arsse\Db\AbstractResult { protected $db; protected $r; protected $cur; + protected $blobs = []; // actual public methods @@ -30,6 +31,11 @@ class Result extends \JKingWeb\Arsse\Db\AbstractResult { public function __construct($db, $result) { $this->db = $db; $this->r = $result; + for ($a = 0, $stop = pg_num_fields($result); $a < $stop; $a++) { + if (pg_field_type($result, $a) === "bytea") { + $this->blobs[$a] = pg_field_name($result, $a); + } + } } public function __destruct() { @@ -41,6 +47,12 @@ class Result extends \JKingWeb\Arsse\Db\AbstractResult { public function valid() { $this->cur = pg_fetch_row($this->r, null, \PGSQL_ASSOC); - return $this->cur !== false; + if ($this->cur !== false) { + foreach($this->blobs as $f) { + $this->cur[$f] = hex2bin(substr($this->cur[$f], 2)); + } + return true; + } + return false; } } diff --git a/tests/cases/Db/BaseResult.php b/tests/cases/Db/BaseResult.php index 4d3d2c4..7d63af2 100644 --- a/tests/cases/Db/BaseResult.php +++ b/tests/cases/Db/BaseResult.php @@ -10,6 +10,7 @@ use JKingWeb\Arsse\Db\Result; abstract class BaseResult extends \JKingWeb\Arsse\Test\AbstractTest { protected static $insertDefault = "INSERT INTO arsse_test default values"; + protected static $selectBlob = "SELECT x'DEADBEEF' as \"blob\""; protected static $interface; protected $resultClass; @@ -129,4 +130,16 @@ abstract class BaseResult extends \JKingWeb\Arsse\Test\AbstractTest { $test = new $this->resultClass(...$this->makeResult("SELECT '2112' as album, '2112' as track union select 'Clockwork Angels' as album, 'The Wreckers' as track")); $this->assertEquals($exp, $test->getAll()); } + + public function testGetBlobRow(): void { + $exp = ['blob' => hex2bin("DEADBEEF")]; + $test = new $this->resultClass(...$this->makeResult(self::$selectBlob)); + $this->assertEquals($exp, $test->getRow()); + } + + public function testGetBlobValue(): void { + $exp = hex2bin("DEADBEEF"); + $test = new $this->resultClass(...$this->makeResult(self::$selectBlob)); + $this->assertEquals($exp, $test->getValue()); + } } diff --git a/tests/cases/Db/PostgreSQL/TestResult.php b/tests/cases/Db/PostgreSQL/TestResult.php index 0992962..73dd8fb 100644 --- a/tests/cases/Db/PostgreSQL/TestResult.php +++ b/tests/cases/Db/PostgreSQL/TestResult.php @@ -15,6 +15,7 @@ class TestResult extends \JKingWeb\Arsse\TestCase\Db\BaseResult { protected static $createMeta = "CREATE TABLE arsse_meta(key text primary key not null, value text)"; protected static $createTest = "CREATE TABLE arsse_test(id bigserial primary key)"; + protected static $selectBlob = "SELECT '\\xDEADBEEF'::bytea as blob"; protected function makeResult(string $q): array { $set = pg_query(static::$interface, $q); @@ -29,4 +30,16 @@ class TestResult extends \JKingWeb\Arsse\TestCase\Db\BaseResult { } parent::tearDownAfterClass(); } + + public function testGetBlobRow(): void { + $exp = ['blob' => hex2bin("DEADBEEF")]; + $test = new $this->resultClass(...$this->makeResult(self::$selectBlob)); + $this->assertEquals($exp, $test->getRow()); + } + + public function testGetBlobValue(): void { + $exp = hex2bin("DEADBEEF"); + $test = new $this->resultClass(...$this->makeResult(self::$selectBlob)); + $this->assertEquals($exp, $test->getValue()); + } } diff --git a/tests/cases/Db/PostgreSQLPDO/TestResult.php b/tests/cases/Db/PostgreSQLPDO/TestResult.php index aaf6bca..c810b71 100644 --- a/tests/cases/Db/PostgreSQLPDO/TestResult.php +++ b/tests/cases/Db/PostgreSQLPDO/TestResult.php @@ -8,16 +8,29 @@ namespace JKingWeb\Arsse\TestCase\Db\PostgreSQLPDO; /** * @group slow - * @covers \JKingWeb\Arsse\Db\PDOResult + * @covers \JKingWeb\Arsse\Db\PostgreSQL\PDOResult */ class TestResult extends \JKingWeb\Arsse\TestCase\Db\BaseResult { use \JKingWeb\Arsse\Test\DatabaseDrivers\PostgreSQLPDO; protected static $createMeta = "CREATE TABLE arsse_meta(key text primary key not null, value text)"; protected static $createTest = "CREATE TABLE arsse_test(id bigserial primary key)"; + protected static $selectBlob = "SELECT '\\xDEADBEEF'::bytea as blob"; protected function makeResult(string $q): array { $set = static::$interface->query($q); return [static::$interface, $set]; } + + public function testGetBlobRow(): void { + $exp = ['blob' => hex2bin("DEADBEEF")]; + $test = new $this->resultClass(...$this->makeResult(self::$selectBlob)); + $this->assertEquals($exp, $test->getRow()); + } + + public function testGetBlobValue(): void { + $exp = hex2bin("DEADBEEF"); + $test = new $this->resultClass(...$this->makeResult(self::$selectBlob)); + $this->assertSame($exp, $test->getValue()); + } } diff --git a/tests/lib/DatabaseDrivers/PostgreSQLPDO.php b/tests/lib/DatabaseDrivers/PostgreSQLPDO.php index 58001b6..116c3b2 100644 --- a/tests/lib/DatabaseDrivers/PostgreSQLPDO.php +++ b/tests/lib/DatabaseDrivers/PostgreSQLPDO.php @@ -11,7 +11,7 @@ use JKingWeb\Arsse\Arsse; trait PostgreSQLPDO { protected static $implementation = "PDO PostgreSQL"; protected static $backend = "PostgreSQL"; - protected static $dbResultClass = \JKingWeb\Arsse\Db\PDOResult::class; + protected static $dbResultClass = \JKingWeb\Arsse\Db\PostgreSQL\PDOResult::class; protected static $dbStatementClass = \JKingWeb\Arsse\Db\PostgreSQL\PDOStatement::class; protected static $dbDriverClass = \JKingWeb\Arsse\Db\PostgreSQL\PDODriver::class; protected static $stringOutput = false; From b5f959aabfc426cfbadf40d8fafedccc42ae9ef7 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Tue, 3 Nov 2020 18:57:26 -0500 Subject: [PATCH 3/3] Fix blob tests --- tests/cases/Db/BaseResult.php | 4 ++-- tests/cases/Db/PostgreSQL/TestResult.php | 12 ------------ tests/cases/Db/PostgreSQLPDO/TestResult.php | 12 ------------ 3 files changed, 2 insertions(+), 26 deletions(-) diff --git a/tests/cases/Db/BaseResult.php b/tests/cases/Db/BaseResult.php index 7d63af2..a43956d 100644 --- a/tests/cases/Db/BaseResult.php +++ b/tests/cases/Db/BaseResult.php @@ -133,13 +133,13 @@ abstract class BaseResult extends \JKingWeb\Arsse\Test\AbstractTest { public function testGetBlobRow(): void { $exp = ['blob' => hex2bin("DEADBEEF")]; - $test = new $this->resultClass(...$this->makeResult(self::$selectBlob)); + $test = new $this->resultClass(...$this->makeResult(static::$selectBlob)); $this->assertEquals($exp, $test->getRow()); } public function testGetBlobValue(): void { $exp = hex2bin("DEADBEEF"); - $test = new $this->resultClass(...$this->makeResult(self::$selectBlob)); + $test = new $this->resultClass(...$this->makeResult(static::$selectBlob)); $this->assertEquals($exp, $test->getValue()); } } diff --git a/tests/cases/Db/PostgreSQL/TestResult.php b/tests/cases/Db/PostgreSQL/TestResult.php index 73dd8fb..658228e 100644 --- a/tests/cases/Db/PostgreSQL/TestResult.php +++ b/tests/cases/Db/PostgreSQL/TestResult.php @@ -30,16 +30,4 @@ class TestResult extends \JKingWeb\Arsse\TestCase\Db\BaseResult { } parent::tearDownAfterClass(); } - - public function testGetBlobRow(): void { - $exp = ['blob' => hex2bin("DEADBEEF")]; - $test = new $this->resultClass(...$this->makeResult(self::$selectBlob)); - $this->assertEquals($exp, $test->getRow()); - } - - public function testGetBlobValue(): void { - $exp = hex2bin("DEADBEEF"); - $test = new $this->resultClass(...$this->makeResult(self::$selectBlob)); - $this->assertEquals($exp, $test->getValue()); - } } diff --git a/tests/cases/Db/PostgreSQLPDO/TestResult.php b/tests/cases/Db/PostgreSQLPDO/TestResult.php index c810b71..caddba7 100644 --- a/tests/cases/Db/PostgreSQLPDO/TestResult.php +++ b/tests/cases/Db/PostgreSQLPDO/TestResult.php @@ -21,16 +21,4 @@ class TestResult extends \JKingWeb\Arsse\TestCase\Db\BaseResult { $set = static::$interface->query($q); return [static::$interface, $set]; } - - public function testGetBlobRow(): void { - $exp = ['blob' => hex2bin("DEADBEEF")]; - $test = new $this->resultClass(...$this->makeResult(self::$selectBlob)); - $this->assertEquals($exp, $test->getRow()); - } - - public function testGetBlobValue(): void { - $exp = hex2bin("DEADBEEF"); - $test = new $this->resultClass(...$this->makeResult(self::$selectBlob)); - $this->assertSame($exp, $test->getValue()); - } }