From a0c8fc6d6ee7f930422929da66ab23b880215f6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Mon, 20 Jun 2022 00:51:59 +0200 Subject: [PATCH 01/20] Changes: - added more type-hints --- src/Core/Renderer.php | 2 +- src/Protocol/Salmon.php | 10 +++--- src/Render/FriendicaSmartyEngine.php | 2 +- src/Render/TemplateEngine.php | 5 +-- src/Util/Strings.php | 54 +++++++++++----------------- 5 files changed, 31 insertions(+), 42 deletions(-) diff --git a/src/Core/Renderer.php b/src/Core/Renderer.php index 29bbb2b894..879fdfe19e 100644 --- a/src/Core/Renderer.php +++ b/src/Core/Renderer.php @@ -71,7 +71,7 @@ class Renderer * @return string * @throws ServiceUnavailableException */ - public static function replaceMacros(string $template, array $vars = []) + public static function replaceMacros(string $template, array $vars = []): string { DI::profiler()->startRecording('rendering'); diff --git a/src/Protocol/Salmon.php b/src/Protocol/Salmon.php index 9724f82df7..98af26c15c 100644 --- a/src/Protocol/Salmon.php +++ b/src/Protocol/Salmon.php @@ -43,7 +43,7 @@ class Salmon * @return mixed * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public static function getKey($uri, $keyhash) + public static function getKey(string $uri, string $keyhash) { $ret = []; @@ -109,18 +109,18 @@ class Salmon * @return integer * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public static function slapper($owner, $url, $slap) + public static function slapper(array $owner, string $url, string $slap): int { // does contact have a salmon endpoint? if (!strlen($url)) { - return; + return -1; } if (!$owner['sprvkey']) { Logger::notice(sprintf("user '%s' (%d) does not have a salmon private key. Send failed.", $owner['name'], $owner['uid'])); - return; + return -1; } Logger::info('slapper called for '.$url.'. Data: ' . $slap); @@ -229,7 +229,7 @@ class Salmon * @return string * @throws \Exception */ - public static function salmonKey($pubkey) + public static function salmonKey(string $pubkey): string { Crypto::pemToMe($pubkey, $modulus, $exponent); return 'RSA' . '.' . Strings::base64UrlEncode($modulus, true) . '.' . Strings::base64UrlEncode($exponent, true); diff --git a/src/Render/FriendicaSmartyEngine.php b/src/Render/FriendicaSmartyEngine.php index 6fc8f44806..9f35fbcc44 100644 --- a/src/Render/FriendicaSmartyEngine.php +++ b/src/Render/FriendicaSmartyEngine.php @@ -69,7 +69,7 @@ final class FriendicaSmartyEngine extends TemplateEngine /** * @inheritDoc */ - public function replaceMacros(string $template, array $vars) + public function replaceMacros(string $template, array $vars): string { if (!Strings::startsWith($template, self::FILE_PREFIX)) { $template = self::STRING_PREFIX . $template; diff --git a/src/Render/TemplateEngine.php b/src/Render/TemplateEngine.php index 6bfbe31971..e160192d36 100644 --- a/src/Render/TemplateEngine.php +++ b/src/Render/TemplateEngine.php @@ -45,6 +45,7 @@ abstract class TemplateEngine * parameter or displays them directly if it's null. * * @param array|null $errors + * @return void */ abstract public function testInstall(array &$errors = null); @@ -53,9 +54,9 @@ abstract class TemplateEngine * * @param string $template * @param array $vars - * @return string + * @return string Template output with replaced macros */ - abstract public function replaceMacros(string $template, array $vars); + abstract public function replaceMacros(string $template, array $vars): string; /** * Returns the template string from a file path and an optional sub-directory from the project root diff --git a/src/Util/Strings.php b/src/Util/Strings.php index d2555132a0..44ddc73259 100644 --- a/src/Util/Strings.php +++ b/src/Util/Strings.php @@ -32,11 +32,11 @@ class Strings /** * Generates a pseudo-random string of hexadecimal characters * - * @param int $size - * @return string + * @param int $size Size of string (default: 64) + * @return string Pseudo-random string * @throws \Exception */ - public static function getRandomHex($size = 64) + public static function getRandomHex(int $size = 64): string { $byte_size = ceil($size / 2); @@ -51,10 +51,9 @@ class Strings * Checks, if the given string is a valid hexadecimal code * * @param string $hexCode - * * @return bool */ - public static function isHex($hexCode) + public static function isHex(string $hexCode): bool { return !empty($hexCode) ? @preg_match("/^[a-f0-9]{2,}$/i", $hexCode) && !(strlen($hexCode) & 1) : false; } @@ -75,10 +74,9 @@ class Strings * Generate a string that's random, but usually pronounceable. Used to generate initial passwords * * @param int $len length - * * @return string */ - public static function getRandomName($len) + public static function getRandomName(int $len): string { if ($len <= 0) { return ''; @@ -161,7 +159,6 @@ class Strings * * @param string $network Network name of the contact (e.g. dfrn, rss and so on) * @param string $url The contact url - * * @return string Formatted network name * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ @@ -189,7 +186,7 @@ class Strings * * @return string Transformed string. */ - public static function deindent(string $text, string $chr = "[\t ]", int $count = null) + public static function deindent(string $text, string $chr = "[\t ]", int $count = null): string { $lines = explode("\n", $text); @@ -218,7 +215,7 @@ class Strings * * @return string Size with measured units. */ - public static function formatBytes($bytes, $precision = 2) + public static function formatBytes(int $bytes, int $precision = 2): string { $units = ['B', 'KB', 'MB', 'GB', 'TB']; $bytes = max($bytes, 0); @@ -233,10 +230,9 @@ class Strings * Protect percent characters in sprintf calls * * @param string $s String to transform. - * * @return string Transformed string. */ - public static function protectSprintf($s) + public static function protectSprintf(string $s): string { return str_replace('%', '%%', $s); } @@ -246,10 +242,9 @@ class Strings * * @param string $s URL to encode * @param boolean $strip_padding Optional. Default false - * * @return string Encoded URL */ - public static function base64UrlEncode($s, $strip_padding = false) + public static function base64UrlEncode(string $s, bool $strip_padding = false): string { $s = strtr(base64_encode($s), '+/', '-_'); @@ -262,18 +257,13 @@ class Strings /** * Decode Base64 Encoded URL and translate -_ to +/ - * @param string $s URL to decode * + * @param string $s URL to decode * @return string Decoded URL * @throws \Exception */ - public static function base64UrlDecode($s) + public static function base64UrlDecode(string $s): string { - if (is_array($s)) { - Logger::notice('base64url_decode: illegal input: ', ['backtrace' => debug_backtrace()]); - return $s; - } - /* * // Placeholder for new rev of salmon which strips base64 padding. * // PHP base64_decode handles the un-padded input without requiring this step @@ -297,10 +287,9 @@ class Strings * Normalize url * * @param string $url URL to be normalized. - * * @return string Normalized URL. */ - public static function normaliseLink($url) + public static function normaliseLink(string $url): string { $ret = str_replace(['https:', '//www.'], ['http:', '//'], $url); return rtrim($ret, '/'); @@ -310,10 +299,9 @@ class Strings * Normalize OpenID identity * * @param string $s OpenID Identity - * * @return string normalized OpenId Identity */ - public static function normaliseOpenID($s) + public static function normaliseOpenID(string $s): string { return trim(str_replace(['http://', 'https://'], ['', ''], $s), '/'); } @@ -329,7 +317,7 @@ class Strings * @return boolean True if the URLs match, otherwise False * */ - public static function compareLink($a, $b) + public static function compareLink(string $a, string $b): bool { return (strcasecmp(self::normaliseLink($a), self::normaliseLink($b)) === 0); } @@ -340,7 +328,7 @@ class Strings * @param string $uri * @return string */ - public static function ensureQueryParameter($uri) + public static function ensureQueryParameter(string $uri): string { if (strpos($uri, '?') === false && ($pos = strpos($uri, '&')) !== false) { $uri = substr($uri, 0, $pos) . '?' . substr($uri, $pos + 1); @@ -356,7 +344,7 @@ class Strings * @param array $chars * @return bool */ - public static function startsWithChars($string, array $chars) + public static function startsWithChars(string $string, array $chars): bool { $return = in_array(substr(trim($string), 0, 1), $chars); @@ -371,7 +359,7 @@ class Strings * @param string $start * @return bool */ - public static function startsWith(string $string, string $start) + public static function startsWith(string $string, string $start): bool { $return = substr_compare($string, $start, 0, strlen($start)) === 0; @@ -386,7 +374,7 @@ class Strings * @param string $end * @return bool */ - public static function endsWith(string $string, string $end) + public static function endsWith(string $string, string $end): string { $return = substr_compare($string, $end, -strlen($end)) === 0; @@ -399,7 +387,7 @@ class Strings * @return string * @see https://daringfireball.net/2010/07/improved_regex_for_matching_urls */ - public static function autoLinkRegEx() + public static function autoLinkRegEx(): string { return '@ (? Date: Mon, 20 Jun 2022 01:08:52 +0200 Subject: [PATCH 02/20] Changes: - added type-hints - added missing documentation --- src/Database/View.php | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/src/Database/View.php b/src/Database/View.php index a31151c211..c8941167e0 100644 --- a/src/Database/View.php +++ b/src/Database/View.php @@ -39,12 +39,12 @@ class View * On first pass, defines DB_UPDATE_VERSION constant. * * @see static/dbview.config.php - * @param boolean $with_addons_structure Whether to tack on addons additional tables * @param string $basePath The base path of this application + * @param boolean $with_addons_structure Whether to tack on addons additional tables * @return array * @throws Exception */ - public static function definition($basePath = '', $with_addons_structure = true) + public static function definition(string $basePath = '', bool $with_addons_structure = true): array { if (!self::$definition) { if (empty($basePath)) { @@ -75,6 +75,13 @@ class View return $definition; } + /** + * Creates a view + * + * @param bool $verbose Whether to show SQL statements + * @param bool $action Whether to execute SQL statements + * @return void + */ public static function create(bool $verbose, bool $action) { // Delete previously used views that aren't used anymore @@ -94,11 +101,17 @@ class View $definition = self::definition(); foreach ($definition as $name => $structure) { - self::createview($name, $structure, $verbose, $action); + self::createView($name, $structure, $verbose, $action); } } - public static function printStructure($basePath) + /** + * Prints view structure + * + * @param string $basePath Base path + * @return void + */ + public static function printStructure(string $basePath) { $database = self::definition($basePath, false); @@ -112,12 +125,21 @@ class View } } - private static function createview($name, $structure, $verbose, $action) + /** + * Creates view + * + * @param string $name Name of view + * @param array $structure Structure of view + * @param bool $verbose Whether to show SQL statements + * @param bool $action Whether to execute SQL statements + * @return bool Whether execution went fine + */ + private static function createView(string $name, array $structure, bool $verbose, bool $action): bool { $r = true; $sql_rows = []; - foreach ($structure["fields"] as $fieldname => $origin) { + foreach ($structure['fields'] as $fieldname => $origin) { if (is_string($origin)) { $sql_rows[] = $origin . " AS `" . DBA::escape($fieldname) . "`"; } elseif (is_array($origin) && (sizeof($origin) == 2)) { @@ -159,7 +181,7 @@ class View * @param string $view * @return boolean "true" if it's a view */ - private static function isView(string $view) + private static function isView(string $view): bool { $status = DBA::selectFirst(['INFORMATION_SCHEMA' => 'TABLES'], ['TABLE_TYPE'], ['TABLE_SCHEMA' => DBA::databaseName(), 'TABLE_NAME' => $view]); @@ -177,7 +199,7 @@ class View * @param string $table * @return boolean "true" if it's a table */ - private static function isTable(string $table) + private static function isTable(string $table): bool { $status = DBA::selectFirst(['INFORMATION_SCHEMA' => 'TABLES'], ['TABLE_TYPE'], ['TABLE_SCHEMA' => DBA::databaseName(), 'TABLE_NAME' => $table]); From 14bf72e4feeb91d70ad0c055c9b6620617e89452 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Mon, 20 Jun 2022 02:45:53 +0200 Subject: [PATCH 03/20] Changes: - added some documentation - added type-hints --- mod/fbrowser.php | 2 +- src/Console/Addon.php | 9 +++------ src/Core/Addon.php | 14 ++++++++------ src/Core/Hook.php | 30 ++++++++++++++++++------------ src/Core/L10n.php | 26 ++++++++++++-------------- src/Core/Theme.php | 37 +++++++++++++++++++++++++------------ 6 files changed, 67 insertions(+), 51 deletions(-) diff --git a/mod/fbrowser.php b/mod/fbrowser.php index 81284d6b91..60b290850a 100644 --- a/mod/fbrowser.php +++ b/mod/fbrowser.php @@ -47,7 +47,7 @@ function fbrowser_content(App $a) } // Needed to match the correct template in a module that uses a different theme than the user/site/default - $theme = Strings::sanitizeFilePathItem($_GET['theme'] ?? null); + $theme = Strings::sanitizeFilePathItem($_GET['theme'] ?? ''); if ($theme && is_file("view/theme/$theme/config.php")) { $a->setCurrentTheme($theme); } diff --git a/src/Console/Addon.php b/src/Console/Addon.php index 1389e97bf4..74b0818100 100644 --- a/src/Console/Addon.php +++ b/src/Console/Addon.php @@ -116,8 +116,7 @@ HELP; /** * Lists plugins * - * @return int Return code of this command - * + * @return int|bool Return code of this command, false on error (?) * @throws \Exception */ private function list() @@ -165,10 +164,9 @@ HELP; * Enables an addon * * @return int Return code of this command - * * @throws \Exception */ - private function enable() + private function enable(): int { $addonname = $this->getArgument(1); @@ -190,10 +188,9 @@ HELP; * Disables an addon * * @return int Return code of this command - * * @throws \Exception */ - private function disable() + private function disable(): int { $addonname = $this->getArgument(1); diff --git a/src/Core/Addon.php b/src/Core/Addon.php index f6d0d49740..37ef335a47 100644 --- a/src/Core/Addon.php +++ b/src/Core/Addon.php @@ -124,7 +124,7 @@ class Addon * @return void * @throws \Exception */ - public static function uninstall($addon) + public static function uninstall(string $addon) { $addon = Strings::sanitizeFilePathItem($addon); @@ -149,7 +149,7 @@ class Addon * @return bool * @throws \Exception */ - public static function install($addon) + public static function install(string $addon): bool { $addon = Strings::sanitizeFilePathItem($addon); @@ -185,6 +185,8 @@ class Addon /** * reload all updated addons + * + * @return void */ public static function reload() { @@ -222,7 +224,7 @@ class Addon * @return array with the addon information * @throws \Exception */ - public static function getInfo($addon) + public static function getInfo(string $addon): array { $addon = Strings::sanitizeFilePathItem($addon); @@ -287,7 +289,7 @@ class Addon * @param string $addon * @return boolean */ - public static function isEnabled($addon) + public static function isEnabled(string $addon): bool { return in_array($addon, self::$addons); } @@ -297,7 +299,7 @@ class Addon * * @return array */ - public static function getEnabledList() + public static function getEnabledList(): array { return self::$addons; } @@ -308,7 +310,7 @@ class Addon * @return array * @throws \Exception */ - public static function getVisibleList() + public static function getVisibleList(): array { $visible_addons = []; $stmt = DBA::select('addon', ['name'], ['hidden' => false, 'installed' => true]); diff --git a/src/Core/Hook.php b/src/Core/Hook.php index 89e882e35e..f2fb52f5e8 100644 --- a/src/Core/Hook.php +++ b/src/Core/Hook.php @@ -49,6 +49,8 @@ class Hook /** * Load hooks + * + * @return void */ public static function loadHooks() { @@ -69,8 +71,9 @@ class Hook * @param string $hook * @param string $file * @param string $function + * @return void */ - public static function add($hook, $file, $function) + public static function add(string $hook, string $file, string $function) { if (!array_key_exists($hook, self::$hooks)) { self::$hooks[$hook] = []; @@ -90,7 +93,7 @@ class Hook * @return mixed|bool * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public static function register($hook, $file, $function, $priority = 0) + public static function register(string $hook, string $file, string $function, int $priority = 0) { $file = str_replace(DI::app()->getBasePath() . DIRECTORY_SEPARATOR, '', $file); @@ -111,7 +114,7 @@ class Hook * @return boolean * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public static function unregister($hook, $file, $function) + public static function unregister(string $hook, string $file, string $function): bool { $relative_file = str_replace(DI::app()->getBasePath() . DIRECTORY_SEPARATOR, '', $file); @@ -120,8 +123,8 @@ class Hook self::delete($condition); $condition = ['hook' => $hook, 'file' => $relative_file, 'function' => $function]; - $result = self::delete($condition); - return $result; + + return self::delete($condition); } /** @@ -130,7 +133,7 @@ class Hook * @param string $name Name of the hook * @return array */ - public static function getByName($name) + public static function getByName(string $name): array { $return = []; @@ -149,9 +152,10 @@ class Hook * @param integer $priority of the hook * @param string $name of the hook to call * @param mixed $data to transmit to the callback handler + * @return void * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public static function fork($priority, $name, $data = null) + public static function fork(int $priority, string $name, $data = null) { if (array_key_exists($name, self::$hooks)) { foreach (self::$hooks[$name] as $hook) { @@ -184,9 +188,10 @@ class Hook * * @param string $name of the hook to call * @param string|array &$data to transmit to the callback handler + * @return void * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public static function callAll($name, &$data = null) + public static function callAll(string $name, &$data = null) { if (array_key_exists($name, self::$hooks)) { foreach (self::$hooks[$name] as $hook) { @@ -202,9 +207,10 @@ class Hook * @param string $name of the hook to call * @param array $hook Hook data * @param string|array &$data to transmit to the callback handler + * @return void * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public static function callSingle(App $a, $name, $hook, &$data = null) + public static function callSingle(App $a, string $name, array $hook, &$data = null) { // Don't run a theme's hook if the user isn't using the theme if (strpos($hook[0], 'view/theme/') !== false && strpos($hook[0], 'view/theme/' . $a->getCurrentTheme()) === false) { @@ -229,7 +235,7 @@ class Hook * @param string $name Name of the addon * @return boolean */ - public static function isAddonApp($name) + public static function isAddonApp(string $name): bool { $name = Strings::sanitizeFilePathItem($name); @@ -253,7 +259,7 @@ class Hook * @return bool * @throws \Exception */ - public static function delete(array $condition) + public static function delete(array $condition): bool { $result = DBA::delete('hook', $condition); @@ -273,7 +279,7 @@ class Hook * @return bool * @throws \Exception */ - private static function insert(array $condition) + private static function insert(array $condition): bool { $result = DBA::insert('hook', $condition); diff --git a/src/Core/L10n.php b/src/Core/L10n.php index 050c190737..08296068bb 100644 --- a/src/Core/L10n.php +++ b/src/Core/L10n.php @@ -128,7 +128,7 @@ class L10n private function setLangFromSession(IHandleSessions $session) { if ($session->get('language') !== $this->lang) { - $this->loadTranslationTable($session->get('language')); + $this->loadTranslationTable($session->get('language') ?? $this->lang); } } @@ -140,10 +140,10 @@ class L10n * Uses an App object shim since all the strings files refer to $a->strings * * @param string $lang language code to load - * + * @return void * @throws \Exception */ - private function loadTranslationTable($lang) + private function loadTranslationTable(string $lang) { $lang = Strings::sanitizeFilePathItem($lang); @@ -183,7 +183,7 @@ class L10n * * @return string The two-letter language code */ - public static function detectLanguage(array $server, array $get, string $sysLang = self::DEFAULT) + public static function detectLanguage(array $server, array $get, string $sysLang = self::DEFAULT): string { $lang_variable = $server['HTTP_ACCEPT_LANGUAGE'] ?? null; @@ -269,7 +269,7 @@ class L10n * * @return string */ - public function t($s, ...$vars) + public function t(array $s, ...$vars): string { if (empty($s)) { return ''; @@ -307,7 +307,7 @@ class L10n * @return string * @throws \Exception */ - public function tt(string $singular, string $plural, int $count) + public function tt(string $singular, string $plural, int $count): string { $s = null; @@ -352,7 +352,7 @@ class L10n * * @return bool */ - private function stringPluralSelectDefault($n) + private function stringPluralSelectDefault(int $n): bool { return $n != 1; } @@ -369,7 +369,7 @@ class L10n * * @return array */ - public static function getAvailableLanguages() + public static function getAvailableLanguages(): array { $langs = []; $strings_file_paths = glob('view/lang/*/strings.php'); @@ -391,10 +391,9 @@ class L10n * Translate days and months names. * * @param string $s String with day or month name. - * * @return string Translated string. */ - public function getDay($s) + public function getDay(string $s): string { $ret = str_replace(['Monday', 'Tuesday', 'Wednesday', 'Thursday', 'Friday', 'Saturday', 'Sunday'], [$this->t('Monday'), $this->t('Tuesday'), $this->t('Wednesday'), $this->t('Thursday'), $this->t('Friday'), $this->t('Saturday'), $this->t('Sunday')], @@ -411,10 +410,9 @@ class L10n * Translate short days and months names. * * @param string $s String with short day or month name. - * * @return string Translated string. */ - public function getDayShort($s) + public function getDayShort(string $s): string { $ret = str_replace(['Mon', 'Tue', 'Wed', 'Thu', 'Fri', 'Sat', 'Sun'], [$this->t('Mon'), $this->t('Tue'), $this->t('Wed'), $this->t('Thu'), $this->t('Fri'), $this->t('Sat'), $this->t('Sun')], @@ -435,7 +433,7 @@ class L10n * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @hook poke_verbs pokes array */ - public function getPokeVerbs() + public function getPokeVerbs(): array { // index is present tense verb // value is array containing past tense verb, translation of present, translation of past @@ -461,7 +459,7 @@ class L10n * @return static A new L10n instance * @throws \Exception */ - public function withLang(string $lang) + public function withLang(string $lang): L10n { // Don't create a new instance for same language if ($lang === $this->lang) { diff --git a/src/Core/Theme.php b/src/Core/Theme.php index 57dfaa4d63..834787da69 100644 --- a/src/Core/Theme.php +++ b/src/Core/Theme.php @@ -32,7 +32,7 @@ require_once 'boot.php'; */ class Theme { - public static function getAllowedList() + public static function getAllowedList(): array { $allowed_themes_str = DI::config()->get('system', 'allowed_themes'); $allowed_themes_raw = explode(',', str_replace(' ', '', $allowed_themes_str)); @@ -69,7 +69,7 @@ class Theme * @param string $theme the name of the theme * @return array */ - public static function getInfo($theme) + public static function getInfo(string $theme): array { $theme = Strings::sanitizeFilePathItem($theme); @@ -133,7 +133,7 @@ class Theme * @return string * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public static function getScreenshot($theme) + public static function getScreenshot(string $theme): string { $theme = Strings::sanitizeFilePathItem($theme); @@ -146,7 +146,13 @@ class Theme return DI::baseUrl() . '/images/blank.png'; } - public static function uninstall($theme) + /** + * Uninstalls given theme name + * + * @param string $theme Name of theme + * @return bool true on success + */ + public static function uninstall(string $theme) { $theme = Strings::sanitizeFilePathItem($theme); @@ -167,10 +173,18 @@ class Theme if ($key !== false) { unset($allowed_themes[$key]); Theme::setAllowedList($allowed_themes); + return true; } + return false; } - public static function install($theme) + /** + * Installs given theme name + * + * @param string $theme Name of theme + * @return bool true on success + */ + public static function install(string $theme): bool { $theme = Strings::sanitizeFilePathItem($theme); @@ -208,7 +222,7 @@ class Theme * @return string Path to the file or empty string if the file isn't found * @throws \Exception */ - public static function getPathForFile($file) + public static function getPathForFile(string $file): string { $a = DI::app(); @@ -237,10 +251,9 @@ class Theme * Provide a sane default if nothing is chosen or the specified theme does not exist. * * @param string $theme Theme name - * * @return string */ - public static function getStylesheetPath($theme) + public static function getStylesheetPath(string $theme): string { $theme = Strings::sanitizeFilePathItem($theme); @@ -263,10 +276,10 @@ class Theme /** * Returns the path of the provided theme * - * @param $theme + * @param string $theme Theme name * @return string|null */ - public static function getConfigFile($theme) + public static function getConfigFile(string $theme) { $theme = Strings::sanitizeFilePathItem($theme); @@ -285,11 +298,11 @@ class Theme /** * Returns the background color of the provided theme if available. * - * @param string $theme + * @param string $theme Theme name * @param int|null $uid Current logged-in user id * @return string|null */ - public static function getBackgroundColor(string $theme, $uid = null) + public static function getBackgroundColor(string $theme, int $uid = null) { $theme = Strings::sanitizeFilePathItem($theme); From ec96f2252ecc51f079d5169851b1f2b7ea5a26ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Mon, 20 Jun 2022 03:10:02 +0200 Subject: [PATCH 04/20] Changes: - added type-hints - added some missing documentation --- src/Database/DBA.php | 70 ++++++++++++------------ src/Database/Database.php | 111 +++++++++++++++++++++++--------------- 2 files changed, 101 insertions(+), 80 deletions(-) diff --git a/src/Database/DBA.php b/src/Database/DBA.php index d62edf6ce1..d0a0bbe2f4 100644 --- a/src/Database/DBA.php +++ b/src/Database/DBA.php @@ -90,7 +90,7 @@ class DBA * * @return string */ - public static function serverInfo() + public static function serverInfo(): string { return DI::dba()->serverInfo(); } @@ -101,7 +101,7 @@ class DBA * @return string * @throws \Exception */ - public static function databaseName() + public static function databaseName(): string { return DI::dba()->databaseName(); } @@ -112,7 +112,7 @@ class DBA * @param string $str * @return string escaped string */ - public static function escape($str) + public static function escape(string $str): string { return DI::dba()->escape($str); } @@ -122,7 +122,7 @@ class DBA * * @return boolean is the database connected? */ - public static function connected() + public static function connected(): bool { return DI::dba()->connected(); } @@ -138,7 +138,7 @@ class DBA * @param string $sql An SQL string without the values * @return string The input SQL string modified if necessary. */ - public static function anyValueFallback($sql) + public static function anyValueFallback(string $sql): string { return DI::dba()->anyValueFallback($sql); } @@ -152,7 +152,7 @@ class DBA * @param string $sql An SQL string without the values * @return string The input SQL string modified if necessary. */ - public static function cleanQuery($sql) + public static function cleanQuery(string $sql): string { $search = ["\t", "\n", "\r", " "]; $replace = [' ', ' ', ' ', ' ']; @@ -169,7 +169,7 @@ class DBA * @param array $args Parameter array * @return array universalized parameter array */ - public static function getParam($args) + public static function getParam(array $args): array { unset($args[0]); @@ -192,7 +192,7 @@ class DBA * @return bool|object statement object or result object * @throws \Exception */ - public static function p($sql) + public static function p(string $sql) { $params = self::getParam(func_get_args()); @@ -208,8 +208,8 @@ class DBA * @return boolean Was the query successfull? False is returned only if an error occurred * @throws \Exception */ - public static function e($sql) { - + public static function e(string $sql): bool + { $params = self::getParam(func_get_args()); return DI::dba()->e($sql, $params); @@ -220,11 +220,10 @@ class DBA * * @param string|array $table Table name or array [schema => table] * @param array $condition array of fields for condition - * * @return boolean Are there rows for that condition? * @throws \Exception */ - public static function exists($table, $condition) + public static function exists(string $table, array $condition): bool { return DI::dba()->exists($table, $condition); } @@ -238,7 +237,7 @@ class DBA * @return array first row of query * @throws \Exception */ - public static function fetchFirst($sql) + public static function fetchFirst(string $sql) { $params = self::getParam(func_get_args()); @@ -250,7 +249,7 @@ class DBA * * @return int Number of rows */ - public static function affectedRows() + public static function affectedRows(): int { return DI::dba()->affectedRows(); } @@ -261,7 +260,7 @@ class DBA * @param object Statement object * @return int Number of columns */ - public static function columnCount($stmt) + public static function columnCount($stmt): int { return DI::dba()->columnCount($stmt); } @@ -271,7 +270,7 @@ class DBA * @param PDOStatement|mysqli_result|mysqli_stmt Statement object * @return int Number of rows */ - public static function numRows($stmt) + public static function numRows($stmt): int { return DI::dba()->numRows($stmt); } @@ -297,7 +296,7 @@ class DBA * @return boolean was the insert successful? * @throws \Exception */ - public static function insert($table, array $param, int $duplicate_mode = Database::INSERT_DEFAULT) + public static function insert($table, array $param, int $duplicate_mode = Database::INSERT_DEFAULT): bool { return DI::dba()->insert($table, $param, $duplicate_mode); } @@ -312,7 +311,7 @@ class DBA * @return boolean was the insert successful? * @throws \Exception */ - public static function replace($table, $param) + public static function replace($table, array $param): bool { return DI::dba()->replace($table, $param); } @@ -322,7 +321,7 @@ class DBA * * @return integer Last inserted id */ - public static function lastInsertId() + public static function lastInsertId(): int { return DI::dba()->lastInsertId(); } @@ -337,7 +336,7 @@ class DBA * @return boolean was the lock successful? * @throws \Exception */ - public static function lock($table) + public static function lock($table): bool { return DI::dba()->lock($table); } @@ -348,7 +347,7 @@ class DBA * @return boolean was the unlock successful? * @throws \Exception */ - public static function unlock() + public static function unlock(): bool { return DI::dba()->unlock(); } @@ -358,7 +357,7 @@ class DBA * * @return boolean Was the command executed successfully? */ - public static function transaction() + public static function transaction(): bool { return DI::dba()->transaction(); } @@ -368,7 +367,7 @@ class DBA * * @return boolean Was the command executed successfully? */ - public static function commit() + public static function commit(): bool { return DI::dba()->commit(); } @@ -378,7 +377,7 @@ class DBA * * @return boolean Was the command executed successfully? */ - public static function rollback() + public static function rollback(): bool { return DI::dba()->rollback(); } @@ -392,7 +391,7 @@ class DBA * @return boolean was the delete successful? * @throws \Exception */ - public static function delete($table, array $conditions, array $options = []) + public static function delete($table, array $conditions, array $options = []): bool { return DI::dba()->delete($table, $conditions, $options); } @@ -427,7 +426,7 @@ class DBA * @return boolean was the update successfull? * @throws \Exception */ - public static function update($table, array $fields, array $condition, $old_fields = [], array $params = []) + public static function update($table, array $fields, array $condition, $old_fields = [], array $params = []): bool { return DI::dba()->update($table, $fields, $condition, $old_fields, $params); } @@ -528,7 +527,7 @@ class DBA * @param string|array $tables * @return string */ - public static function buildTableString($tables) + public static function buildTableString($tables): string { if (is_string($tables)) { $tables = [$tables]; @@ -553,7 +552,7 @@ class DBA * @param $identifier * @return string */ - public static function quoteIdentifier($identifier) + public static function quoteIdentifier(string $identifier): string { return '`' . str_replace('`', '``', $identifier) . '`'; } @@ -576,7 +575,7 @@ class DBA * @param array $condition * @return string */ - public static function buildCondition(array &$condition = []) + public static function buildCondition(array &$condition = []): string { $condition = self::collapseCondition($condition); @@ -600,7 +599,7 @@ class DBA * @param array $condition * @return array */ - public static function collapseCondition(array $condition) + public static function collapseCondition(array $condition): array { // Ensures an always true condition is returned if (count($condition) < 1) { @@ -675,7 +674,7 @@ class DBA * @return array A collapsed condition * @see DBA::collapseCondition() for the condition array formats */ - public static function mergeConditions(array ...$conditions) + public static function mergeConditions(array ...$conditions): array { if (count($conditions) == 1) { return current($conditions); @@ -724,7 +723,7 @@ class DBA * @param array $params * @return string */ - public static function buildParameter(array $params = []) + public static function buildParameter(array $params = []): string { $groupby_string = ''; if (!empty($params['group_by'])) { @@ -771,7 +770,7 @@ class DBA * * @return array Data array */ - public static function toArray($stmt, $do_close = true, int $count = 0): array + public static function toArray($stmt, bool $do_close = true, int $count = 0): array { return DI::dba()->toArray($stmt, $do_close, $count); } @@ -847,10 +846,9 @@ class DBA * Checks if $array is a filled array with at least one entry. * * @param mixed $array A filled array with at least one entry - * * @return boolean Whether $array is a filled array or an object with rows */ - public static function isResult($array) + public static function isResult($array): bool { return DI::dba()->isResult($array); } @@ -862,7 +860,7 @@ class DBA * @param boolean $add_quotation add quotation marks for string values * @return void */ - public static function escapeArray(&$arr, $add_quotation = false) + public static function escapeArray(&$arr, bool $add_quotation = false) { DI::dba()->escapeArray($arr, $add_quotation); } diff --git a/src/Database/Database.php b/src/Database/Database.php index 7edbdf2a2c..590c3d83d0 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -88,7 +88,12 @@ class Database } } - public function connect() + /** + * Tries to connect to database + * + * @return bool Success + */ + public function connect(): bool { if (!is_null($this->connection) && $this->connected()) { return $this->connected; @@ -255,7 +260,7 @@ class Database * * @return string with either "pdo" or "mysqli" */ - public function getDriver() + public function getDriver(): string { return $this->driver; } @@ -266,9 +271,9 @@ class Database * This function discriminate between the deprecated mysql API and the current * object-oriented mysqli API. Example of returned string: 5.5.46-0+deb8u1 * - * @return string + * @return string Database server information */ - public function serverInfo() + public function serverInfo(): string { if ($this->server_info == '') { switch ($this->driver) { @@ -286,10 +291,10 @@ class Database /** * Returns the selected database name * - * @return string + * @return string Database name * @throws \Exception */ - public function databaseName() + public function databaseName(): string { $ret = $this->p("SELECT DATABASE() AS `db`"); $data = $this->toArray($ret); @@ -300,10 +305,10 @@ class Database * Analyze a database query and log this if some conditions are met. * * @param string $query The database query that will be analyzed - * + * @return void * @throws \Exception */ - private function logIndex($query) + private function logIndex(string $query) { if (!$this->configCache->get('system', 'db_log_index')) { @@ -359,11 +364,10 @@ class Database * Removes every not allowlisted character from the identifier string * * @param string $identifier - * * @return string sanitized identifier * @throws \Exception */ - private function sanitizeIdentifier($identifier) + private function sanitizeIdentifier(string $identifier): string { return preg_replace('/[^A-Za-z0-9_\-]+/', '', $identifier); } @@ -383,11 +387,21 @@ class Database } } - public function isConnected() + /** + * Returns connected flag + * + * @return bool Whether connection to database was success + */ + public function isConnected(): bool { return $this->connected; } + /** + * Checks connection status + * + * @return bool Whether connection to database was success + */ public function connected() { $connected = false; @@ -424,7 +438,7 @@ class Database * * @return string The input SQL string modified if necessary. */ - public function anyValueFallback($sql) + public function anyValueFallback(string $sql): string { $server_info = $this->serverInfo(); if (version_compare($server_info, '5.7.5', '<') || @@ -442,7 +456,7 @@ class Database * * @return string The replaced SQL query */ - private function replaceParameters($sql, $args) + private function replaceParameters(string $sql, array $args): string { $offset = 0; foreach ($args as $param => $value) { @@ -476,7 +490,7 @@ class Database * @return bool|object statement object or result object * @throws \Exception */ - public function p($sql) + public function p(string $sql) { $this->profiler->startRecording('database'); @@ -741,8 +755,9 @@ class Database * @return boolean Was the query successfull? False is returned only if an error occurred * @throws \Exception */ - public function e($sql) + public function e(string $sql): bool { + $retval = false; $this->profiler->startRecording('database_write'); @@ -810,7 +825,7 @@ class Database * @return boolean Are there rows for that condition? * @throws \Exception */ - public function exists($table, $condition) + public function exists($table, array $condition): bool { if (empty($table)) { return false; @@ -850,10 +865,10 @@ class Database * * @param string $sql SQL statement * - * @return array first row of query + * @return array|bool first row of query or false on failure * @throws \Exception */ - public function fetchFirst($sql) + public function fetchFirst(string $sql) { $params = DBA::getParam(func_get_args()); @@ -875,7 +890,7 @@ class Database * * @return int Number of rows */ - public function affectedRows() + public function affectedRows(): int { return $this->affected_rows; } @@ -887,7 +902,7 @@ class Database * * @return int Number of columns */ - public function columnCount($stmt) + public function columnCount($stmt): int { if (!is_object($stmt)) { return 0; @@ -908,7 +923,7 @@ class Database * * @return int Number of rows */ - public function numRows($stmt) + public function numRows($stmt): int { if (!is_object($stmt)) { return 0; @@ -927,7 +942,7 @@ class Database * * @param bool|PDOStatement|mysqli_stmt $stmt statement object * - * @return array|false current row + * @return array|bool Current row or false on failure */ public function fetch($stmt) { @@ -994,7 +1009,7 @@ class Database * @return boolean was the insert successful? * @throws \Exception */ - public function insert($table, array $param, int $duplicate_mode = self::INSERT_DEFAULT) + public function insert($table, array $param, int $duplicate_mode = self::INSERT_DEFAULT): bool { if (empty($table) || empty($param)) { $this->logger->info('Table and fields have to be set'); @@ -1044,7 +1059,7 @@ class Database * @return boolean was the insert successful? * @throws \Exception */ - public function replace($table, array $param) + public function replace($table, array $param): bool { if (empty($table) || empty($param)) { $this->logger->info('Table and fields have to be set'); @@ -1069,7 +1084,7 @@ class Database * * @return integer Last inserted id */ - public function lastInsertId() + public function lastInsertId(): int { switch ($this->driver) { case self::PDO: @@ -1092,7 +1107,7 @@ class Database * @return boolean was the lock successful? * @throws \Exception */ - public function lock($table) + public function lock($table): bool { // See here: https://dev.mysql.com/doc/refman/5.7/en/lock-tables-and-transactions.html if ($this->driver == self::PDO) { @@ -1126,7 +1141,7 @@ class Database * @return boolean was the unlock successful? * @throws \Exception */ - public function unlock() + public function unlock(): bool { // See here: https://dev.mysql.com/doc/refman/5.7/en/lock-tables-and-transactions.html $this->performCommit(); @@ -1177,7 +1192,12 @@ class Database return true; } - protected function performCommit() + /** + * Performs the commit + * + * @return boolean Was the command executed successfully? + */ + protected function performCommit(): bool { switch ($this->driver) { case self::PDO: @@ -1199,7 +1219,7 @@ class Database * * @return boolean Was the command executed successfully? */ - public function commit() + public function commit(): bool { if (!$this->performCommit()) { return false; @@ -1213,7 +1233,7 @@ class Database * * @return boolean Was the command executed successfully? */ - public function rollback() + public function rollback(): bool { $ret = false; @@ -1230,6 +1250,7 @@ class Database $ret = $this->connection->rollback(); break; } + $this->in_transaction = false; return $ret; } @@ -1243,7 +1264,7 @@ class Database * @return boolean was the delete successful? * @throws \Exception */ - public function delete($table, array $conditions) + public function delete($table, array $conditions): bool { if (empty($table) || empty($conditions)) { $this->logger->info('Table and conditions have to be set'); @@ -1288,8 +1309,9 @@ class Database * * @return boolean was the update successfull? * @throws \Exception + * @todo Implement "bool $update_on_duplicate" to avoid mixed type for $old_fields */ - public function update($table, $fields, $condition, $old_fields = [], $params = []) + public function update($table, array $fields, array $condition, $old_fields = [], array $params = []) { if (empty($table) || empty($fields) || empty($condition)) { $this->logger->info('Table, fields and condition have to be set'); @@ -1354,7 +1376,7 @@ class Database * @throws \Exception * @see $this->select */ - public function selectFirst($table, array $fields = [], array $condition = [], $params = []) + public function selectFirst($table, array $fields = [], array $condition = [], array $params = []) { $params['limit'] = 1; $result = $this->select($table, $fields, $condition, $params); @@ -1390,9 +1412,9 @@ class Database * * @param array $fields * @param array $options - * @return array + * @return array Escaped fields */ - private function escapeFields(array $fields, array $options) + private function escapeFields(array $fields, array $options): array { // In the case of a "GROUP BY" we have to add all the ORDER fields to the fieldlist. // This needs to done to apply the "ANY_VALUE(...)" treatment from below to them. @@ -1504,6 +1526,7 @@ class Database */ public function count($table, array $condition = [], array $params = []) { + // @TODO Can we dump this to have ": int" as returned type-hint? if (empty($table)) { return false; } @@ -1569,7 +1592,8 @@ class Database * @param array $fields * @return array casted fields */ - public function castFields(string $table, array $fields) { + public function castFields(string $table, array $fields): array + { // When there is no data, we don't need to do something if (empty($fields)) { return $fields; @@ -1642,7 +1666,7 @@ class Database * * @return string Error message ('' if no error) */ - public function errorMessage() + public function errorMessage(): string { return $this->error; } @@ -1729,7 +1753,7 @@ class Database * @param string $name * @return string content */ - public function getVariable(string $name) + public function getVariable(string $name): string { $result = $this->fetchFirst("SHOW GLOBAL VARIABLES WHERE `Variable_name` = ?", $name); return $result['Value'] ?? null; @@ -1742,7 +1766,7 @@ class Database * * @return boolean Whether $array is a filled array or an object with rows */ - public function isResult($array) + public function isResult($array): bool { // It could be a return value from an update statement if (is_bool($array)) { @@ -1762,10 +1786,9 @@ class Database * @param mixed $value Array value * @param string $key Array key * @param boolean $add_quotation add quotation marks for string values - * * @return void */ - private function escapeArrayCallback(&$value, $key, $add_quotation) + private function escapeArrayCallback(&$value, string $key, bool $add_quotation) { if (!$add_quotation) { if (is_bool($value)) { @@ -1790,10 +1813,9 @@ class Database * * @param mixed $arr Array with values to be escaped * @param boolean $add_quotation add quotation marks for string values - * * @return void */ - public function escapeArray(&$arr, $add_quotation = false) + public function escapeArray(&$arr, bool $add_quotation = false) { array_walk($arr, [$this, 'escapeArrayCallback'], $add_quotation); } @@ -1801,10 +1823,11 @@ class Database /** * Replaces a string in the provided fields of the provided table * - * @param string $table_name + * @param string $table_name Table name * @param array $fields List of field names in the provided table * @param string $search * @param string $replace + * @return void * @throws \Exception */ public function replaceInTableFields(string $table_name, array $fields, string $search, string $replace) From 94a594eeb2035377b776bba7b4b876fa93670dc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Mon, 20 Jun 2022 03:36:13 +0200 Subject: [PATCH 05/20] Ops, wrong type-hint --- src/Core/L10n.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/L10n.php b/src/Core/L10n.php index 08296068bb..2c45fb9551 100644 --- a/src/Core/L10n.php +++ b/src/Core/L10n.php @@ -269,7 +269,7 @@ class L10n * * @return string */ - public function t(array $s, ...$vars): string + public function t(string $s, ...$vars): string { if (empty($s)) { return ''; From cd3b01fd82f9230a4bc4029b3defe82cd702ffae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Mon, 20 Jun 2022 03:48:12 +0200 Subject: [PATCH 06/20] Changed: - cannot have type-hints :-( --- src/Database/DBA.php | 2 +- view/theme/frio/style.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Database/DBA.php b/src/Database/DBA.php index d0a0bbe2f4..4eff592f09 100644 --- a/src/Database/DBA.php +++ b/src/Database/DBA.php @@ -223,7 +223,7 @@ class DBA * @return boolean Are there rows for that condition? * @throws \Exception */ - public static function exists(string $table, array $condition): bool + public static function exists($table, array $condition): bool { return DI::dba()->exists($table, $condition); } diff --git a/view/theme/frio/style.php b/view/theme/frio/style.php index b39cef1029..6d7e7b73f6 100644 --- a/view/theme/frio/style.php +++ b/view/theme/frio/style.php @@ -79,7 +79,7 @@ if (!empty($_REQUEST['scheme'])) { $scheme = $_REQUEST['scheme']; } -$scheme = Strings::sanitizeFilePathItem($scheme); +$scheme = Strings::sanitizeFilePathItem($scheme ?? ''); if (($scheme) && ($scheme != '---')) { if (file_exists('view/theme/frio/scheme/' . $scheme . '.php')) { From 9c80dd35e5bff23deb0142095f4bb1b408932d2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Mon, 20 Jun 2022 08:06:45 +0200 Subject: [PATCH 07/20] Both declarations must be the same --- src/Database/DBA.php | 4 ++-- tests/Util/Database/StaticDatabase.php | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Database/DBA.php b/src/Database/DBA.php index 4eff592f09..d7c0421b7a 100644 --- a/src/Database/DBA.php +++ b/src/Database/DBA.php @@ -42,7 +42,7 @@ class DBA */ const NULL_DATETIME = '0001-01-01 00:00:00'; - public static function connect() + public static function connect(): bool { return DI::dba()->connect(); } @@ -58,7 +58,7 @@ class DBA /** * Perform a reconnect of an existing database connection */ - public static function reconnect() + public static function reconnect(): bool { return DI::dba()->reconnect(); } diff --git a/tests/Util/Database/StaticDatabase.php b/tests/Util/Database/StaticDatabase.php index a6b2575f59..0782a9745f 100644 --- a/tests/Util/Database/StaticDatabase.php +++ b/tests/Util/Database/StaticDatabase.php @@ -47,7 +47,7 @@ class StaticDatabase extends Database * * @return bool|void */ - public function connect() + public function connect(): bool { if (!is_null($this->connection) && $this->connected()) { return true; @@ -81,7 +81,7 @@ class StaticDatabase extends Database } /** Mock for locking tables */ - public function lock($table) + public function lock($table): bool { if ($this->_locked) { return false; @@ -94,7 +94,7 @@ class StaticDatabase extends Database } /** Mock for unlocking tables */ - public function unlock() + public function unlock(): bool { // See here: https://dev.mysql.com/doc/refman/5.7/en/lock-tables-and-transactions.html $this->performCommit(); @@ -110,7 +110,7 @@ class StaticDatabase extends Database * * @return bool Was the command executed successfully? */ - public function commit() + public function commit(): bool { if (!$this->performCommit()) { return false; From cc750d743b9008daf28b1088fbc560a4f7cc2dc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Mon, 20 Jun 2022 08:12:09 +0200 Subject: [PATCH 08/20] Changes: - some methods now need to return bool to be compatible - added some missing type-hints --- tests/Util/Database/ExtendedPDO.php | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/tests/Util/Database/ExtendedPDO.php b/tests/Util/Database/ExtendedPDO.php index b127e312ca..b36c126d5b 100644 --- a/tests/Util/Database/ExtendedPDO.php +++ b/tests/Util/Database/ExtendedPDO.php @@ -33,7 +33,7 @@ class ExtendedPDO extends PDO /** * @var array Database drivers that support SAVEPOINT * statements. */ - protected static $_supportedDrivers = ["pgsql", "mysql"]; + protected static $_supportedDrivers = ['pgsql', 'mysql']; /** * @var int the current transaction depth @@ -80,9 +80,9 @@ class ExtendedPDO extends PDO /** * Commit current transaction * - * @return bool|void + * @return bool */ - public function commit() + public function commit(): bool { // We don't want to "really" commit something, so skip the most outer hierarchy if ($this->_transactionDepth <= 1 && $this->hasSavepoint()) { @@ -92,28 +92,29 @@ class ExtendedPDO extends PDO $this->_transactionDepth--; - $this->exec("RELEASE SAVEPOINT LEVEL{$this->_transactionDepth}"); + return $this->exec("RELEASE SAVEPOINT LEVEL{$this->_transactionDepth}"); } /** * Rollback current transaction, * * @throws PDOException if there is no transaction started - * @return bool|void + * @return bool Whether rollback was successful */ - public function rollBack() + public function rollback(): bool { $this->_transactionDepth--; - if($this->_transactionDepth <= 0 || !$this->hasSavepoint()) { + if ($this->_transactionDepth <= 0 || !$this->hasSavepoint()) { $this->_transactionDepth = 0; try { - parent::rollBack(); + return parent::rollBack(); } catch (PDOException $e) { // this shouldn't happen, but it does ... } } else { - $this->exec("ROLLBACK TO SAVEPOINT LEVEL{$this->_transactionDepth}"); + return $this->exec("ROLLBACK TO SAVEPOINT LEVEL{$this->_transactionDepth}"); } + return false; } } From 6743de63f5d0d276d6ee6bfa0d065b7f289f48ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Mon, 20 Jun 2022 08:18:53 +0200 Subject: [PATCH 09/20] Changed: - DBA::exists() should only be used for checking if records exists. - if you want to check if a table exists, please ALWAYS use DBStructure::existsTable() instead --- src/Database/DBA.php | 4 ++-- tests/src/Database/DBATest.php | 3 --- tests/src/Database/DBStructureTest.php | 1 - 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/Database/DBA.php b/src/Database/DBA.php index d7c0421b7a..e519a63cd6 100644 --- a/src/Database/DBA.php +++ b/src/Database/DBA.php @@ -218,8 +218,8 @@ class DBA /** * Check if data exists * - * @param string|array $table Table name or array [schema => table] - * @param array $condition array of fields for condition + * @param string|array $table Table name or array [schema => table] + * @param array $condition array of fields for condition * @return boolean Are there rows for that condition? * @throws \Exception */ diff --git a/tests/src/Database/DBATest.php b/tests/src/Database/DBATest.php index c92133ac54..febe9c8c62 100644 --- a/tests/src/Database/DBATest.php +++ b/tests/src/Database/DBATest.php @@ -55,9 +55,6 @@ class DBATest extends DatabaseTest self::assertTrue(DBA::exists('config', [])); self::assertFalse(DBA::exists('notable', [])); - self::assertTrue(DBA::exists('config', null)); - self::assertFalse(DBA::exists('notable', null)); - self::assertTrue(DBA::exists('config', ['k' => 'hostname'])); self::assertFalse(DBA::exists('config', ['k' => 'nonsense'])); } diff --git a/tests/src/Database/DBStructureTest.php b/tests/src/Database/DBStructureTest.php index 638a98ed7a..613e2f7832 100644 --- a/tests/src/Database/DBStructureTest.php +++ b/tests/src/Database/DBStructureTest.php @@ -45,7 +45,6 @@ class DBStructureTest extends DatabaseTest */ public function testExists() { self::assertTrue(DBStructure::existsTable('config')); - self::assertFalse(DBStructure::existsTable('notatable')); self::assertTrue(DBStructure::existsColumn('config', ['k'])); From e5cc7a5ab1229ff341a64f02d883b6ee5159b1ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Mon, 20 Jun 2022 08:25:17 +0200 Subject: [PATCH 10/20] Fixes: - Strings::isHex() should not be misused for checking on NULL --- mod/photos.php | 6 +++--- tests/src/Util/StringsTest.php | 10 +++------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/mod/photos.php b/mod/photos.php index 0e2299770a..977cebf20d 100644 --- a/mod/photos.php +++ b/mod/photos.php @@ -187,7 +187,7 @@ function photos_post(App $a) } if (DI::args()->getArgc() > 3 && DI::args()->getArgv()[2] === 'album') { - if (!Strings::isHex(DI::args()->getArgv()[3])) { + if (!Strings::isHex(DI::args()->getArgv()[3] ?? '')) { DI::baseUrl()->redirect('photos/' . $user['nickname'] . '/album'); } $album = hex2bin(DI::args()->getArgv()[3]); @@ -892,7 +892,7 @@ function photos_content(App $a) return; } - $selname = Strings::isHex($datum) ? hex2bin($datum) : ''; + $selname = (!is_null($datum) && Strings::isHex($datum)) ? hex2bin($datum) : ''; $albumselect = ''; @@ -954,7 +954,7 @@ function photos_content(App $a) // Display a single photo album if ($datatype === 'album') { // if $datum is not a valid hex, redirect to the default page - if (!Strings::isHex($datum)) { + if (is_null($datum) || !Strings::isHex($datum)) { DI::baseUrl()->redirect('photos/' . $user['nickname']. '/album'); } $album = hex2bin($datum); diff --git a/tests/src/Util/StringsTest.php b/tests/src/Util/StringsTest.php index 6f0fc32fe3..67a8aa259c 100644 --- a/tests/src/Util/StringsTest.php +++ b/tests/src/Util/StringsTest.php @@ -113,22 +113,18 @@ class StringsTest extends TestCase 'input' => '', 'valid' => false, ], - 'nullHex' => [ - 'input' => null, - 'valid' => false, - ], ]; } /** * Tests if the string is a valid hexadecimal value * - * @param string|null $input - * @param bool $valid + * @param string $input + * @param bool $valid * * @dataProvider dataIsHex */ - public function testIsHex(string $input = null, bool $valid = false) + public function testIsHex(string $input = '', bool $valid = false) { self::assertEquals($valid, Strings::isHex($input)); } From e96a5482865fad8e76f7bf366816465027ebf499 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Mon, 20 Jun 2022 18:55:17 +0200 Subject: [PATCH 11/20] Changes: - dumped default value for $input - added unknown 'platform' which prevents an "Undefined index: platform in /var/www/.../src/Model/GServer.php on line 940" error --- src/Model/GServer.php | 7 +++++-- tests/src/Util/StringsTest.php | 6 +++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/Model/GServer.php b/src/Model/GServer.php index e5c1bb3b69..f6c5e14af2 100644 --- a/src/Model/GServer.php +++ b/src/Model/GServer.php @@ -918,8 +918,11 @@ class GServer return []; } - $server = ['detection-method' => self::DETECT_NODEINFO_2, - 'register_policy' => Register::CLOSED]; + $server = [ + 'detection-method' => self::DETECT_NODEINFO_2, + 'register_policy' => Register::CLOSED, + 'platform' => 'unknown', + ]; if (!empty($nodeinfo['openRegistrations'])) { $server['register_policy'] = Register::OPEN; diff --git a/tests/src/Util/StringsTest.php b/tests/src/Util/StringsTest.php index 67a8aa259c..830f97b2f2 100644 --- a/tests/src/Util/StringsTest.php +++ b/tests/src/Util/StringsTest.php @@ -119,12 +119,12 @@ class StringsTest extends TestCase /** * Tests if the string is a valid hexadecimal value * - * @param string $input - * @param bool $valid + * @param string $input Input string + * @param bool $valid Whether testing on valid or invalid * * @dataProvider dataIsHex */ - public function testIsHex(string $input = '', bool $valid = false) + public function testIsHex(string $input, bool $valid = false) { self::assertEquals($valid, Strings::isHex($input)); } From 4fb03cf1632dd41a91cbfdf70207fd253fb7f9d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Mon, 20 Jun 2022 19:04:01 +0200 Subject: [PATCH 12/20] Changes: - fixed a null value handled over to Friendica\Model\APContact::getByURL() - added missing type-hints --- src/Model/Item.php | 10 +++++----- src/Protocol/ActivityPub/Receiver.php | 9 ++++----- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/Model/Item.php b/src/Model/Item.php index 1e3326e285..5144e73e0e 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -1841,13 +1841,13 @@ class Item $parsed = parse_url($uri); // Remove the scheme to make sure that "https" and "http" doesn't make a difference - unset($parsed["scheme"]); + unset($parsed['scheme']); // Glue it together to be able to make a hash from it - $host_id = implode("/", $parsed); + $host_id = implode('/', $parsed); // Use a mixture of several hashes to provide some GUID like experience - return hash("crc32", $host) . '-'. hash('joaat', $host_id) . '-'. hash('fnv164', $host_id); + return hash('crc32', $host) . '-'. hash('joaat', $host_id) . '-'. hash('fnv164', $host_id); } /** @@ -1859,9 +1859,9 @@ class Item * @return string * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public static function newURI($uid, $guid = "") + public static function newURI(int $uid, string $guid = ''): string { - if ($guid == "") { + if ($guid == '') { $guid = System::createUUID(); } diff --git a/src/Protocol/ActivityPub/Receiver.php b/src/Protocol/ActivityPub/Receiver.php index be1c3b7e7c..29f6b5bec7 100644 --- a/src/Protocol/ActivityPub/Receiver.php +++ b/src/Protocol/ActivityPub/Receiver.php @@ -80,13 +80,12 @@ class Receiver /** * Checks incoming message from the inbox * - * @param $body - * @param $header + * @param string $body Body string + * @param array $header Header lines * @param integer $uid User ID * @throws \Exception - * @todo Find type for $body/$header */ - public static function processInbox($body, $header, int $uid) + public static function processInbox(string $body, array $header, int $uid) { $activity = json_decode($body, true); if (empty($activity)) { @@ -98,7 +97,7 @@ class Receiver $actor = JsonLD::fetchElement($ldactivity, 'as:actor', '@id'); - $apcontact = APContact::getByURL($actor); + $apcontact = APContact::getByURL($actor ?? ''); if (empty($apcontact)) { Logger::notice('Unable to retrieve AP contact for actor - message is discarded', ['actor' => $actor]); return; From feb87e8dc32f922b9c9233c7facc91bad0492dbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Mon, 20 Jun 2022 19:08:23 +0200 Subject: [PATCH 13/20] Changes: - let's start throwing exceptions on e.g. invalid arguments instead of returning FALSE --- src/Database/Database.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 590c3d83d0..e0471b3635 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -26,6 +26,7 @@ use Friendica\Core\System; use Friendica\Network\HTTPException\ServiceUnavailableException; use Friendica\Util\DateTimeFormat; use Friendica\Util\Profiler; +use InvalidArgumentException; use mysqli; use mysqli_result; use mysqli_stmt; @@ -1512,7 +1513,7 @@ class Database * @param array $condition Array of fields for condition * @param array $params Array of several parameters * - * @return int + * @return int Count of rows * * Example: * $table = "post"; @@ -1524,11 +1525,10 @@ class Database * $count = DBA::count($table, $condition); * @throws \Exception */ - public function count($table, array $condition = [], array $params = []) + public function count($table, array $condition = [], array $params = []): int { - // @TODO Can we dump this to have ": int" as returned type-hint? if (empty($table)) { - return false; + throw new InvalidArgumentException('Parameter "table" cannot be empty.'); } $table_string = DBA::buildTableString($table); From 0c12e947dd9f5f1db26b4c9eed43319ab364581b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Mon, 20 Jun 2022 20:01:25 +0200 Subject: [PATCH 14/20] Changes: - null was 2nd argument's value before, an empty string is basically the same here --- src/Protocol/Feed.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Protocol/Feed.php b/src/Protocol/Feed.php index e884c8b205..15c522040d 100644 --- a/src/Protocol/Feed.php +++ b/src/Protocol/Feed.php @@ -312,7 +312,7 @@ class Feed $item['uri'] = $guid; // Don't use the GUID value directly but instead use it as a basis for the GUID - $item['guid'] = Item::guidFromUri($guid, parse_url($guid, PHP_URL_HOST) ?? parse_url($item['plink'], PHP_URL_HOST)); + $item['guid'] = Item::guidFromUri($guid, parse_url($guid, PHP_URL_HOST) ?? parse_url($item['plink'], PHP_URL_HOST) ?? ''); } if (empty($item['uri'])) { From 752953e472dec0548cdf5291843e6b64e9d9f083 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Mon, 20 Jun 2022 20:59:08 +0200 Subject: [PATCH 15/20] Changes: - as @MrPetovan pointed out, $actor can be NULL earlier and used later as NULL - added some missing type-hints - added missing documentation - the added @TODO points out to avoid true|false|null for a boolean --- src/Model/APContact.php | 1 + src/Protocol/ActivityPub/Receiver.php | 4 ++-- src/Protocol/ActivityPub/Transmitter.php | 28 ++++++++++++++++++------ 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/Model/APContact.php b/src/Model/APContact.php index 5c1d962a84..9269ee7903 100644 --- a/src/Model/APContact.php +++ b/src/Model/APContact.php @@ -116,6 +116,7 @@ class APContact * @return array profile array * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException + * @todo Rewrite parameter $update to avoid true|false|null (boolean is binary, null adds a third case) */ public static function getByURL(string $url, $update = null): array { diff --git a/src/Protocol/ActivityPub/Receiver.php b/src/Protocol/ActivityPub/Receiver.php index 29f6b5bec7..e4f938c01d 100644 --- a/src/Protocol/ActivityPub/Receiver.php +++ b/src/Protocol/ActivityPub/Receiver.php @@ -95,9 +95,9 @@ class Receiver $ldactivity = JsonLD::compact($activity); - $actor = JsonLD::fetchElement($ldactivity, 'as:actor', '@id'); + $actor = JsonLD::fetchElement($ldactivity, 'as:actor', '@id') ?? ''; + $apcontact = APContact::getByURL($actor); - $apcontact = APContact::getByURL($actor ?? ''); if (empty($apcontact)) { Logger::notice('Unable to retrieve AP contact for actor - message is discarded', ['actor' => $actor]); return; diff --git a/src/Protocol/ActivityPub/Transmitter.php b/src/Protocol/ActivityPub/Transmitter.php index 8483d5d1dd..52c140fe13 100644 --- a/src/Protocol/ActivityPub/Transmitter.php +++ b/src/Protocol/ActivityPub/Transmitter.php @@ -2201,13 +2201,14 @@ class Transmitter * Transmits a message that we don't want to follow this contact anymore * * @param string $target Target profile + * @param integer $cid Contact id * @param integer $uid User ID + * @return bool success * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException * @throws \Exception - * @return bool success */ - public static function sendContactUndo($target, $cid, $uid) + public static function sendContactUndo(string $target, int $cid, int $uid) { $profile = APContact::getByURL($target); if (empty($profile['inbox'])) { @@ -2223,15 +2224,20 @@ class Transmitter $id = DI::baseUrl() . '/activity/' . System::createGUID(); $owner = User::getOwnerDataById($uid); - $data = ['@context' => ActivityPub::CONTEXT, + $data = [ + '@context' => ActivityPub::CONTEXT, 'id' => $id, 'type' => 'Undo', 'actor' => $owner['url'], - 'object' => ['id' => $object_id, 'type' => 'Follow', + 'object' => [ + 'id' => $object_id, + 'type' => 'Follow', 'actor' => $owner['url'], - 'object' => $profile['url']], + 'object' => $profile['url'] + ], 'instrument' => self::getService(), - 'to' => [$profile['url']]]; + 'to' => [$profile['url']] + ]; Logger::info('Sending undo to ' . $target . ' for user ' . $uid . ' with id ' . $id); @@ -2239,7 +2245,15 @@ class Transmitter return HTTPSignature::transmit($signed, $profile['inbox'], $uid); } - private static function prependMentions($body, int $uriid, string $authorLink) + /** + * Prepends mentions (@) to $body variable + * + * @param string $body HTML code + * @param int $uriid URI id + * @param string $authorLink Author link + * @return string HTML code with prepended mentions + */ + private static function prependMentions(string $body, int $uriid, string $authorLink): string { $mentions = []; From ea22e88896193a337535121187c94942b45af08a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Mon, 20 Jun 2022 21:02:18 +0200 Subject: [PATCH 16/20] Added documentation --- src/Protocol/ActivityPub/Processor.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Protocol/ActivityPub/Processor.php b/src/Protocol/ActivityPub/Processor.php index eab23222c9..a044b7368f 100644 --- a/src/Protocol/ActivityPub/Processor.php +++ b/src/Protocol/ActivityPub/Processor.php @@ -1245,6 +1245,7 @@ class Processor * perform a "follow" request * * @param array $activity + * @return void * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException */ From fbae0b8bcfac80d494d81b656f62d04216ed0a04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Mon, 20 Jun 2022 21:21:32 +0200 Subject: [PATCH 17/20] Changes: - renamed ItemArrayFromMail() to getItemArrayFromMail() to follow naming-convetion - added missing type-hints - added missing documentation --- src/Factory/Api/Mastodon/Status.php | 2 +- src/Protocol/ActivityPub/Processor.php | 7 +- src/Protocol/ActivityPub/Receiver.php | 1 + src/Protocol/ActivityPub/Transmitter.php | 233 +++++++++++------------ src/Worker/Notifier.php | 2 +- 5 files changed, 118 insertions(+), 127 deletions(-) diff --git a/src/Factory/Api/Mastodon/Status.php b/src/Factory/Api/Mastodon/Status.php index 5d58944150..0fc9281dbe 100644 --- a/src/Factory/Api/Mastodon/Status.php +++ b/src/Factory/Api/Mastodon/Status.php @@ -190,7 +190,7 @@ class Status extends BaseFactory */ public function createFromMailId(int $id): \Friendica\Object\Api\Mastodon\Status { - $item = ActivityPub\Transmitter::ItemArrayFromMail($id, true); + $item = ActivityPub\Transmitter::getItemArrayFromMail($id, true); if (empty($item)) { $this->mstdnErrorFactory->RecordNotFound(); } diff --git a/src/Protocol/ActivityPub/Processor.php b/src/Protocol/ActivityPub/Processor.php index a044b7368f..0b58427c34 100644 --- a/src/Protocol/ActivityPub/Processor.php +++ b/src/Protocol/ActivityPub/Processor.php @@ -1297,8 +1297,8 @@ class Processor /** * Transmit pending events to the new follower * - * @param integer $cid - * @param integer $uid + * @param integer $cid Contact id + * @param integer $uid User id * @return void */ private static function transmitPendingEvents(int $cid, int $uid) @@ -1341,6 +1341,7 @@ class Processor * Delete the given profile * * @param array $activity + * @return void * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ public static function deletePerson(array $activity) @@ -1368,6 +1369,7 @@ class Processor * Blocks the user by the contact * * @param array $activity + * @return void * @throws \Exception */ public static function blockAccount(array $activity) @@ -1391,6 +1393,7 @@ class Processor * Unblocks the user by the contact * * @param array $activity + * @return void * @throws \Exception */ public static function unblockAccount(array $activity) diff --git a/src/Protocol/ActivityPub/Receiver.php b/src/Protocol/ActivityPub/Receiver.php index e4f938c01d..38b6c4fc54 100644 --- a/src/Protocol/ActivityPub/Receiver.php +++ b/src/Protocol/ActivityPub/Receiver.php @@ -83,6 +83,7 @@ class Receiver * @param string $body Body string * @param array $header Header lines * @param integer $uid User ID + * @return void * @throws \Exception */ public static function processInbox(string $body, array $header, int $uid) diff --git a/src/Protocol/ActivityPub/Transmitter.php b/src/Protocol/ActivityPub/Transmitter.php index 52c140fe13..b196f196a1 100644 --- a/src/Protocol/ActivityPub/Transmitter.php +++ b/src/Protocol/ActivityPub/Transmitter.php @@ -103,12 +103,12 @@ class Transmitter } /** - * Subscribe to a relay + * Subscribe to a relay and updates contact on success * * @param string $url Subscribe actor url * @return bool success */ - public static function sendRelayFollow(string $url) + public static function sendRelayFollow(string $url): bool { $contact = Contact::getByURL($url); if (empty($contact)) { @@ -125,13 +125,13 @@ class Transmitter } /** - * Unsubscribe from a relay + * Unsubscribe from a relay and updates contact on success or forced * * @param string $url Subscribe actor url * @param bool $force Set the relay status as non follower even if unsubscribe hadn't worked * @return bool success */ - public static function sendRelayUndoFollow(string $url, bool $force = false) + public static function sendRelayUndoFollow(string $url, bool $force = false): bool { $contact = Contact::getByURL($url); if (empty($contact)) { @@ -139,6 +139,7 @@ class Transmitter } $success = self::sendContactUndo($url, $contact['id'], 0); + if ($success || $force) { Contact::update(['rel' => Contact::NOTHING], ['id' => $contact['id']]); } @@ -155,11 +156,10 @@ class Transmitter * @param integer $page Page number * @param string $requester URL of the requester * @param boolean $nocache Wether to bypass caching - * * @return array of owners * @throws \Exception */ - public static function getContacts(array $owner, array $rel, string $module, int $page = null, string $requester = null, $nocache = false) + public static function getContacts(array $owner, array $rel, string $module, int $page = null, string $requester = null, bool $nocache = false): array { if (empty($page)) { $cachekey = self::CACHEKEY_CONTACTS . $module . ':'. $owner['uid']; @@ -246,12 +246,11 @@ class Transmitter * @param integer $page Page number * @param string $requester URL of requesting account * @param boolean $nocache Wether to bypass caching - * * @return array of posts * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException */ - public static function getOutbox(array $owner, int $page = null, string $requester = '', $nocache = false) + public static function getOutbox(array $owner, int $page = null, string $requester = '', bool $nocache = false): array { if (empty($page)) { $cachekey = self::CACHEKEY_OUTBOX . $owner['uid']; @@ -274,15 +273,16 @@ class Transmitter } } - $condition = array_merge($condition, - ['uid' => $owner['uid'], + $condition = array_merge($condition, [ + 'uid' => $owner['uid'], 'author-id' => Contact::getIdForURL($owner['url'], 0, false), 'gravity' => [GRAVITY_PARENT, GRAVITY_COMMENT], 'network' => Protocol::FEDERATED, 'parent-network' => Protocol::FEDERATED, 'origin' => true, 'deleted' => false, - 'visible' => true]); + 'visible' => true + ]); $count = Post::count($condition); @@ -340,7 +340,7 @@ class Transmitter * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException */ - public static function getFeatured(array $owner, int $page = null, $nocache = false) + public static function getFeatured(array $owner, int $page = null, bool $nocache = false): array { if (empty($page)) { $cachekey = self::CACHEKEY_FEATURED . $owner['uid']; @@ -355,8 +355,8 @@ class Transmitter $condition = ["`uri-id` IN (SELECT `uri-id` FROM `collection-view` WHERE `cid` = ? AND `type` = ?)", $owner_cid, Post\Collection::FEATURED]; - $condition = DBA::mergeConditions($condition, - ['uid' => $owner['uid'], + $condition = DBA::mergeConditions($condition, [ + 'uid' => $owner['uid'], 'author-id' => $owner_cid, 'private' => [Item::PUBLIC, Item::UNLISTED], 'gravity' => [GRAVITY_PARENT, GRAVITY_COMMENT], @@ -364,7 +364,8 @@ class Transmitter 'parent-network' => Protocol::FEDERATED, 'origin' => true, 'deleted' => false, - 'visible' => true]); + 'visible' => true + ]); $count = Post::count($condition); @@ -418,11 +419,13 @@ class Transmitter * * @return array with service data */ - private static function getService() + private static function getService(): array { - return ['type' => 'Service', + return [ + 'type' => 'Service', 'name' => FRIENDICA_PLATFORM . " '" . FRIENDICA_CODENAME . "' " . FRIENDICA_VERSION . '-' . DB_UPDATE_VERSION, - 'url' => DI::baseUrl()->get()]; + 'url' => DI::baseUrl()->get() + ]; } /** @@ -537,7 +540,7 @@ class Transmitter * @return array * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public static function getDeletedUser($username) + public static function getDeletedUser(string $username): array { return [ '@context' => ActivityPub::CONTEXT, @@ -559,7 +562,7 @@ class Transmitter * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException */ - private static function fetchPermissionBlockFromThreadParent(array $item, bool $is_forum_thread) + private static function fetchPermissionBlockFromThreadParent(array $item, bool $is_forum_thread): array { if (empty($item['thr-parent-id'])) { return []; @@ -606,7 +609,7 @@ class Transmitter * @param integer $item_id * @return boolean "true" if the post is from ActivityPub */ - private static function isAPPost(int $item_id) + private static function isAPPost(int $item_id): bool { if (empty($item_id)) { return false; @@ -626,7 +629,7 @@ class Transmitter * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException */ - private static function createPermissionBlockForItem($item, $blindcopy, $last_id = 0) + private static function createPermissionBlockForItem(array $item, bool $blindcopy, int $last_id = 0): array { if ($last_id == 0) { $last_id = $item['id']; @@ -858,10 +861,9 @@ class Transmitter * Check if an inbox is archived * * @param string $url Inbox url - * * @return boolean "true" if inbox is archived */ - public static function archivedInbox($url) + public static function archivedInbox(string $url): bool { return DBA::exists('inbox-status', ['url' => $url, 'archive' => true]); } @@ -869,12 +871,12 @@ class Transmitter /** * Check if a given contact should be delivered via AP * - * @param array $contact - * @param array $networks - * @return bool + * @param array $contact Contact array + * @param array $networks Array with networks + * @return bool Whether the used protocol matches ACTIVITYPUB * @throws Exception */ - private static function isAPContact(array $contact, array $networks) + private static function isAPContact(array $contact, array $networks): bool { if (in_array($contact['network'], $networks) || ($contact['protocol'] == Protocol::ACTIVITYPUB)) { return true; @@ -889,12 +891,11 @@ class Transmitter * @param integer $uid User ID * @param boolean $personal fetch personal inboxes * @param boolean $all_ap Retrieve all AP enabled inboxes - * * @return array of follower inboxes * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException */ - public static function fetchTargetInboxesforUser($uid, $personal = false, bool $all_ap = false) + public static function fetchTargetInboxesforUser(int $uid, bool $personal = false, bool $all_ap = false): array { $inboxes = []; @@ -963,7 +964,7 @@ class Transmitter * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException */ - public static function fetchTargetInboxes($item, $uid, $personal = false, $last_id = 0) + public static function fetchTargetInboxes(array $item, int $uid, bool $personal = false, int $last_id = 0): array { $permissions = self::createPermissionBlockForItem($item, true, $last_id); if (empty($permissions)) { @@ -1022,12 +1023,11 @@ class Transmitter /** * Creates an array in the structure of the item table for a given mail id * - * @param integer $mail_id - * + * @param integer $mail_id Mail id * @return array * @throws \Exception */ - public static function ItemArrayFromMail($mail_id, $use_title = false) + public static function getItemArrayFromMail(int $mail_id, bool $use_title = false): array { $mail = DBA::selectFirst('mail', [], ['id' => $mail_id]); if (!DBA::isResult($mail)) { @@ -1079,9 +1079,9 @@ class Transmitter * @return array of activity * @throws \Exception */ - public static function createActivityFromMail($mail_id, $object_mode = false) + public static function createActivityFromMail(int $mail_id, bool $object_mode = false): array { - $mail = self::ItemArrayFromMail($mail_id); + $mail = self::getItemArrayFromMail($mail_id); if (empty($mail)) { return []; } @@ -1133,18 +1133,17 @@ class Transmitter /** * Returns the activity type of a given item * - * @param array $item - * + * @param array $item Item array * @return string with activity type * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException */ - private static function getTypeOfItem($item) + private static function getTypeOfItem(array $item): string { $reshared = false; // Only check for a reshare, if it is a real reshare and no quoted reshare - if (strpos($item['body'], "[share") === 0) { + if (strpos($item['body'], '[share') === 0) { $announce = self::getAnnounceArray($item); $reshared = !empty($announce); } @@ -1183,13 +1182,12 @@ class Transmitter /** * Creates the activity or fetches it from the cache * - * @param integer $item_id + * @param integer $item_id Item id * @param boolean $force Force new cache entry - * * @return array with the activity * @throws \Exception */ - public static function createCachedActivityFromItem($item_id, $force = false) + public static function createCachedActivityFromItem(int $item_id, bool $force = false): array { $cachekey = 'APDelivery:createActivity:' . $item_id; @@ -1211,7 +1209,6 @@ class Transmitter * * @param integer $item_id * @param boolean $object_mode Is the activity item is used inside another object? - * * @return false|array * @throws \Exception */ @@ -1335,11 +1332,10 @@ class Transmitter /** * Creates a location entry for a given item array * - * @param array $item - * + * @param array $item Item array * @return array with location array */ - private static function createLocation($item) + private static function createLocation(array $item): array { $location = ['type' => 'Place']; @@ -1369,12 +1365,11 @@ class Transmitter /** * Returns a tag array for a given item array * - * @param array $item - * + * @param array $item Item array * @return array of tags * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - private static function createTagList($item) + private static function createTagList(array $item): array { $tags = []; @@ -1416,7 +1411,7 @@ class Transmitter * @return array with attachment data * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - private static function createAttachmentList($item, $type) + private static function createAttachmentList(array $item, string $type): array { $attachments = []; @@ -1468,7 +1463,7 @@ class Transmitter * @return string Replaced mention * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - private static function mentionAddrCallback($match) + private static function mentionAddrCallback(array $match): string { if (empty($match[1])) { return ''; @@ -1485,11 +1480,10 @@ class Transmitter /** * Remove image elements since they are added as attachment * - * @param string $body - * + * @param string $body HTML code * @return string with removed images */ - private static function removePictures($body) + private static function removePictures(string $body): string { // Simplify image codes $body = preg_replace("/\[img\=([0-9]*)x([0-9]*)\](.*?)\[\/img\]/ism", '[img]$3[/img]', $body); @@ -1518,12 +1512,11 @@ class Transmitter /** * Fetches the "context" value for a givem item array from the "conversation" table * - * @param array $item - * + * @param array $item Item array * @return string with context url * @throws \Exception */ - private static function fetchContextURLForItem($item) + private static function fetchContextURLForItem(array $item): string { $conversation = DBA::selectFirst('conversation', ['conversation-href', 'conversation-uri'], ['item-uri' => $item['parent-uri']]); if (DBA::isResult($conversation) && !empty($conversation['conversation-href'])) { @@ -1539,12 +1532,11 @@ class Transmitter /** * Returns if the post contains sensitive content ("nsfw") * - * @param integer $uri_id - * - * @return boolean + * @param integer $uri_id URI id + * @return boolean Whether URI id was found * @throws \Exception */ - private static function isSensitive($uri_id) + private static function isSensitive(int $uri_id): bool { return DBA::exists('tag-view', ['uri-id' => $uri_id, 'name' => 'nsfw', 'type' => Tag::HASHTAG]); } @@ -1552,12 +1544,11 @@ class Transmitter /** * Creates event data * - * @param array $item - * + * @param array $item Item array * @return array with the event data * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - private static function createEvent($item) + private static function createEvent(array $item): array { $event = []; $event['name'] = $item['event-summary']; @@ -1583,12 +1574,11 @@ class Transmitter * Creates a note/article object array * * @param array $item - * * @return array with the object data * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException */ - public static function createNote($item) + public static function createNote(array $item): array { if (empty($item)) { return []; @@ -1739,10 +1729,9 @@ class Transmitter * Fetches the language from the post, the user or the system. * * @param array $item - * * @return string language string */ - private static function getLanguage(array $item) + private static function getLanguage(array $item): string { // Try to fetch the language from the post itself if (!empty($item['language'])) { @@ -1767,74 +1756,71 @@ class Transmitter /** * Creates an an "add tag" entry * - * @param array $item - * @param array $data activity data - * + * @param array $item Item array + * @param array $activity activity data * @return array with activity data for adding tags * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException */ - private static function createAddTag($item, $data) + private static function createAddTag(array $item, array $activity): array { $object = XML::parseString($item['object']); - $target = XML::parseString($item["target"]); + $target = XML::parseString($item['target']); - $data['diaspora:guid'] = $item['guid']; - $data['actor'] = $item['author-link']; - $data['target'] = (string)$target->id; - $data['summary'] = BBCode::toPlaintext($item['body']); - $data['object'] = ['id' => (string)$object->id, 'type' => 'tag', 'name' => (string)$object->title, 'content' => (string)$object->content]; + $activity['diaspora:guid'] = $item['guid']; + $activity['actor'] = $item['author-link']; + $activity['target'] = (string)$target->id; + $activity['summary'] = BBCode::toPlaintext($item['body']); + $activity['object'] = ['id' => (string)$object->id, 'type' => 'tag', 'name' => (string)$object->title, 'content' => (string)$object->content]; - return $data; + return $activity; } /** * Creates an announce object entry * - * @param array $item - * @param array $data activity data - * + * @param array $item Item array + * @param array $activity activity data * @return array with activity data * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException */ - private static function createAnnounce($item, $data) + private static function createAnnounce(array $item, array $activity): array { $orig_body = $item['body']; $announce = self::getAnnounceArray($item); if (empty($announce)) { - $data['type'] = 'Create'; - $data['object'] = self::createNote($item); - return $data; + $activity['type'] = 'Create'; + $activity['object'] = self::createNote($item); + return $activity; } if (empty($announce['comment'])) { // Pure announce, without a quote - $data['type'] = 'Announce'; - $data['object'] = $announce['object']['uri']; - return $data; + $activity['type'] = 'Announce'; + $activity['object'] = $announce['object']['uri']; + return $activity; } // Quote - $data['type'] = 'Create'; + $activity['type'] = 'Create'; $item['body'] = $announce['comment'] . "\n" . $announce['object']['plink']; - $data['object'] = self::createNote($item); + $activity['object'] = self::createNote($item); /// @todo Finally descide how to implement this in AP. This is a possible way: - $data['object']['attachment'][] = self::createNote($announce['object']); + $activity['object']['attachment'][] = self::createNote($announce['object']); - $data['object']['source']['content'] = $orig_body; - return $data; + $activity['object']['source']['content'] = $orig_body; + return $activity; } /** * Return announce related data if the item is an annunce * * @param array $item - * - * @return array + * @return array Announcement array */ - public static function getAnnounceArray($item) + public static function getAnnounceArray(array $item): array { $reshared = Item::getShareArray($item); if (empty($reshared['guid'])) { @@ -1861,11 +1847,10 @@ class Transmitter /** * Checks if the provided item array is an announce * - * @param array $item - * - * @return boolean + * @param array $item Item array + * @return boolean Whether item is an announcement */ - public static function isAnnounce($item) + public static function isAnnounce(array $item): bool { if (!empty($item['verb']) && ($item['verb'] == Activity::ANNOUNCE)) { return true; @@ -1886,7 +1871,7 @@ class Transmitter * * @return bool|string activity id */ - public static function activityIDFromContact($cid) + public static function activityIDFromContact(int $cid) { $contact = DBA::selectFirst('contact', ['uid', 'id', 'created'], ['id' => $cid]); if (!DBA::isResult($contact)) { @@ -1904,17 +1889,17 @@ class Transmitter * @param integer $uid User ID * @param string $inbox Target inbox * @param integer $suggestion_id Suggestion ID - * * @return boolean was the transmission successful? * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public static function sendContactSuggestion($uid, $inbox, $suggestion_id) + public static function sendContactSuggestion(int $uid, string $inbox, int $suggestion_id): bool { $owner = User::getOwnerDataById($uid); $suggestion = DI::fsuggest()->selectOneById($suggestion_id); - $data = ['@context' => ActivityPub::CONTEXT, + $data = [ + '@context' => ActivityPub::CONTEXT, 'id' => DI::baseUrl() . '/activity/' . System::createGUID(), 'type' => 'Announce', 'actor' => $owner['url'], @@ -1922,7 +1907,8 @@ class Transmitter 'content' => $suggestion->note, 'instrument' => self::getService(), 'to' => [ActivityPub::PUBLIC_COLLECTION], - 'cc' => []]; + 'cc' => [] + ]; $signed = LDSignature::sign($data, $owner); @@ -1935,15 +1921,15 @@ class Transmitter * * @param integer $uid User ID * @param string $inbox Target inbox - * * @return boolean was the transmission successful? * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public static function sendProfileRelocation($uid, $inbox) + public static function sendProfileRelocation(int $uid, string $inbox): bool { $owner = User::getOwnerDataById($uid); - $data = ['@context' => ActivityPub::CONTEXT, + $data = [ + '@context' => ActivityPub::CONTEXT, 'id' => DI::baseUrl() . '/activity/' . System::createGUID(), 'type' => 'dfrn:relocate', 'actor' => $owner['url'], @@ -1951,7 +1937,8 @@ class Transmitter 'published' => DateTimeFormat::utcNow(DateTimeFormat::ATOM), 'instrument' => self::getService(), 'to' => [ActivityPub::PUBLIC_COLLECTION], - 'cc' => []]; + 'cc' => [] + ]; $signed = LDSignature::sign($data, $owner); @@ -1964,11 +1951,10 @@ class Transmitter * * @param integer $uid User ID * @param string $inbox Target inbox - * * @return boolean was the transmission successful? * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public static function sendProfileDeletion($uid, $inbox) + public static function sendProfileDeletion(int $uid, string $inbox): bool { $owner = User::getOwnerDataById($uid); @@ -2003,7 +1989,6 @@ class Transmitter * * @param integer $uid User ID * @param string $inbox Target inbox - * * @return boolean was the transmission successful? * @throws HTTPException\InternalServerErrorException * @throws HTTPException\NotFoundException @@ -2036,17 +2021,18 @@ class Transmitter * @param string $activity Type name * @param string $target Target profile * @param integer $uid User ID + * @param string $id Activity-identifier * @return bool * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException * @throws \Exception */ - public static function sendActivity($activity, $target, $uid, $id = '') + public static function sendActivity(string $activity, string $target, int $uid, string $id = ''): bool { $profile = APContact::getByURL($target); if (empty($profile['inbox'])) { Logger::warning('No inbox found for target', ['target' => $target, 'profile' => $profile]); - return; + return false; } $owner = User::getOwnerDataById($uid); @@ -2081,12 +2067,12 @@ class Transmitter * @throws \ImagickException * @throws \Exception */ - public static function sendFollowObject($object, $target, $uid = 0) + public static function sendFollowObject(string $object, string $target, int $uid = 0): bool { $profile = APContact::getByURL($target); if (empty($profile['inbox'])) { Logger::warning('No inbox found for target', ['target' => $target, 'profile' => $profile]); - return; + return false; } if (empty($uid)) { @@ -2126,12 +2112,13 @@ class Transmitter * Transmit a message that the contact request had been accepted * * @param string $target Target profile - * @param $id + * @param integer $id Object id * @param integer $uid User ID + * @return void * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException */ - public static function sendContactAccept($target, $id, $uid) + public static function sendContactAccept(string $target, int $id, int $uid) { $profile = APContact::getByURL($target); if (empty($profile['inbox'])) { @@ -2163,13 +2150,13 @@ class Transmitter * Reject a contact request or terminates the contact relation * * @param string $target Target profile - * @param $id + * @param integer $id Object id * @param integer $uid User ID * @return bool Operation success * @throws HTTPException\InternalServerErrorException * @throws \ImagickException */ - public static function sendContactReject($target, $id, $uid): bool + public static function sendContactReject(string $target, int $id, int $uid): bool { $profile = APContact::getByURL($target); if (empty($profile['inbox'])) { @@ -2208,7 +2195,7 @@ class Transmitter * @throws \ImagickException * @throws \Exception */ - public static function sendContactUndo(string $target, int $cid, int $uid) + public static function sendContactUndo(string $target, int $cid, int $uid): bool { $profile = APContact::getByURL($target); if (empty($profile['inbox'])) { diff --git a/src/Worker/Notifier.php b/src/Worker/Notifier.php index 2039448e14..8c0801c66d 100644 --- a/src/Worker/Notifier.php +++ b/src/Worker/Notifier.php @@ -81,7 +81,7 @@ class Notifier $uid = $message['uid']; $recipients[] = $message['contact-id']; - $mail = ActivityPub\Transmitter::ItemArrayFromMail($target_id); + $mail = ActivityPub\Transmitter::getItemArrayFromMail($target_id); $inboxes = ActivityPub\Transmitter::fetchTargetInboxes($mail, $uid, true); foreach ($inboxes as $inbox => $receivers) { $ap_contacts = array_merge($ap_contacts, $receivers); From b6fa022a73b995ab2871b187df1a1853ae2a0628 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Mon, 20 Jun 2022 23:49:29 +0200 Subject: [PATCH 18/20] Changes: - added type-hints - added some documentation - marked some generic methods to be moved to Util\Strings class instead --- src/Util/XML.php | 115 ++++++++++++++++++++++++++++------------------- 1 file changed, 70 insertions(+), 45 deletions(-) diff --git a/src/Util/XML.php b/src/Util/XML.php index f00cb7894c..76947e9431 100644 --- a/src/Util/XML.php +++ b/src/Util/XML.php @@ -21,6 +21,9 @@ namespace Friendica\Util; +use DOMDocument; +use DOMElement; +use DOMNode; use DOMXPath; use Friendica\Core\Logger; use Friendica\Core\System; @@ -39,22 +42,21 @@ class XML * @param bool $remove_header Should the XML header be removed or not? * @param array $namespaces List of namespaces * @param bool $root interally used parameter. Mustn't be used from outside. - * - * @return string The created XML + * @return void */ - public static function fromArray($array, &$xml, $remove_header = false, $namespaces = [], $root = true) + public static function fromArray(array $array, &$xml, bool $remove_header = false, array $namespaces = [], bool $root = true) { if ($root) { foreach ($array as $key => $value) { foreach ($namespaces as $nskey => $nsvalue) { - $key .= " xmlns".($nskey == "" ? "":":").$nskey.'="'.$nsvalue.'"'; + $key .= ' xmlns' . ($nskey == '' ? '' : ':') . $nskey . '="' . $nsvalue . '"'; } if (is_array($value)) { - $root = new SimpleXMLElement("<".$key."/>"); + $root = new SimpleXMLElement('<' . $key . '/>'); self::fromArray($value, $root, $remove_header, $namespaces, false); } else { - $root = new SimpleXMLElement("<".$key.">".self::escape($value).""); + $root = new SimpleXMLElement('<' . $key . '>'.self::escape($value).''); } $dom = dom_import_simplexml($root)->ownerDocument; @@ -88,11 +90,11 @@ class XML continue; } - $element_parts = explode(":", $key); + $element_parts = explode(':', $key); if ((count($element_parts) > 1) && isset($namespaces[$element_parts[0]])) { $namespace = $namespaces[$element_parts[0]]; - } elseif (isset($namespaces[""])) { - $namespace = $namespaces[""]; + } elseif (isset($namespaces[''])) { + $namespace = $namespaces['']; } else { $namespace = null; } @@ -102,13 +104,13 @@ class XML $key = $element_parts[1]; } - if (substr($key, 0, 11) == "@attributes") { + if (substr($key, 0, 11) == '@attributes') { if (!isset($element) || !is_array($value)) { continue; } foreach ($value as $attr_key => $attr_value) { - $element_parts = explode(":", $attr_key); + $element_parts = explode(':', $attr_key); if ((count($element_parts) > 1) && isset($namespaces[$element_parts[0]])) { $namespace = $namespaces[$element_parts[0]]; } else { @@ -138,7 +140,7 @@ class XML * @param string $elementname Name of the XML element of the target * @return void */ - public static function copy(&$source, &$target, $elementname) + public static function copy(&$source, &$target, string $elementname) { if (count($source->children()) == 0) { $target->addChild($elementname, self::escape($source)); @@ -153,14 +155,14 @@ class XML /** * Create an XML element * - * @param \DOMDocument $doc XML root + * @param DOMDocument $doc XML root * @param string $element XML element name * @param string $value XML value * @param array $attributes array containing the attributes * * @return \DOMElement XML element object */ - public static function createElement(\DOMDocument $doc, $element, $value = "", $attributes = []) + public static function createElement(DOMDocument $doc, string $element, string $value = '', array $attributes = []): DOMElement { $element = $doc->createElement($element, self::escape($value)); @@ -175,14 +177,14 @@ class XML /** * Create an XML and append it to the parent object * - * @param \DOMDocument $doc XML root + * @param DOMDocument $doc XML root * @param object $parent parent object * @param string $element XML element name * @param string $value XML value * @param array $attributes array containing the attributes * @return void */ - public static function addElement(\DOMDocument $doc, $parent, $element, $value = "", $attributes = []) + public static function addElement(DOMDocument $doc, $parent, string $element, string $value = '', array $attributes = []) { $element = self::createElement($doc, $element, $value, $attributes); $parent->appendChild($element); @@ -198,7 +200,7 @@ class XML * * @return array | string The array from the xml element or the string */ - public static function elementToArray($xml_element, &$recursion_depth = 0) + public static function elementToArray($xml_element, int &$recursion_depth = 0) { // If we're getting too deep, bail out if ($recursion_depth > 512) { @@ -217,7 +219,7 @@ class XML if (is_array($xml_element)) { $result_array = []; if (count($xml_element) <= 0) { - return (trim(strval($xml_element_copy))); + return trim(strval($xml_element_copy)); } foreach ($xml_element as $key => $value) { @@ -233,9 +235,9 @@ class XML ]; } - return ($result_array); + return $result_array; } else { - return (trim(strval($xml_element))); + return trim(strval($xml_element)); } } @@ -261,7 +263,7 @@ class XML * @return array The parsed XML in an array form. Use print_r() to see the resulting array structure. * @throws \Exception */ - public static function toArray($contents, $namespaces = true, $get_attributes = 1, $priority = 'attribute') + public static function toArray(string $contents, bool $namespaces = true, int $get_attributes = 1, string $priority = 'attribute'): array { if (!$contents) { return []; @@ -300,7 +302,7 @@ class XML Logger::debug('libxml: parse: ' . $err->code . " at " . $err->line . ":" . $err->column . " : " . $err->message); } libxml_clear_errors(); - return; + return []; } //Initializations @@ -414,20 +416,20 @@ class XML } } - return($xml_array); + return $xml_array; } /** * Delete a node in a XML object * - * @param \DOMDocument $doc XML document + * @param DOMDocument $doc XML document * @param string $node Node name * @return void */ - public static function deleteNode(\DOMDocument $doc, $node) + public static function deleteNode(DOMDocument $doc, string $node) { $xpath = new DOMXPath($doc); - $list = $xpath->query("//".$node); + $list = $xpath->query('//' . $node); foreach ($list as $child) { $child->parentNode->removeChild($child); } @@ -436,9 +438,9 @@ class XML /** * Parse XML string * - * @param string $s - * @param boolean $suppress_log - * @return Object + * @param string $s XML string to parse into object + * @param boolean $suppress_log Whether to supressing logging + * @return SimpleXMLElement|bool SimpleXMLElement or false on failure */ public static function parseString(string $s, bool $suppress_log = false) { @@ -458,7 +460,15 @@ class XML return $x; } - public static function getFirstNodeValue(DOMXPath $xpath, $element, $context = null) + /** + * Gets first node value + * + * @param DOMXPath $xpath XPath object + * @param string $element Element name + * @param DOMNode $context Context object or NULL + * @return string XML node value or empty string on failure + */ + public static function getFirstNodeValue(DOMXPath $xpath, string $element, DOMNode $context = null) { $result = @$xpath->evaluate($element, $context); if (!is_object($result)) { @@ -473,7 +483,15 @@ class XML return $first_item->nodeValue; } - public static function getFirstAttributes(DOMXPath $xpath, $element, $context = null) + /** + * Gets first attributes + * + * @param DOMXPath $xpath XPath object + * @param string $element Element name + * @param DOMNode $context Context object or NULL + * @return ???|bool First element's attributes field or false on failure + */ + public static function getFirstAttributes(DOMXPath $xpath, string $element, DOMNode $context = null) { $result = @$xpath->query($element, $context); if (!is_object($result)) { @@ -488,9 +506,17 @@ class XML return $first_item->attributes; } - public static function getFirstValue($xpath, $search, $context) + /** + * Gets first node's value + * + * @param DOMXPath $xpath XPath object + * @param string $element Element name + * @param DOMNode $context Context object or NULL + * @return string First value or empty string on failure + */ + public static function getFirstValue(DOMXPath $xpath, string $element, DOMNode $context = null): string { - $result = @$xpath->query($search, $context); + $result = @$xpath->query($element, $context); if (!is_object($result)) { return ''; } @@ -508,32 +534,31 @@ class XML * * @param string $str * @return string Escaped text. + * @todo Move this generic method to Util\Strings and also rewrite all other findingd */ - public static function escape($str) + public static function escape(string $str): string { - $buffer = htmlspecialchars($str, ENT_QUOTES, 'UTF-8'); - $buffer = trim($buffer); - - return $buffer; + return trim(htmlspecialchars($str, ENT_QUOTES, 'UTF-8')); } /** - * undo an escape + * Undo an escape * * @param string $s xml escaped text * @return string unescaped text + * @todo Move this generic method to Util\Strings and also rewrite all other findingd */ - public static function unescape($s) + public static function unescape(string $s): string { - $ret = htmlspecialchars_decode($s, ENT_QUOTES); - return $ret; + return htmlspecialchars_decode($s, ENT_QUOTES); } /** - * apply escape() to all values of array $val, recursively + * Apply escape() to all values of array $val, recursively * - * @param array $val - * @return array|string + * @param array|bool|string $val Value of type bool, array or string + * @return array|string Returns array if array provided or string in other cases + * @todo Move this generic method to Util\Strings */ public static function arrayEscape($val) { From 95f9eb34ac88a06934792738fe5507431914c521 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Tue, 21 Jun 2022 01:29:20 +0200 Subject: [PATCH 19/20] Fixed indenting --- src/Protocol/ActivityPub/Transmitter.php | 26 ++++++++++++++++-------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/Protocol/ActivityPub/Transmitter.php b/src/Protocol/ActivityPub/Transmitter.php index b196f196a1..6384e2ed44 100644 --- a/src/Protocol/ActivityPub/Transmitter.php +++ b/src/Protocol/ActivityPub/Transmitter.php @@ -2041,13 +2041,15 @@ class Transmitter $id = DI::baseUrl() . '/activity/' . System::createGUID(); } - $data = ['@context' => ActivityPub::CONTEXT, + $data = [ + '@context' => ActivityPub::CONTEXT, 'id' => $id, 'type' => $activity, 'actor' => $owner['url'], 'object' => $profile['url'], 'instrument' => self::getService(), - 'to' => [$profile['url']]]; + 'to' => [$profile['url']], + ]; Logger::info('Sending activity ' . $activity . ' to ' . $target . ' for user ' . $uid); @@ -2094,13 +2096,15 @@ class Transmitter $owner = User::getOwnerDataById($uid); - $data = ['@context' => ActivityPub::CONTEXT, + $data = [ + '@context' => ActivityPub::CONTEXT, 'id' => DI::baseUrl() . '/activity/' . System::createGUID(), 'type' => 'Follow', 'actor' => $owner['url'], 'object' => $object, 'instrument' => self::getService(), - 'to' => [$profile['url']]]; + 'to' => [$profile['url']], + ]; Logger::info('Sending follow ' . $object . ' to ' . $target . ' for user ' . $uid); @@ -2127,7 +2131,8 @@ class Transmitter } $owner = User::getOwnerDataById($uid); - $data = ['@context' => ActivityPub::CONTEXT, + $data = [ + '@context' => ActivityPub::CONTEXT, 'id' => DI::baseUrl() . '/activity/' . System::createGUID(), 'type' => 'Accept', 'actor' => $owner['url'], @@ -2138,7 +2143,8 @@ class Transmitter 'object' => $owner['url'] ], 'instrument' => self::getService(), - 'to' => [$profile['url']]]; + 'to' => [$profile['url']], + ]; Logger::debug('Sending accept to ' . $target . ' for user ' . $uid . ' with id ' . $id); @@ -2165,7 +2171,8 @@ class Transmitter } $owner = User::getOwnerDataById($uid); - $data = ['@context' => ActivityPub::CONTEXT, + $data = [ + '@context' => ActivityPub::CONTEXT, 'id' => DI::baseUrl() . '/activity/' . System::createGUID(), 'type' => 'Reject', 'actor' => $owner['url'], @@ -2176,7 +2183,8 @@ class Transmitter 'object' => $owner['url'] ], 'instrument' => self::getService(), - 'to' => [$profile['url']]]; + 'to' => [$profile['url']], + ]; Logger::debug('Sending reject to ' . $target . ' for user ' . $uid . ' with id ' . $id); @@ -2223,7 +2231,7 @@ class Transmitter 'object' => $profile['url'] ], 'instrument' => self::getService(), - 'to' => [$profile['url']] + 'to' => [$profile['url']], ]; Logger::info('Sending undo to ' . $target . ' for user ' . $uid . ' with id ' . $id); From 3e522ed5126a840687c8be56ce0b47f2acff0337 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20H=C3=A4der?= Date: Tue, 21 Jun 2022 01:41:34 +0200 Subject: [PATCH 20/20] Fixed: - prevent NULL from being handled over to XML::escape() - still I wonder that an object can be? It is a string-only accepting method --- src/Util/XML.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Util/XML.php b/src/Util/XML.php index 76947e9431..734fd9298a 100644 --- a/src/Util/XML.php +++ b/src/Util/XML.php @@ -56,7 +56,7 @@ class XML $root = new SimpleXMLElement('<' . $key . '/>'); self::fromArray($value, $root, $remove_header, $namespaces, false); } else { - $root = new SimpleXMLElement('<' . $key . '>'.self::escape($value).''); + $root = new SimpleXMLElement('<' . $key . '>' . self::escape($value ?? '') . ''); } $dom = dom_import_simplexml($root)->ownerDocument; @@ -124,7 +124,7 @@ class XML } if (!is_array($value)) { - $element = $xml->addChild($key, self::escape($value), $namespace); + $element = $xml->addChild($key, self::escape($value ?? ''), $namespace); } elseif (is_array($value)) { $element = $xml->addChild($key, null, $namespace); self::fromArray($value, $element, $remove_header, $namespaces, false);