1
1
Fork 0
mirror of https://code.mensbeam.com/MensBeam/Arsse.git synced 2024-12-22 21:22:40 +00:00

Merge fixes from master

This commit is contained in:
J. King 2017-09-26 20:25:11 -04:00
commit 7b121c74ec
9 changed files with 410 additions and 287 deletions

View file

@ -3,8 +3,7 @@ declare(strict_types=1);
namespace JKingWeb\Arsse;
abstract class AbstractException extends \Exception {
const CODES = [
"Exception.uncoded" => -1,
const CODES = [ "Exception.uncoded" => -1,
"Exception.unknown" => 10000,
"Lang/Exception.defaultFileMissing" => 10101,
"Lang/Exception.fileMissing" => 10102,
@ -40,7 +39,9 @@ abstract class AbstractException extends \Exception {
"Db/ExceptionInput.tooShort" => 10234,
"Db/ExceptionInput.idMissing" => 10235,
"Db/ExceptionInput.constraintViolation" => 10236,
"Db/ExceptionInput.engineConstraintViolation" => 10236,
"Db/ExceptionInput.typeViolation" => 10237,
"Db/ExceptionInput.engineTypeViolation" => 10237,
"Db/ExceptionInput.circularDependence" => 10238,
"Db/ExceptionInput.subjectMissing" => 10239,
"Db/ExceptionTimeout.general" => 10241,

View file

@ -7,6 +7,7 @@ use JKingWeb\DrUUID\UUID;
use JKingWeb\Arsse\Misc\Query;
use JKingWeb\Arsse\Misc\Context;
use JKingWeb\Arsse\Misc\Date;
use JKingWeb\Arsse\Misc\ValueInfo;
class Database {
const SCHEMA_VERSION = 2;
@ -281,31 +282,13 @@ class Database {
if (!Arsse::$user->authorize($user, __FUNCTION__)) {
throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]);
}
// if the desired folder name is missing or invalid, throw an exception
if (!array_key_exists("name", $data) || $data['name']=="") {
throw new Db\ExceptionInput("missing", ["action" => __FUNCTION__, "field" => "name"]);
} elseif (!strlen(trim($data['name']))) {
throw new Db\ExceptionInput("whitespace", ["action" => __FUNCTION__, "field" => "name"]);
}
// normalize folder's parent, if there is one
$parent = array_key_exists("parent", $data) ? (int) $data['parent'] : 0;
if ($parent===0) {
// if no parent is specified, do nothing
$parent = null;
} else {
// if a parent is specified, make sure it exists and belongs to the user; get its root (first-level) folder if it's a nested folder
$p = $this->db->prepare("SELECT id from arsse_folders where owner is ? and id is ?", "str", "int")->run($user, $parent)->getValue();
if (!$p) {
throw new Db\ExceptionInput("idMissing", ["action" => __FUNCTION__, "field" => "parent", 'id' => $parent]);
}
}
// check if a folder by the same name already exists, because nulls are wonky in SQL
// FIXME: How should folder name be compared? Should a Unicode normalization be applied before comparison and insertion?
if ($this->db->prepare("SELECT count(*) from arsse_folders where owner is ? and parent is ? and name is ?", "str", "int", "str")->run($user, $parent, $data['name'])->getValue() > 0) {
throw new Db\ExceptionInput("constraintViolation"); // FIXME: There needs to be a practical message here
}
// actually perform the insert (!)
return $this->db->prepare("INSERT INTO arsse_folders(owner,parent,name) values(?,?,?)", "str", "int", "str")->run($user, $parent, $data['name'])->lastId();
$parent = array_key_exists("parent", $data) ? $this->folderValidateId($user, $data['parent'])['id'] : null;
// validate the folder name and parent (if specified); this also checks for duplicates
$name = array_key_exists("name", $data) ? $data['name'] : "";
$this->folderValidateName($name, true, $parent);
// actually perform the insert
return $this->db->prepare("INSERT INTO arsse_folders(owner,parent,name) values(?,?,?)", "str", "int", "str")->run($user, $parent, $name)->lastId();
}
public function folderList(string $user, int $parent = null, bool $recursive = true): Db\Result {
@ -356,70 +339,110 @@ class Database {
if (!Arsse::$user->authorize($user, __FUNCTION__)) {
throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]);
}
// validate the folder ID and, if specified, the parent to move it to
$parent = null;
if (array_key_exists("parent", $data)) {
$parent = $data['parent'];
}
$f = $this->folderValidateId($user, $id, $parent, true);
// if a new name is specified, validate it
if (array_key_exists("name", $data)) {
// verify the folder belongs to the user
$in = $this->folderValidateId($user, $id, true);
$name = array_key_exists("name", $data);
$parent = array_key_exists("parent", $data);
if ($name && $parent) {
// if a new name and parent are specified, validate both together
$this->folderValidateName($data['name']);
}
$data = array_merge($f, $data);
// check to make sure the target folder name/location would not create a duplicate (we must do this check because null is not distinct in SQL)
$existing = $this->db->prepare("SELECT id from arsse_folders where owner is ? and parent is ? and name is ?", "str", "int", "str")->run($user, $data['parent'], $data['name'])->getValue();
if (!is_null($existing) && $existing != $id) {
throw new Db\ExceptionInput("constraintViolation"); // FIXME: There needs to be a practical message here
$in['name'] = $data['name'];
$in['parent'] = $this->folderValidateMove($user, $id, $data['parent'], $data['name']);
} elseif ($name) {
// if a new name is specified, validate it
$this->folderValidateName($data['name'], true, $in['parent']);
$in['name'] = $data['name'];
} elseif ($parent) {
// if a new parent is specified, validate it
$in['parent'] = $this->folderValidateMove($user, $id, $data['parent']);
} else {
// if neither was specified, do nothing
return false;
}
$valid = [
'name' => "str",
'parent' => "int",
];
list($setClause, $setTypes, $setValues) = $this->generateSet($data, $valid);
return (bool) $this->db->prepare("UPDATE arsse_folders set $setClause where owner is ? and id is ?", $setTypes, "str", "int")->run($setValues, $user, $id)->changes();
list($setClause, $setTypes, $setValues) = $this->generateSet($in, $valid);
return (bool) $this->db->prepare("UPDATE arsse_folders set $setClause, modified = CURRENT_TIMESTAMP where owner is ? and id is ?", $setTypes, "str", "int")->run($setValues, $user, $id)->changes();
}
protected function folderValidateId(string $user, int $id = null, int $parent = null, bool $subject = false): array {
if (is_null($id)) {
// if no ID is specified this is a no-op, unless a parent is specified, which is always a circular dependence (the root cannot be moved)
if (!is_null($parent)) {
throw new Db\ExceptionInput("circularDependence", ["action" => $this->caller(), "field" => "parent", 'id' => $parent]); // @codeCoverageIgnore
protected function folderValidateId(string $user, $id = null, bool $subject = false): array {
$idInfo = ValueInfo::int($id);
if ($idInfo & (ValueInfo::NULL | ValueInfo::ZERO)) {
// if a null or zero ID is specified this is a no-op
return ['id' => null, 'name' => null, 'parent' => null];
}
return ['name' => null, 'parent' => null];
// if a negative integer or non-integer is specified this will always fail
if (!($idInfo & ValueInfo::VALID) || (($idInfo & ValueInfo::NEG))) {
throw new Db\ExceptionInput($subject ? "subjectMissing" : "idMissing", ["action" => $this->caller(), "field" => "folder", 'id' => $id]);
}
// check whether the folder exists and is owned by the user
$f = $this->db->prepare("SELECT name,parent from arsse_folders where owner is ? and id is ?", "str", "int")->run($user, $id)->getRow();
$f = $this->db->prepare("SELECT id,name,parent from arsse_folders where owner is ? and id is ?", "str", "int")->run($user, $id)->getRow();
if (!$f) {
throw new Db\ExceptionInput($subject ? "subjectMissing" : "idMissing", ["action" => $this->caller(), "field" => "folder", 'id' => $parent]);
}
// if we're moving a folder to a new parent, check that the parent is valid
if (!is_null($parent)) {
// make sure both that the parent exists, and that the parent is not either the folder itself or one of its children (a circular dependence)
$p = $this->db->prepare(
"WITH RECURSIVE folders(id) as (SELECT id from arsse_folders where owner is ? and id is ? union select arsse_folders.id from arsse_folders join folders on arsse_folders.parent=folders.id) ".
"SELECT id,(id not in (select id from folders)) as valid from arsse_folders where owner is ? and id is ?",
"str", "int", "str", "int"
)->run($user, $id, $user, $parent)->getRow();
if (!$p) {
// if the parent doesn't exist or doesn't below to the user, throw an exception
throw new Db\ExceptionInput("idMissing", ["action" => $this->caller(), "field" => "parent", 'id' => $parent]);
} else {
// if using the desired parent would create a circular dependence, throw a different exception
if (!$p['valid']) {
throw new Db\ExceptionInput("circularDependence", ["action" => $this->caller(), "field" => "parent", 'id' => $parent]);
}
}
throw new Db\ExceptionInput($subject ? "subjectMissing" : "idMissing", ["action" => $this->caller(), "field" => "folder", 'id' => $id]);
}
return $f;
}
protected function folderValidateName($name): bool {
$name = (string) $name;
if (!strlen($name)) {
protected function folderValidateMove(string $user, int $id = null, $parent = null, string $name = null) {
$errData = ["action" => $this->caller(), "field" => "parent", 'id' => $parent];
if (!$id) {
// the root cannot be moved
throw new Db\ExceptionInput("circularDependence", $errData);
}
$info = ValueInfo::int($parent);
// the root is always a valid parent
if ($info & (ValueInfo::NULL | ValueInfo::ZERO)) {
$parent = null;
} else {
// if a negative integer or non-integer is specified this will always fail
if (!($info & ValueInfo::VALID) || (($info & ValueInfo::NEG))) {
throw new Db\ExceptionInput("idMissing", $errData);
}
$parent = (int) $parent;
}
// if the target parent is the folder itself, this is a circular dependence
if ($id==$parent) {
throw new Db\ExceptionInput("circularDependence", $errData);
}
// make sure both that the prospective parent exists, and that the it is not one of its children (a circular dependence)
$p = $this->db->prepare(
"WITH RECURSIVE
target as (select ? as user, ? as source, ? as dest, ? as rename),
folders as (SELECT id from arsse_folders join target on owner is user and parent is source union select arsse_folders.id as id from arsse_folders join folders on arsse_folders.parent=folders.id)
".
"SELECT
((select dest from target) is null or exists(select id from arsse_folders join target on owner is user and id is dest)) as extant,
not exists(select id from folders where id is (select dest from target)) as valid,
not exists(select id from arsse_folders join target on parent is dest and name is coalesce((select rename from target),(select name from arsse_folders join target on id is source))) as available
", "str", "int", "int","str"
)->run($user, $id, $parent, $name)->getRow();
if (!$p['extant']) {
// if the parent doesn't exist or doesn't below to the user, throw an exception
throw new Db\ExceptionInput("idMissing", $errData);
} elseif (!$p['valid']) {
// if using the desired parent would create a circular dependence, throw a different exception
throw new Db\ExceptionInput("circularDependence", $errData);
} elseif (!$p['available']) {
throw new Db\ExceptionInput("constraintViolation", ["action" => $this->caller(), "field" => (is_null($name) ? "parent" : "name")]);
}
return $parent;
}
protected function folderValidateName($name, bool $checkDuplicates = false, int $parent = null): bool {
$info = ValueInfo::str($name);
if ($info & (ValueInfo::NULL | ValueInfo::EMPTY)) {
throw new Db\ExceptionInput("missing", ["action" => $this->caller(), "field" => "name"]);
} elseif (!strlen(trim($name))) {
} elseif ($info & ValueInfo::WHITE) {
throw new Db\ExceptionInput("whitespace", ["action" => $this->caller(), "field" => "name"]);
} elseif (!($info & ValueInfo::VALID)) {
throw new Db\ExceptionInput("typeViolation", ["action" => $this->caller(), "field" => "name", 'type' => "string"]);
} elseif($checkDuplicates) {
if ($this->db->prepare("SELECT exists(select id from arsse_folders where parent is ? and name is ?)", "int", "str")->run($parent, $name)->getValue()) {
throw new Db\ExceptionInput("constraintViolation", ["action" => $this->caller(), "field" => "name"]);
}
return true;
} else {
return true;
}
@ -473,7 +496,7 @@ class Database {
// this condition facilitates the implementation of subscriptionPropertiesGet, which would otherwise have to duplicate the complex query; it takes precedence over a specified folder
// if an ID is specified, add a suitable WHERE condition and bindings
$q->setWhere("arsse_subscriptions.id is ?", "int", $id);
} elseif (!is_null($folder)) {
} elseif ($folder) {
// if a folder is specified, make sure it exists
$this->folderValidateId($user, $folder);
// if it does exist, add a common table expression to list it and its children so that we select from the entire subtree
@ -520,18 +543,19 @@ class Database {
}
if (array_key_exists("folder", $data)) {
// ensure the target folder exists and belong to the user
$this->folderValidateId($user, $data['folder']);
$data['folder'] = $this->folderValidateId($user, $data['folder'])['id'];
}
if (array_key_exists("title", $data)) {
// if the title is null, this signals intended use of the default title; otherwise make sure it's not effectively an empty string
if (!is_null($data['title'])) {
$title = (string) $data['title'];
if (!strlen($title)) {
$info = ValueInfo::str($data['title']);
if ($info & ValueInfo::EMPTY) {
throw new Db\ExceptionInput("missing", ["action" => __FUNCTION__, "field" => "title"]);
} elseif (!strlen(trim($title))) {
} elseif ($info & ValueInfo::WHITE) {
throw new Db\ExceptionInput("whitespace", ["action" => __FUNCTION__, "field" => "title"]);
} elseif (!($info & ValueInfo::VALID)) {
throw new Db\ExceptionInput("typeViolation", ["action" => __FUNCTION__, "field" => "title", 'type' => "string"]);
}
$data['title'] = $title;
}
}
$valid = [

View file

@ -12,9 +12,9 @@ trait ExceptionBuilder {
case self::SQLITE_BUSY:
return [ExceptionTimeout::class, 'general', $this->db->lastErrorMsg()];
case self::SQLITE_CONSTRAINT:
return [ExceptionInput::class, 'constraintViolation', $this->db->lastErrorMsg()];
return [ExceptionInput::class, 'engineConstraintViolation', $this->db->lastErrorMsg()];
case self::SQLITE_MISMATCH:
return [ExceptionInput::class, 'typeViolation', $this->db->lastErrorMsg()];
return [ExceptionInput::class, 'engineTypeViolation', $this->db->lastErrorMsg()];
default:
return [Exception::class, 'engineErrorGeneral', $this->db->lastErrorMsg()];
}

View file

@ -3,6 +3,7 @@ declare(strict_types=1);
namespace JKingWeb\Arsse\Misc;
use JKingWeb\Arsse\Misc\Date;
use JKingWeb\Arsse\Misc\ValueInfo;
class Context {
public $reverse = false;
@ -36,22 +37,11 @@ class Context {
protected function cleanArray(array $spec): array {
$spec = array_values($spec);
for ($a = 0; $a < sizeof($spec); $a++) {
$id = $spec[$a];
if (is_int($id) && $id > -1) {
continue;
} elseif (is_float($id) && !fmod($id, 1) && $id >= 0) {
$spec[$a] = (int) $id;
continue;
} elseif (is_string($id)) {
$ch1 = strval(@intval($id));
$ch2 = strval($id);
if ($ch1 !== $ch2 || $id < 1) {
$id = 0;
}
if(ValueInfo::int($spec[$a])===ValueInfo::VALID) {
$spec[$a] = (int) $spec[$a];
} else {
$id = 0;
$spec[$a] = 0;
}
$spec[$a] = (int) $id;
}
return array_values(array_filter($spec));
}

73
lib/Misc/ValueInfo.php Normal file
View file

@ -0,0 +1,73 @@
<?php
declare(strict_types=1);
namespace JKingWeb\Arsse\Misc;
class ValueInfo {
// universal
const VALID = 1 << 0;
const NULL = 1 << 1;
// integers
const ZERO = 1 << 2;
const NEG = 1 << 3;
// strings
const EMPTY = 1 << 2;
const WHITE = 1 << 3;
static public function int($value): int {
$out = 0;
// check if the input is null
if (is_null($value)) {
$out += self::NULL;
}
// normalize the value to an integer or float if possible
if (is_string($value)) {
if (strval(@intval($value))===$value) {
$value = (int) $value;
} elseif (strval(@floatval($value))===$value) {
$value = (float) $value;
}
// the empty string is equivalent to null when evaluating an integer
if (!strlen((string) $value)) {
$out += self::NULL;
}
}
// if the value is not an integer or integral float, stop
if (!is_int($value) && (!is_float($value) || fmod($value, 1))) {
return $out;
}
// mark validity
$value = (int) $value;
$out += self::VALID;
// mark zeroness
if(!$value) {
$out += self::ZERO;
}
// mark negativeness
if ($value < 0) {
$out += self::NEG;
}
return $out;
}
static public function str($value): int {
$out = 0;
// check if the input is null
if (is_null($value)) {
$out += self::NULL;
}
// if the value is not scalar, it cannot be valid
if (!is_scalar($value)) {
return $out;
}
// mark validity
$out += self::VALID;
if (!strlen((string) $value)) {
// mark emptiness
$out += self::EMPTY;
} elseif (!strlen(trim((string) $value))) {
// mark whitespacedness
$out += self::WHITE;
}
return $out;
}
}

View file

@ -3,6 +3,7 @@ declare(strict_types=1);
namespace JKingWeb\Arsse\REST;
use JKingWeb\Arsse\Misc\Date;
use JKingWeb\Arsse\Misc\ValueInfo;
abstract class AbstractHandler implements Handler {
abstract public function __construct();
@ -32,9 +33,7 @@ abstract class AbstractHandler implements Handler {
}
protected function validateInt($id): bool {
$ch1 = strval(@intval($id));
$ch2 = strval($id);
return ($ch1 === $ch2);
return (bool) (ValueInfo::int($id) & ValueInfo::VALID);
}
protected function NormalizeInput(array $data, array $types, string $dateFormat = null): array {

View file

@ -117,11 +117,13 @@ return [
'Exception.JKingWeb/Arsse/Db/ExceptionInput.whitespace' => 'Field "{field}" of action "{action}" may not contain only whitespace',
'Exception.JKingWeb/Arsse/Db/ExceptionInput.tooLong' => 'Field "{field}" of action "{action}" has a maximum length of {max}',
'Exception.JKingWeb/Arsse/Db/ExceptionInput.tooShort' => 'Field "{field}" of action "{action}" has a minimum length of {min}',
'Exception.JKingWeb/Arsse/Db/ExceptionInput.typeViolation' => 'Field "{field}" of action "{action}" expects a value of type "{type}"',
'Exception.JKingWeb/Arsse/Db/ExceptionInput.subjectMissing' => 'Referenced ID ({id}) in field "{field}" does not exist',
'Exception.JKingWeb/Arsse/Db/ExceptionInput.idMissing' => 'Referenced ID ({id}) in field "{field}" does not exist',
'Exception.JKingWeb/Arsse/Db/ExceptionInput.circularDependence' => 'Referenced ID ({id}) in field "{field}" creates a circular dependence',
'Exception.JKingWeb/Arsse/Db/ExceptionInput.constraintViolation' => '{0}',
'Exception.JKingWeb/Arsse/Db/ExceptionInput.typeViolation' => '{0}',
'Exception.JKingWeb/Arsse/Db/ExceptionInput.constraintViolation' => 'Specified value in field "{0}" already exists',
'Exception.JKingWeb/Arsse/Db/ExceptionInput.engineConstraintViolation' => '{0}',
'Exception.JKingWeb/Arsse/Db/ExceptionInput.engineTypeViolation' => '{0}',
'Exception.JKingWeb/Arsse/Db/ExceptionTimeout.general' => '{0}',
'Exception.JKingWeb/Arsse/Db/ExceptionSavepoint.invalid' => 'Tried to {action} invalid savepoint {index}',
'Exception.JKingWeb/Arsse/Db/ExceptionSavepoint.stale' => 'Tried to {action} stale savepoint {index}',

View file

@ -76,6 +76,11 @@ trait SeriesFolder {
Arsse::$db->folderAdd("john.doe@example.com", ['name' => "Sociology", 'parent' => 2112]);
}
public function testAddANestedFolderToAnInvalidParent() {
$this->assertException("idMissing", "Db", "ExceptionInput");
Arsse::$db->folderAdd("john.doe@example.com", ['name' => "Sociology", 'parent' => "stringFolderId"]);
}
public function testAddANestedFolderForTheWrongOwner() {
$this->assertException("idMissing", "Db", "ExceptionInput");
Arsse::$db->folderAdd("john.doe@example.com", ['name' => "Sociology", 'parent' => 4]); // folder ID 4 belongs to Jane
@ -216,6 +221,10 @@ trait SeriesFolder {
Arsse::$db->folderPropertiesGet("john.doe@example.com", 1);
}
public function testMakeNoChangesToAFolder() {
$this->assertFalse(Arsse::$db->folderPropertiesSet("john.doe@example.com", 6, []));
}
public function testRenameAFolder() {
$this->assertTrue(Arsse::$db->folderPropertiesSet("john.doe@example.com", 6, ['name' => "Opinion"]));
Phake::verify(Arsse::$user)->authorize("john.doe@example.com", "folderPropertiesSet");
@ -234,6 +243,11 @@ trait SeriesFolder {
$this->assertTrue(Arsse::$db->folderPropertiesSet("john.doe@example.com", 6, ['name' => " "]));
}
public function testRenameAFolderToAnInvalidValue() {
$this->assertException("typeViolation", "Db", "ExceptionInput");
$this->assertTrue(Arsse::$db->folderPropertiesSet("john.doe@example.com", 6, ['name' => []]));
}
public function testMoveAFolder() {
$this->assertTrue(Arsse::$db->folderPropertiesSet("john.doe@example.com", 6, ['parent' => 5]));
Phake::verify(Arsse::$user)->authorize("john.doe@example.com", "folderPropertiesSet");
@ -242,6 +256,11 @@ trait SeriesFolder {
$this->compareExpectations($state);
}
public function testMoveTheRootFolder() {
$this->assertException("circularDependence", "Db", "ExceptionInput");
Arsse::$db->folderPropertiesSet("john.doe@example.com", 0, ['parent' => 1]);
}
public function testMoveAFolderToItsDescendant() {
$this->assertException("circularDependence", "Db", "ExceptionInput");
Arsse::$db->folderPropertiesSet("john.doe@example.com", 1, ['parent' => 3]);
@ -257,11 +276,21 @@ trait SeriesFolder {
Arsse::$db->folderPropertiesSet("john.doe@example.com", 1, ['parent' => 2112]);
}
public function testMoveAFolderToAnInvalidParent() {
$this->assertException("idMissing", "Db", "ExceptionInput");
Arsse::$db->folderPropertiesSet("john.doe@example.com", 1, ['parent' => "ThisFolderDoesNotExist"]);
}
public function testCauseAFolderCollision() {
$this->assertException("constraintViolation", "Db", "ExceptionInput");
Arsse::$db->folderPropertiesSet("john.doe@example.com", 6, ['parent' => null]);
}
public function testCauseACompoundFolderCollision() {
$this->assertException("constraintViolation", "Db", "ExceptionInput");
Arsse::$db->folderPropertiesSet("john.doe@example.com", 3, ['parent' => null, 'name' => "Technology"]);
}
public function testSetThePropertiesOfAMissingFolder() {
$this->assertException("subjectMissing", "Db", "ExceptionInput");
Arsse::$db->folderPropertiesSet("john.doe@example.com", 2112, ['parent' => null]);

View file

@ -319,6 +319,11 @@ trait SeriesSubscription {
$this->assertTrue(Arsse::$db->subscriptionPropertiesSet($this->user, 1, ['title' => 0]));
}
public function testRenameASubscriptionToAnArray() {
$this->assertException("typeViolation", "Db", "ExceptionInput");
Arsse::$db->subscriptionPropertiesSet($this->user, 1, ['title' => []]);
}
public function testSetThePropertiesOfAMissingSubscription() {
$this->assertException("subjectMissing", "Db", "ExceptionInput");
Arsse::$db->subscriptionPropertiesSet($this->user, 2112, ['folder' => null]);