diff --git a/mod/lockview.php b/mod/lockview.php index af57730c8..9f9dcfea4 100644 --- a/mod/lockview.php +++ b/mod/lockview.php @@ -64,10 +64,10 @@ function lockview_content(App $a) /** @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'] ?? ''); + $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 e298cc160..9501c8e5d 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -2904,10 +2904,10 @@ class Item extends BaseObject /** @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); + $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 0d3b4eaac..c44e4c61a 100644 --- a/src/Module/Item/Compose.php +++ b/src/Module/Item/Compose.php @@ -74,8 +74,8 @@ class Compose extends BaseModule $compose_title = L10n::t('Compose new post'); $type = 'post'; $doesFederate = true; - $contact_allow = implode(',', $aclFormatter->expand($user['allow_cid'] ?? '')); - $group_allow = implode(',', $aclFormatter->expand($user['allow_gid'] ?? '')) ?: Group::FOLLOWERS; + $contact_allow = implode(',', $aclFormatter->expand($user['allow_cid'])); + $group_allow = implode(',', $aclFormatter->expand($user['allow_gid'])) ?: Group::FOLLOWERS; break; } @@ -86,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(',', $aclFormatter->expand($user['deny_cid'] ?? '')); - $group_deny = $_REQUEST['group_deny'] ?? implode(',', $aclFormatter->expand($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 index 4a36f3ebf..a7d851508 100644 --- a/src/Util/ACLFormatter.php +++ b/src/Util/ACLFormatter.php @@ -12,12 +12,17 @@ final class ACLFormatter /** * Turn user/group ACLs stored as angle bracketed text into arrays * - * @param string $ids A angle-bracketed list of IDs + * @param string|null $ids A angle-bracketed list of IDs * - * @return array The array based on the IDs + * @return array The array based on the IDs (empty in case there is no list) */ - public function expand(string $ids) + public function expand(string $ids = null) { + // In case there is no ID list, return empty array (=> no ACL set) + if (!isset($ids)) { + return []; + } + // 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]+)>/', $ids, $matches, PREG_PATTERN_ORDER); @@ -31,12 +36,18 @@ final class ACLFormatter * @param string $item The item to sanitise */ private function sanitize(string &$item) { + // The item is an ACL int value if (intval($item)) { $item = '<' . intval(Strings::escapeTags(trim($item))) . '>'; + // The item is a allowed ACL character } elseif (in_array($item, [Group::FOLLOWERS, Group::MUTUALS])) { $item = '<' . $item . '>'; - } else { + // The item is already a ACL string + } elseif (preg_match('/<\d+?>/', $item)) { unset($item); + // The item is not supported, so remove it (cleanup) + } else { + $item = ''; } } diff --git a/src/Worker/Notifier.php b/src/Worker/Notifier.php index 9ab07e6d4..8b6be76b5 100644 --- a/src/Worker/Notifier.php +++ b/src/Worker/Notifier.php @@ -279,10 +279,10 @@ class Notifier /** @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'] ?? '')); + $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/src/Util/ACLFormaterTest.php b/tests/src/Util/ACLFormaterTest.php index ac5b25e6c..4dccda600 100644 --- a/tests/src/Util/ACLFormaterTest.php +++ b/tests/src/Util/ACLFormaterTest.php @@ -12,167 +12,111 @@ use PHPUnit\Framework\TestCase; */ class ACLFormaterTest extends TestCase { - /** - * test expand_acl, perfect input - */ - public function testExpandAclNormal() + public function assertAcl($text, array $assert = []) { $aclFormatter = new ACLFormatter(); - $text='<1><2><3><' . Group::FOLLOWERS . '><' . Group::MUTUALS . '>'; - $this->assertEquals(array('1', '2', '3', Group::FOLLOWERS, Group::MUTUALS), $aclFormatter->expand($text)); + $acl = $aclFormatter->expand($text); + + $this->assertEquals($assert, $acl); + + $this->assertMergable($acl); } - /** - * test with a big number - */ - public function testExpandAclBigNumber() + public function assertMergable(array $aclOne, array $aclTwo = []) { - $aclFormatter = new ACLFormatter(); + $this->assertTrue(is_array($aclOne)); + $this->assertTrue(is_array($aclTwo)); - $text='<1><' . PHP_INT_MAX . '><15>'; - $this->assertEquals(array('1', (string)PHP_INT_MAX, '15'), $aclFormatter->expand($text)); + $aclMerged = array_unique(array_merge($aclOne, $aclTwo)); + $this->assertTrue(is_array($aclMerged)); + + return $aclMerged; } - /** - * test with a string in it. - * - * @todo is this valid input? Otherwise: should there be an exception? - */ - public function testExpandAclString() + public function dataExpand() { - $aclFormatter = new ACLFormatter(); - - $text="<1><279012>"; - $this->assertEquals(array('1', '279012'), $aclFormatter->expand($text)); + return [ + 'normal' => [ + 'input' => '<1><2><3><' . Group::FOLLOWERS . '><' . Group::MUTUALS . '>', + 'assert' => ['1', '2', '3', Group::FOLLOWERS, Group::MUTUALS], + ], + 'nigNumber' => [ + 'input' => '<1><' . PHP_INT_MAX . '><15>', + 'assert' => ['1', (string)PHP_INT_MAX, '15'], + ], + 'string' => [ + 'input' => '<1><279012>', + 'assert' => ['1', '279012'], + ], + 'space' => [ + 'input' => '<1><279 012><32>', + 'assert' => ['1', '32'], + ], + 'empty' => [ + 'input' => '', + 'assert' => [], + ], + /// @todo should there be an exception? + 'noBrackets' => [ + 'input' => 'According to documentation, that\'s invalid. ', //should be invalid + 'assert' => [], + ], + /// @todo should there be an exception? + 'justOneBracket' => [ + 'input' => ' [], + ], + /// @todo should there be an exception? + 'justOneBracket2' => [ + 'input' => 'Another invalid> string', //should be invalid + 'assert' => [], + ], + /// @todo should there be an exception? + 'closeOnly' => [ + 'input' => 'Another> invalid> string>', //should be invalid + 'assert' => [], + ], + /// @todo should there be an exception? + 'openOnly' => [ + 'input' => ' [], + ], + /// @todo should there be an exception? + 'noMatching1' => [ + 'input' => ' invalid ', //should be invalid + 'assert' => [], + ], + 'emptyMatch' => [ + 'input' => '<1><><3>', + 'assert' => ['1', '3'], + ], + ]; } /** - * test with a ' ' in it. - * - * @todo is this valid input? Otherwise: should there be an exception? + * @dataProvider dataExpand */ - public function testExpandAclSpace() + public function testExpand($input, array $assert) { - $aclFormatter = new ACLFormatter(); - - $text="<1><279 012><32>"; - $this->assertEquals(array('1', '32'), $aclFormatter->expand($text)); + $this->assertAcl($input, $assert); } /** - * 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)); - } - - /** - * Test expected exception in case of wrong typehint - * - * @expectedException Error + * Test nullable expand (=> no ACL set) */ public function testExpandNull() { $aclFormatter = new ACLFormatter(); - $aclFormatter->expand(null); + $allow_people = $aclFormatter->expand(); + $allow_groups = $aclFormatter->expand(); + + $this->assertEmpty($aclFormatter->expand(null)); + $this->assertEmpty($aclFormatter->expand()); + + $recipients = array_unique(array_merge($allow_people, $allow_groups)); + $this->assertEmpty($recipients); } public function dataAclToString() @@ -198,6 +142,23 @@ class ACLFormaterTest extends TestCase 'input' => 'a,bsd23,4', 'assert' => '<4>', ], + /** @see https://github.com/friendica/friendica/pull/7787 */ + 'bug-7778-angle-brackets' => [ + 'input' => ["<40195>"], + 'assert' => "<40195>", + ], + Group::FOLLOWERS => [ + 'input' => [Group::FOLLOWERS, 1], + 'assert' => '<' . Group::FOLLOWERS . '><1>', + ], + Group::MUTUALS => [ + 'input' => [Group::MUTUALS, 1], + 'assert' => '<' . Group::MUTUALS . '><1>', + ], + 'wrong-angle-brackets' => [ + 'input' => ["","<123>"], + 'assert' => "<123>", + ], ]; }