Merge pull request #7793 from nupplaphil/bug/fix_travis_test

Fix/FollowUp ACLFormatter
This commit is contained in:
Hypolite Petovan 2019-11-01 11:05:15 -04:00 committed by GitHub
commit 291bc621b2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 130 additions and 158 deletions

View file

@ -64,10 +64,10 @@ function lockview_content(App $a)
/** @var ACLFormatter $aclFormatter */ /** @var ACLFormatter $aclFormatter */
$aclFormatter = BaseObject::getClass(ACLFormatter::class); $aclFormatter = BaseObject::getClass(ACLFormatter::class);
$allowed_users = $aclFormatter->expand($item['allow_cid'] ?? ''); $allowed_users = $aclFormatter->expand($item['allow_cid']);
$allowed_groups = $aclFormatter->expand($item['allow_gid'] ?? ''); $allowed_groups = $aclFormatter->expand($item['allow_gid']);
$deny_users = $aclFormatter->expand($item['deny_cid'] ?? ''); $deny_users = $aclFormatter->expand($item['deny_cid']);
$deny_groups = $aclFormatter->expand($item['deny_gid'] ?? ''); $deny_groups = $aclFormatter->expand($item['deny_gid']);
$o = L10n::t('Visible to:') . '<br />'; $o = L10n::t('Visible to:') . '<br />';
$l = []; $l = [];

View file

@ -2904,10 +2904,10 @@ class Item extends BaseObject
/** @var ACLFormatter $aclFormater */ /** @var ACLFormatter $aclFormater */
$aclFormater = self::getClass(ACLFormatter::class); $aclFormater = self::getClass(ACLFormatter::class);
$allow_people = $aclFormater->expand($obj['allow_cid'] ?? ''); $allow_people = $aclFormater->expand($obj['allow_cid']);
$allow_groups = Group::expand($obj['uid'], $aclFormater->expand($obj['allow_gid'] ?? ''), $check_dead); $allow_groups = Group::expand($obj['uid'], $aclFormater->expand($obj['allow_gid']), $check_dead);
$deny_people = $aclFormater->expand($obj['deny_cid'] ?? ''); $deny_people = $aclFormater->expand($obj['deny_cid']);
$deny_groups = Group::expand($obj['uid'], $aclFormater->expand($obj['deny_gid'] ?? ''), $check_dead); $deny_groups = Group::expand($obj['uid'], $aclFormater->expand($obj['deny_gid']), $check_dead);
$recipients = array_unique(array_merge($allow_people, $allow_groups)); $recipients = array_unique(array_merge($allow_people, $allow_groups));
$deny = array_unique(array_merge($deny_people, $deny_groups)); $deny = array_unique(array_merge($deny_people, $deny_groups));
$recipients = array_diff($recipients, $deny); $recipients = array_diff($recipients, $deny);

View file

@ -74,8 +74,8 @@ class Compose extends BaseModule
$compose_title = L10n::t('Compose new post'); $compose_title = L10n::t('Compose new post');
$type = 'post'; $type = 'post';
$doesFederate = true; $doesFederate = true;
$contact_allow = implode(',', $aclFormatter->expand($user['allow_cid'] ?? '')); $contact_allow = implode(',', $aclFormatter->expand($user['allow_cid']));
$group_allow = implode(',', $aclFormatter->expand($user['allow_gid'] ?? '')) ?: Group::FOLLOWERS; $group_allow = implode(',', $aclFormatter->expand($user['allow_gid'])) ?: Group::FOLLOWERS;
break; break;
} }
@ -86,8 +86,8 @@ class Compose extends BaseModule
$wall = $_REQUEST['wall'] ?? $type == 'post'; $wall = $_REQUEST['wall'] ?? $type == 'post';
$contact_allow = $_REQUEST['contact_allow'] ?? $contact_allow; $contact_allow = $_REQUEST['contact_allow'] ?? $contact_allow;
$group_allow = $_REQUEST['group_allow'] ?? $group_allow; $group_allow = $_REQUEST['group_allow'] ?? $group_allow;
$contact_deny = $_REQUEST['contact_deny'] ?? implode(',', $aclFormatter->expand($user['deny_cid'] ?? '')); $contact_deny = $_REQUEST['contact_deny'] ?? implode(',', $aclFormatter->expand($user['deny_cid']));
$group_deny = $_REQUEST['group_deny'] ?? implode(',', $aclFormatter->expand($user['deny_gid'] ?? '')); $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'; $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]]); $acl_contacts = Contact::selectToArray(['id', 'name', 'addr', 'micro'], ['uid' => local_user(), 'pending' => false, 'rel' => [Contact::FOLLOWER, Contact::FRIEND]]);

View file

