From f65f7f11c3e7158e90b7660e08b7b94f5795d41e Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Wed, 23 Oct 2019 00:40:14 +0200 Subject: [PATCH] Move expand_acl to ACLFormatter::expand() - including tests --- include/text.php | 17 --- mod/lockview.php | 13 ++- src/Model/Item.php | 12 ++- src/Module/Item/Compose.php | 12 ++- src/Util/ACLFormatter.php | 27 +++++ src/Worker/Notifier.php | 12 ++- tests/include/TextTest.php | 127 ---------------------- tests/src/Util/ACLFormaterTest.php | 164 +++++++++++++++++++++++++++++ 8 files changed, 224 insertions(+), 160 deletions(-) create mode 100644 src/Util/ACLFormatter.php create mode 100644 tests/src/Util/ACLFormaterTest.php diff --git a/include/text.php b/include/text.php index 216e87ed8c..289802136c 100644 --- a/include/text.php +++ b/include/text.php @@ -3,28 +3,11 @@ * @file include/text.php */ -use Friendica\App; use Friendica\Content\Text\BBCode; -use Friendica\Core\Protocol; use Friendica\Model\FileTag; use Friendica\Model\Group; use Friendica\Util\Strings; -/** - * Turn user/group ACLs stored as angle bracketed text into arrays - * - * @param string $s - * @return array - */ -function expand_acl($s) { - // turn string array of angle-bracketed elements into numeric array - // e.g. "<1><2><3>" => array(1,2,3); - preg_match_all('/<(' . Group::FOLLOWERS . '|'. Group::MUTUALS . '|[0-9]+)>/', $s, $matches, PREG_PATTERN_ORDER); - - return $matches[1]; -} - - /** * Wrap ACL elements in angle brackets for storage * @param string $item diff --git a/mod/lockview.php b/mod/lockview.php index eede1b6a0d..9f9dcfea42 100644 --- a/mod/lockview.php +++ b/mod/lockview.php @@ -3,11 +3,13 @@ * @file mod/lockview.php */ use Friendica\App; +use Friendica\BaseObject; use Friendica\Core\Hook; use Friendica\Core\L10n; use Friendica\Database\DBA; use Friendica\Model\Group; use Friendica\Model\Item; +use Friendica\Util\ACLFormatter; function lockview_content(App $a) { @@ -59,10 +61,13 @@ function lockview_content(App $a) exit(); } - $allowed_users = expand_acl($item['allow_cid']); - $allowed_groups = expand_acl($item['allow_gid']); - $deny_users = expand_acl($item['deny_cid']); - $deny_groups = expand_acl($item['deny_gid']); + /** @var ACLFormatter $aclFormatter */ + $aclFormatter = BaseObject::getClass(ACLFormatter::class); + + $allowed_users = $aclFormatter->expand($item['allow_cid']); + $allowed_groups = $aclFormatter->expand($item['allow_gid']); + $deny_users = $aclFormatter->expand($item['deny_cid']); + $deny_groups = $aclFormatter->expand($item['deny_gid']); $o = L10n::t('Visible to:') . '
'; $l = []; diff --git a/src/Model/Item.php b/src/Model/Item.php index 650390e529..456d770260 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -24,6 +24,7 @@ use Friendica\Database\DBA; use Friendica\Protocol\ActivityPub; use Friendica\Protocol\Diaspora; use Friendica\Protocol\OStatus; +use Friendica\Util\ACLFormatter; use Friendica\Util\DateTimeFormat; use Friendica\Util\Map; use Friendica\Util\Network; @@ -2892,10 +2893,13 @@ class Item extends BaseObject */ public static function enumeratePermissions(array $obj, bool $check_dead = false) { - $allow_people = expand_acl($obj['allow_cid']); - $allow_groups = Group::expand($obj['uid'], expand_acl($obj['allow_gid']), $check_dead); - $deny_people = expand_acl($obj['deny_cid']); - $deny_groups = Group::expand($obj['uid'], expand_acl($obj['deny_gid']), $check_dead); + /** @var ACLFormatter $aclFormater */ + $aclFormater = self::getClass(ACLFormatter::class); + + $allow_people = $aclFormater->expand($obj['allow_cid']); + $allow_groups = Group::expand($obj['uid'], $aclFormater->expand($obj['allow_gid']), $check_dead); + $deny_people = $aclFormater->expand($obj['deny_cid']); + $deny_groups = Group::expand($obj['uid'], $aclFormater->expand($obj['deny_gid']), $check_dead); $recipients = array_unique(array_merge($allow_people, $allow_groups)); $deny = array_unique(array_merge($deny_people, $deny_groups)); $recipients = array_diff($recipients, $deny); diff --git a/src/Module/Item/Compose.php b/src/Module/Item/Compose.php index 11b886a2ed..c44e4c61ab 100644 --- a/src/Module/Item/Compose.php +++ b/src/Module/Item/Compose.php @@ -16,6 +16,7 @@ use Friendica\Model\Item; use Friendica\Model\User; use Friendica\Module\Login; use Friendica\Network\HTTPException\NotImplementedException; +use Friendica\Util\ACLFormatter; use Friendica\Util\Crypto; class Compose extends BaseModule @@ -58,6 +59,9 @@ class Compose extends BaseModule $user = User::getById(local_user(), ['allow_cid', 'allow_gid', 'deny_cid', 'deny_gid', 'hidewall', 'default-location']); + /** @var ACLFormatter $aclFormatter */ + $aclFormatter = self::getClass(ACLFormatter::class); + switch ($posttype) { case Item::PT_PERSONAL_NOTE: $compose_title = L10n::t('Compose new personal note'); @@ -70,8 +74,8 @@ class Compose extends BaseModule $compose_title = L10n::t('Compose new post'); $type = 'post'; $doesFederate = true; - $contact_allow = implode(',', expand_acl($user['allow_cid'])); - $group_allow = implode(',', expand_acl($user['allow_gid'])) ?: Group::FOLLOWERS; + $contact_allow = implode(',', $aclFormatter->expand($user['allow_cid'])); + $group_allow = implode(',', $aclFormatter->expand($user['allow_gid'])) ?: Group::FOLLOWERS; break; } @@ -82,8 +86,8 @@ class Compose extends BaseModule $wall = $_REQUEST['wall'] ?? $type == 'post'; $contact_allow = $_REQUEST['contact_allow'] ?? $contact_allow; $group_allow = $_REQUEST['group_allow'] ?? $group_allow; - $contact_deny = $_REQUEST['contact_deny'] ?? implode(',', expand_acl($user['deny_cid'])); - $group_deny = $_REQUEST['group_deny'] ?? implode(',', expand_acl($user['deny_gid'])); + $contact_deny = $_REQUEST['contact_deny'] ?? implode(',', $aclFormatter->expand($user['deny_cid'])); + $group_deny = $_REQUEST['group_deny'] ?? implode(',', $aclFormatter->expand($user['deny_gid'])); $visibility = ($contact_allow . $user['allow_gid'] . $user['deny_cid'] . $user['deny_gid']) ? 'custom' : 'public'; $acl_contacts = Contact::selectToArray(['id', 'name', 'addr', 'micro'], ['uid' => local_user(), 'pending' => false, 'rel' => [Contact::FOLLOWER, Contact::FRIEND]]); diff --git a/src/Util/ACLFormatter.php b/src/Util/ACLFormatter.php new file mode 100644 index 0000000000..4e3d32b157 --- /dev/null +++ b/src/Util/ACLFormatter.php @@ -0,0 +1,27 @@ +<2><3>" => array(1,2,3); + preg_match_all('/<(' . Group::FOLLOWERS . '|'. Group::MUTUALS . '|[0-9]+)>/', $ids, $matches, PREG_PATTERN_ORDER); + + return $matches[1]; + } +} diff --git a/src/Worker/Notifier.php b/src/Worker/Notifier.php index 4bf97aca5f..ebc70ffb50 100644 --- a/src/Worker/Notifier.php +++ b/src/Worker/Notifier.php @@ -24,6 +24,7 @@ use Friendica\Protocol\ActivityPub; use Friendica\Protocol\Diaspora; use Friendica\Protocol\OStatus; use Friendica\Protocol\Salmon; +use Friendica\Util\ACLFormatter; require_once 'include/items.php'; @@ -272,10 +273,13 @@ class Notifier $public_message = false; // private recipients, not public } - $allow_people = expand_acl($parent['allow_cid']); - $allow_groups = Group::expand($uid, expand_acl($parent['allow_gid']),true); - $deny_people = expand_acl($parent['deny_cid']); - $deny_groups = Group::expand($uid, expand_acl($parent['deny_gid'])); + /** @var ACLFormatter $aclFormatter */ + $aclFormatter = BaseObject::getClass(ACLFormatter::class); + + $allow_people = $aclFormatter->expand($parent['allow_cid']); + $allow_groups = Group::expand($uid, $aclFormatter->expand($parent['allow_gid']),true); + $deny_people = $aclFormatter->expand($parent['deny_cid']); + $deny_groups = Group::expand($uid, $aclFormatter->expand($parent['deny_gid'])); // if our parent is a public forum (forum_mode == 1), uplink to the origional author causing // a delivery fork. private groups (forum_mode == 2) do not uplink diff --git a/tests/include/TextTest.php b/tests/include/TextTest.php index 1137c0415d..e41b71b874 100644 --- a/tests/include/TextTest.php +++ b/tests/include/TextTest.php @@ -13,133 +13,6 @@ use PHPUnit\Framework\TestCase; */ class TextTest extends TestCase { - /** - * test expand_acl, perfect input - */ - public function testExpandAclNormal() - { - $text='<1><2><3><' . Group::FOLLOWERS . '><' . Group::MUTUALS . '>'; - $this->assertEquals(array('1', '2', '3', Group::FOLLOWERS, Group::MUTUALS), expand_acl($text)); - } - - /** - * test with a big number - */ - public function testExpandAclBigNumber() - { - $text='<1><' . PHP_INT_MAX . '><15>'; - $this->assertEquals(array('1', (string)PHP_INT_MAX, '15'), expand_acl($text)); - } - - /** - * test with a string in it. - * - * @todo is this valid input? Otherwise: should there be an exception? - */ - public function testExpandAclString() - { - $text="<1><279012>"; - $this->assertEquals(array('1', '279012'), expand_acl($text)); - } - - /** - * test with a ' ' in it. - * - * @todo is this valid input? Otherwise: should there be an exception? - */ - public function testExpandAclSpace() - { - $text="<1><279 012><32>"; - $this->assertEquals(array('1', '32'), expand_acl($text)); - } - - /** - * test empty input - */ - public function testExpandAclEmpty() - { - $text=""; - $this->assertEquals(array(), expand_acl($text)); - } - - /** - * test invalid input, no < at all - * - * @todo should there be an exception? - */ - public function testExpandAclNoBrackets() - { - $text="According to documentation, that's invalid. "; //should be invalid - $this->assertEquals(array(), expand_acl($text)); - } - - /** - * test invalid input, just open < - * - * @todo should there be an exception? - */ - public function testExpandAclJustOneBracket1() - { - $text="assertEquals(array(), expand_acl($text)); - } - - /** - * test invalid input, just close > - * - * @todo should there be an exception? - */ - public function testExpandAclJustOneBracket2() - { - $text="Another invalid> string"; //should be invalid - $this->assertEquals(array(), expand_acl($text)); - } - - /** - * test invalid input, just close > - * - * @todo should there be an exception? - */ - public function testExpandAclCloseOnly() - { - $text="Another> invalid> string>"; //should be invalid - $this->assertEquals(array(), expand_acl($text)); - } - - /** - * test invalid input, just open < - * - * @todo should there be an exception? - */ - public function testExpandAclOpenOnly() - { - $text="assertEquals(array(), expand_acl($text)); - } - - /** - * test invalid input, open and close do not match - * - * @todo should there be an exception? - */ - public function testExpandAclNoMatching1() - { - $text=" invalid "; //should be invalid - $this->assertEquals(array(), expand_acl($text)); - } - - /** - * test invalid input, empty <> - * - * @todo should there be an exception? Or array(1, 3) - * (This should be array(1,3) - mike) - */ - public function testExpandAclEmptyMatch() - { - $text="<1><><3>"; - $this->assertEquals(array('1', '3'), expand_acl($text)); - } - /** * test hex2bin and reverse */ diff --git a/tests/src/Util/ACLFormaterTest.php b/tests/src/Util/ACLFormaterTest.php new file mode 100644 index 0000000000..c3cfb70514 --- /dev/null +++ b/tests/src/Util/ACLFormaterTest.php @@ -0,0 +1,164 @@ +<2><3><' . Group::FOLLOWERS . '><' . Group::MUTUALS . '>'; + $this->assertEquals(array('1', '2', '3', Group::FOLLOWERS, Group::MUTUALS), $aclFormatter->expand($text)); + } + + /** + * test with a big number + */ + public function testExpandAclBigNumber() + { + $aclFormatter = new ACLFormatter(); + + $text='<1><' . PHP_INT_MAX . '><15>'; + $this->assertEquals(array('1', (string)PHP_INT_MAX, '15'), $aclFormatter->expand($text)); + } + + /** + * test with a string in it. + * + * @todo is this valid input? Otherwise: should there be an exception? + */ + public function testExpandAclString() + { + $aclFormatter = new ACLFormatter(); + + $text="<1><279012>"; + $this->assertEquals(array('1', '279012'), $aclFormatter->expand($text)); + } + + /** + * test with a ' ' in it. + * + * @todo is this valid input? Otherwise: should there be an exception? + */ + public function testExpandAclSpace() + { + $aclFormatter = new ACLFormatter(); + + $text="<1><279 012><32>"; + $this->assertEquals(array('1', '32'), $aclFormatter->expand($text)); + } + + /** + * test empty input + */ + public function testExpandAclEmpty() + { + $aclFormatter = new ACLFormatter(); + + $text=""; + $this->assertEquals(array(), $aclFormatter->expand($text)); + } + + /** + * test invalid input, no < at all + * + * @todo should there be an exception? + */ + public function testExpandAclNoBrackets() + { + $aclFormatter = new ACLFormatter(); + + $text="According to documentation, that's invalid. "; //should be invalid + $this->assertEquals(array(), $aclFormatter->expand($text)); + } + + /** + * test invalid input, just open < + * + * @todo should there be an exception? + */ + public function testExpandAclJustOneBracket1() + { + $aclFormatter = new ACLFormatter(); + + $text="assertEquals(array(), $aclFormatter->expand($text)); + } + + /** + * test invalid input, just close > + * + * @todo should there be an exception? + */ + public function testExpandAclJustOneBracket2() + { + $aclFormatter = new ACLFormatter(); + + $text="Another invalid> string"; //should be invalid + $this->assertEquals(array(), $aclFormatter->expand($text)); + } + + /** + * test invalid input, just close > + * + * @todo should there be an exception? + */ + public function testExpandAclCloseOnly() + { + $aclFormatter = new ACLFormatter(); + + $text="Another> invalid> string>"; //should be invalid + $this->assertEquals(array(), $aclFormatter->expand($text)); + } + + /** + * test invalid input, just open < + * + * @todo should there be an exception? + */ + public function testExpandAclOpenOnly() + { + $aclFormatter = new ACLFormatter(); + + $text="assertEquals(array(), $aclFormatter->expand($text)); + } + + /** + * test invalid input, open and close do not match + * + * @todo should there be an exception? + */ + public function testExpandAclNoMatching1() + { + $aclFormatter = new ACLFormatter(); + + $text=" invalid "; //should be invalid + $this->assertEquals(array(), $aclFormatter->expand($text)); + } + + /** + * test invalid input, empty <> + * + * @todo should there be an exception? Or array(1, 3) + * (This should be array(1,3) - mike) + */ + public function testExpandAclEmptyMatch() + { + $aclFormatter = new ACLFormatter(); + + $text="<1><><3>"; + $this->assertEquals(array('1', '3'), $aclFormatter->expand($text)); + } +}