@ -12,12 +12,17 @@ final class ACLFormatter
/** /**
* Turn user/group ACLs stored as angle bracketed text into arrays * 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 // turn string array of angle-bracketed elements into numeric array
// e.g. "<1><2><3>" => array(1,2,3); // e.g. "<1><2><3>" => array(1,2,3);
preg_match_all('/<(' . Group::FOLLOWERS . '|'. Group::MUTUALS . '|[0-9]+)>/', $ids, $matches, PREG_PATTERN_ORDER); 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 * @param string $item The item to sanitise
*/ */
private function sanitize(string &$item) { private function sanitize(string &$item) {
// The item is an ACL int value
if (intval($item)) { if (intval($item)) {
$item = '<' . intval(Strings::escapeTags(trim($item))) . '>'; $item = '<' . intval(Strings::escapeTags(trim($item))) . '>';
// The item is a allowed ACL character
} elseif (in_array($item, [Group::FOLLOWERS, Group::MUTUALS])) { } elseif (in_array($item, [Group::FOLLOWERS, Group::MUTUALS])) {
$item = '<' . $item . '>'; $item = '<' . $item . '>';
} else { // The item is already a ACL string
} elseif (preg_match('/<\d+?>/', $item)) {
unset($item); unset($item);
// The item is not supported, so remove it (cleanup)
} else {
$item = '';
} }
} }

View file

@ -279,10 +279,10 @@ class Notifier
/** @var ACLFormatter $aclFormatter */ /** @var ACLFormatter $aclFormatter */
$aclFormatter = BaseObject::getClass(ACLFormatter::class); $aclFormatter = BaseObject::getClass(ACLFormatter::class);
$allow_people = $aclFormatter->expand($parent['allow_cid'] ?? ''); $allow_people = $aclFormatter->expand($parent['allow_cid']);
$allow_groups = Group::expand($uid, $aclFormatter->expand($parent['allow_gid'] ?? ''),true); $allow_groups = Group::expand($uid, $aclFormatter->expand($parent['allow_gid']),true);
$deny_people = $aclFormatter->expand($parent['deny_cid'] ?? ''); $deny_people = $aclFormatter->expand($parent['deny_cid']);
$deny_groups = Group::expand($uid, $aclFormatter->expand($parent['deny_gid'] ?? '')); $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 // 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 // a delivery fork. private groups (forum_mode == 2) do not uplink

View file

@ -12,167 +12,111 @@ use PHPUnit\Framework\TestCase;
*/ */
class ACLFormaterTest extends TestCase class ACLFormaterTest extends TestCase
{ {
/** public function assertAcl($text, array $assert = [])
* test expand_acl, perfect input
*/
public function testExpandAclNormal()
{ {
$aclFormatter = new ACLFormatter(); $aclFormatter = new ACLFormatter();
$text='<1><2><3><' . Group::FOLLOWERS . '><' . Group::MUTUALS . '>'; $acl = $aclFormatter->expand($text);
$this->assertEquals(array('1', '2', '3', Group::FOLLOWERS, Group::MUTUALS), $aclFormatter->expand($text));
$this->assertEquals($assert, $acl);
$this->assertMergable($acl);
} }
/** public function assertMergable(array $aclOne, array $aclTwo = [])
* test with a big number
*/
public function testExpandAclBigNumber()
{ {
$aclFormatter = new ACLFormatter(); $this->assertTrue(is_array($aclOne));
$this->assertTrue(is_array($aclTwo));
$text='<1><' . PHP_INT_MAX . '><15>'; $aclMerged = array_unique(array_merge($aclOne, $aclTwo));
$this->assertEquals(array('1', (string)PHP_INT_MAX, '15'), $aclFormatter->expand($text)); $this->assertTrue(is_array($aclMerged));
return $aclMerged;
} }
/** public function dataExpand()
* test with a string in it.
*
* @todo is this valid input? Otherwise: should there be an exception?
*/
public function testExpandAclString()
{ {
$aclFormatter = new ACLFormatter(); return [
'normal' => [
$text="<1><279012><tt>"; 'input' => '<1><2><3><' . Group::FOLLOWERS . '><' . Group::MUTUALS . '>',
$this->assertEquals(array('1', '279012'), $aclFormatter->expand($text)); '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><tt>',
'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' => '<Another invalid string', //should be invalid
'assert' => [],
],
/// @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' => '<Another< invalid string<', //should be invalid
'assert' => [],
],
/// @todo should there be an exception?
'noMatching1' => [
'input' => '<Another<> invalid <string>', //should be invalid
'assert' => [],
],
'emptyMatch' => [
'input' => '<1><><3>',
'assert' => ['1', '3'],
],
];
} }
/** /**
* test with a ' ' in it. * @dataProvider dataExpand
*
* @todo is this valid input? Otherwise: should there be an exception?
*/ */
public function testExpandAclSpace() public function testExpand($input, array $assert)
{ {
$aclFormatter = new ACLFormatter(); $this->assertAcl($input, $assert);
$text="<1><279 012><32>";
$this->assertEquals(array('1', '32'), $aclFormatter->expand($text));
} }
/** /**
* test empty input * Test nullable expand (=> no ACL set)
*/
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="<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 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="<Another< invalid string<"; //should be invalid
$this->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="<Another<> invalid <string>"; //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
*/ */
public function testExpandNull() public function testExpandNull()
{ {
$aclFormatter = new ACLFormatter(); $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() public function dataAclToString()
@ -198,6 +142,23 @@ class ACLFormaterTest extends TestCase
'input' => 'a,bsd23,4', 'input' => 'a,bsd23,4',
'assert' => '<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' => ["<asd>","<123>"],
'assert' => "<123>",
],
]; ];
} }