From da4b446ac610734ef9f0733de57a58ec4b5602eb Mon Sep 17 00:00:00 2001 From: rxu <rxu@mail.ru> Date: Thu, 4 Jun 2020 15:08:06 +0700 Subject: [PATCH 1/5] [ticket/16512] Fix make_clickable() to make use of custom link classes PHPBB3-16512 --- phpBB/includes/functions_content.php | 68 +++++++--- tests/functions/make_clickable_test.php | 173 +++++++++++++++--------- 2 files changed, 156 insertions(+), 85 deletions(-) diff --git a/phpBB/includes/functions_content.php b/phpBB/includes/functions_content.php index b89eec9593..f35ae9b058 100644 --- a/phpBB/includes/functions_content.php +++ b/phpBB/includes/functions_content.php @@ -927,7 +927,7 @@ function make_clickable_callback($type, $whitespace, $url, $relative_url, $class * Cuts down displayed size of link if over 50 chars, turns absolute links * into relative versions when the server/script path matches the link */ -function make_clickable($text, $server_url = false, $class = 'postlink') +function make_clickable($text, $server_url = false, string $class = 'postlink') { if ($server_url === false) { @@ -948,29 +948,51 @@ function make_clickable($text, $server_url = false, $class = 'postlink') $magic_url_match_args = array(); } - // relative urls for this board - $magic_url_match_args[$server_url][] = array( - '#(^|[\n\t (>.])(' . preg_quote($server_url, '#') . ')/(' . get_preg_expression('relative_url_inline') . ')#iu', - MAGIC_URL_LOCAL, - $local_class, - ); + // Check if tne match for this $server_url and $class already exists + $element_exists = false; + if (isset($magic_url_match_args[$server_url])) + { + array_walk_recursive($magic_url_match_args[$server_url], function($value, $key) use (&$element_exists, $static_class) + { + if ($value == $static_class) + { + $element_exists = true; + return; + } + } + ); + } - // matches a xxxx://aaaaa.bbb.cccc. ... - $magic_url_match_args[$server_url][] = array( - '#(^|[\n\t (>.])(' . get_preg_expression('url_inline') . ')#iu', - MAGIC_URL_FULL, - $class, - ); + // Only add new $server_url and $class matches if not exist + if (!$element_exists) + { + // relative urls for this board + $magic_url_match_args[$server_url][] = array( + '#(^|[\n\t (>.])(' . preg_quote($server_url, '#') . ')/(' . get_preg_expression('relative_url_inline') . ')#iu', + MAGIC_URL_LOCAL, + $local_class, + $static_class, + ); - // matches a "www.xxxx.yyyy[/zzzz]" kinda lazy URL thing - $magic_url_match_args[$server_url][] = array( - '#(^|[\n\t (>])(' . get_preg_expression('www_url_inline') . ')#iu', - MAGIC_URL_WWW, - $class, - ); + // matches a xxxx://aaaaa.bbb.cccc. ... + $magic_url_match_args[$server_url][] = array( + '#(^|[\n\t (>.])(' . get_preg_expression('url_inline') . ')#iu', + MAGIC_URL_FULL, + $class, + $static_class, + ); + + // matches a "www.xxxx.yyyy[/zzzz]" kinda lazy URL thing + $magic_url_match_args[$server_url][] = array( + '#(^|[\n\t (>])(' . get_preg_expression('www_url_inline') . ')#iu', + MAGIC_URL_WWW, + $class, + $static_class, + ); + } // matches an email@domain type address at the start of a line, or after a space or after what might be a BBCode. - $magic_url_match_args[$server_url][] = array( + $magic_url_match_args[$server_url]['email'] = array( '/(^|[\n\t (>])(' . get_preg_expression('email') . ')/iu', MAGIC_URL_EMAIL, '', @@ -981,6 +1003,12 @@ function make_clickable($text, $server_url = false, $class = 'postlink') { if (preg_match($magic_args[0], $text, $matches)) { + // Only apply $class from the correcponding function call argument (excepting emails which never has a class) + if ($magic_args[3] != $static_class && $magic_args[1] != MAGIC_URL_EMAIL) + { + continue; + } + $text = preg_replace_callback($magic_args[0], function($matches) use ($magic_args) { $relative_url = isset($matches[3]) ? $matches[3] : ''; diff --git a/tests/functions/make_clickable_test.php b/tests/functions/make_clickable_test.php index d8d5eb5e0e..e01a86b023 100644 --- a/tests/functions/make_clickable_test.php +++ b/tests/functions/make_clickable_test.php @@ -27,131 +27,173 @@ class phpbb_functions_make_clickable_test extends phpbb_test_case **/ public function data_test_make_clickable_url_positive() { - return array( - array( + return [ + [ 'http://www.phpbb.com/community/', '<!-- m --><a class="postlink" href="http://www.phpbb.com/community/">http://www.phpbb.com/community/</a><!-- m -->' - ), - array( + ], + [ 'http://www.phpbb.com/path/file.ext#section', '<!-- m --><a class="postlink" href="http://www.phpbb.com/path/file.ext#section">http://www.phpbb.com/path/file.ext#section</a><!-- m -->' - ), - array( + ], + [ 'ftp://ftp.phpbb.com/', '<!-- m --><a class="postlink" href="ftp://ftp.phpbb.com/">ftp://ftp.phpbb.com/</a><!-- m -->' - ), - array( + ], + [ 'sip://bantu@phpbb.com', '<!-- m --><a class="postlink" href="sip://bantu@phpbb.com">sip://bantu@phpbb.com</a><!-- m -->' - ), - array( + ], + [ 'www.phpbb.com/community/', '<!-- w --><a class="postlink" href="http://www.phpbb.com/community/">www.phpbb.com/community/</a><!-- w -->' - ), - array( + ], + [ 'http://testhost/viewtopic.php?t=1', '<!-- l --><a class="postlink-local" href="http://testhost/viewtopic.php?t=1">viewtopic.php?t=1</a><!-- l -->' - ), - array( + ], + [ 'javascript://testhost/viewtopic.php?t=1', 'javascript://testhost/viewtopic.php?t=1' - ), - array( + ], + [ "java\nscri\npt://testhost/viewtopic.php?t=1", "java\nscri\n<!-- m --><a class=\"postlink\" href=\"pt://testhost/viewtopic.php?t=1\">pt://testhost/viewtopic.php?t=1</a><!-- m -->" - ), - array( + ], + [ 'email@domain.com', '<!-- e --><a href="mailto:email@domain.com">email@domain.com</a><!-- e -->' - ), + ], // Test appending punctuation mark to the URL - array( + [ 'http://testhost/viewtopic.php?t=1!', '<!-- l --><a class="postlink-local" href="http://testhost/viewtopic.php?t=1">viewtopic.php?t=1</a><!-- l -->!' - ), - array( + ], + [ 'www.phpbb.com/community/?', '<!-- w --><a class="postlink" href="http://www.phpbb.com/community/">www.phpbb.com/community/</a><!-- w -->?' - ), + ], // Test shortened text for URL > 55 characters long // URL text should be turned into: first 39 chars + ' ... ' + last 10 chars - array( + [ 'http://www.phpbb.com/community/path/to/long/url/file.ext#section', '<!-- m --><a class="postlink" href="http://www.phpbb.com/community/path/to/long/url/file.ext#section">http://www.phpbb.com/community/path/to/ ... xt#section</a><!-- m -->' - ), - ); + ], + ]; } public function data_test_make_clickable_url_idn() { - return array( - array( + return [ + [ 'http://www.täst.de/community/', '<!-- m --><a class="postlink" href="http://www.täst.de/community/">http://www.täst.de/community/</a><!-- m -->' - ), - array( + ], + [ 'http://www.täst.de/path/file.ext#section', '<!-- m --><a class="postlink" href="http://www.täst.de/path/file.ext#section">http://www.täst.de/path/file.ext#section</a><!-- m -->' - ), - array( + ], + [ 'ftp://ftp.täst.de/', '<!-- m --><a class="postlink" href="ftp://ftp.täst.de/">ftp://ftp.täst.de/</a><!-- m -->' - ), - array( + ], + [ 'javascript://täst.de/', 'javascript://täst.de/' - ), - array( + ], + [ 'sip://bantu@täst.de', '<!-- m --><a class="postlink" href="sip://bantu@täst.de">sip://bantu@täst.de</a><!-- m -->' - ), - array( + ], + [ 'www.täst.de/community/', '<!-- w --><a class="postlink" href="http://www.täst.de/community/">www.täst.de/community/</a><!-- w -->' - ), + ], // Test appending punctuation mark to the URL - array( + [ 'http://домен.рф/viewtopic.php?t=1!', '<!-- m --><a class="postlink" href="http://домен.рф/viewtopic.php?t=1">http://домен.рф/viewtopic.php?t=1</a><!-- m -->!' - ), - array( + ], + [ 'www.домен.рф/сообщество/?', '<!-- w --><a class="postlink" href="http://www.домен.рф/сообщество/">www.домен.рф/сообщество/</a><!-- w -->?' - ), + ], // Test shortened text for URL > 55 characters long // URL text should be turned into: first 39 chars + ' ... ' + last 10 chars - array( + [ 'http://www.домен.рф/сообщество/путь/по/длинной/ссылке/file.ext#section', '<!-- m --><a class="postlink" href="http://www.домен.рф/сообщество/путь/по/длинной/ссылке/file.ext#section">http://www.домен.рф/сообщество/путь/по/ ... xt#section</a><!-- m -->' - ), + ], // IDN with invalid characters shouldn't be parsed correctly (only 'valid' part) - array( + [ 'http://www.täst╫.de', '<!-- m --><a class="postlink" href="http://www.täst">http://www.täst</a><!-- m -->╫.de' - ), + ], // IDN in emails is unsupported yet - array('почта@домен.рф', 'почта@домен.рф'), - ); + ['почта@домен.рф', 'почта@домен.рф'], + ]; } public function data_test_make_clickable_local_url_idn() { - return array( - array( + return [ + [ 'http://www.домен.рф/viewtopic.php?t=1', '<!-- l --><a class="postlink-local" href="http://www.домен.рф/viewtopic.php?t=1">viewtopic.php?t=1</a><!-- l -->' - ), + ], // Test appending punctuation mark to the URL - array( + [ 'http://www.домен.рф/viewtopic.php?t=1!', '<!-- l --><a class="postlink-local" href="http://www.домен.рф/viewtopic.php?t=1">viewtopic.php?t=1</a><!-- l -->!' - ), - array( + ], + [ 'http://www.домен.рф/сообщество/?', '<!-- l --><a class="postlink-local" href="http://www.домен.рф/сообщество/">сообщество/</a><!-- l -->?' - ), - ); + ], + ]; + } + + public function data_test_make_clickable_custom_classes() + { + return [ + [ + 'http://www.домен.рф/viewtopic.php?t=1', + 'http://www.домен.рф', + 'class1', + '<!-- l --><a class="class1-local" href="http://www.домен.рф/viewtopic.php?t=1">viewtopic.php?t=1</a><!-- l -->' + ], + [ + 'http://www.домен.рф/viewtopic.php?t=1!', + false, + 'class2', + '<!-- m --><a class="class2" href="http://www.домен.рф/viewtopic.php?t=1">http://www.домен.рф/viewtopic.php?t=1</a><!-- m -->!' + ], + [ + 'http://www.домен.рф/сообщество/?', + false, + 'class3', + '<!-- m --><a class="class3" href="http://www.домен.рф/сообщество/">http://www.домен.рф/сообщество/</a><!-- m -->?' + ], + [ + 'www.phpbb.com/community/', + false, + 'class2', + '<!-- w --><a class="class2" href="http://www.phpbb.com/community/">www.phpbb.com/community/</a><!-- w -->' + ], + [ + 'http://testhost/viewtopic.php?t=1', + false, + 'class1', + '<!-- l --><a class="class1-local" href="http://testhost/viewtopic.php?t=1">viewtopic.php?t=1</a><!-- l -->' + ], + [ + 'email@domain.com', + false, + 'class-email', + '<!-- e --><a href="mailto:email@domain.com">email@domain.com</a><!-- e -->' + ], + ]; } protected function setUp(): void @@ -166,16 +208,9 @@ class phpbb_functions_make_clickable_test extends phpbb_test_case /** * @dataProvider data_test_make_clickable_url_positive - */ - public function test_urls_matching_positive($url, $expected) - { - $this->assertSame($expected, make_clickable($url)); - } - - /** * @dataProvider data_test_make_clickable_url_idn */ - public function test_urls_matching_idn($url, $expected) + public function test_urls_matching_positive($url, $expected) { $this->assertSame($expected, make_clickable($url)); } @@ -187,4 +222,12 @@ class phpbb_functions_make_clickable_test extends phpbb_test_case { $this->assertSame($expected, make_clickable($url, "http://www.домен.рф")); } + + /** + * @dataProvider data_test_make_clickable_custom_classes + */ + public function test_make_clickable_custom_classes($url, $server_url, $class, $expected) + { + $this->assertSame($expected, make_clickable($url, $server_url, $class)); + } } From f340e8ca04a76da60289dcaaf48105e625e174fc Mon Sep 17 00:00:00 2001 From: rxu <rxu@mail.ru> Date: Thu, 4 Jun 2020 23:09:26 +0700 Subject: [PATCH 2/5] [ticket/16512] Docblock and code adjustments PHPBB3-16512 --- phpBB/includes/functions_content.php | 45 ++++++++++++++++------------ 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/phpBB/includes/functions_content.php b/phpBB/includes/functions_content.php index f35ae9b058..e82fc09f7b 100644 --- a/phpBB/includes/functions_content.php +++ b/phpBB/includes/functions_content.php @@ -921,12 +921,16 @@ function make_clickable_callback($type, $whitespace, $url, $relative_url, $class } /** -* make_clickable function -* -* Replace magic urls of form http://xxx.xxx., www.xxx. and xxx@xxx.xxx. -* Cuts down displayed size of link if over 50 chars, turns absolute links -* into relative versions when the server/script path matches the link -*/ + * Replaces magic urls of form http://xxx.xxx., www.xxx. and xxx@xxx.xxx. + * Cuts down displayed size of link if over 50 chars, turns absolute links + * into relative versions when the server/script path matches the link + * + * @param string $text Message text to parse URL/email entries + * @param bool|string $server_url The server URL. If false, the board URL will be used + * @param string $class CSS class selector to add to the parsed URL entries + * + * @return string A text with parsed URL/email entries + */ function make_clickable($text, $server_url = false, string $class = 'postlink') { if ($server_url === false) @@ -948,7 +952,7 @@ function make_clickable($text, $server_url = false, string $class = 'postlink') $magic_url_match_args = array(); } - // Check if tne match for this $server_url and $class already exists + // Check if the match for this $server_url and $class already exists $element_exists = false; if (isset($magic_url_match_args[$server_url])) { @@ -967,36 +971,39 @@ function make_clickable($text, $server_url = false, string $class = 'postlink') if (!$element_exists) { // relative urls for this board - $magic_url_match_args[$server_url][] = array( + $magic_url_match_args[$server_url][] = [ '#(^|[\n\t (>.])(' . preg_quote($server_url, '#') . ')/(' . get_preg_expression('relative_url_inline') . ')#iu', MAGIC_URL_LOCAL, $local_class, $static_class, - ); + ]; // matches a xxxx://aaaaa.bbb.cccc. ... - $magic_url_match_args[$server_url][] = array( + $magic_url_match_args[$server_url][] = [ '#(^|[\n\t (>.])(' . get_preg_expression('url_inline') . ')#iu', MAGIC_URL_FULL, $class, $static_class, - ); + ]; // matches a "www.xxxx.yyyy[/zzzz]" kinda lazy URL thing - $magic_url_match_args[$server_url][] = array( + $magic_url_match_args[$server_url][] = [ '#(^|[\n\t (>])(' . get_preg_expression('www_url_inline') . ')#iu', MAGIC_URL_WWW, $class, $static_class, - ); + ]; } - // matches an email@domain type address at the start of a line, or after a space or after what might be a BBCode. - $magic_url_match_args[$server_url]['email'] = array( - '/(^|[\n\t (>])(' . get_preg_expression('email') . ')/iu', - MAGIC_URL_EMAIL, - '', - ); + if (!isset($magic_url_match_args[$server_url]['email'])) + { + // matches an email@domain type address at the start of a line, or after a space or after what might be a BBCode. + $magic_url_match_args[$server_url]['email'] = [ + '/(^|[\n\t (>])(' . get_preg_expression('email') . ')/iu', + MAGIC_URL_EMAIL, + '', + ]; + } } foreach ($magic_url_match_args[$server_url] as $magic_args) From 68122fca273e6a29fc26b3733382bbd1cb6339f5 Mon Sep 17 00:00:00 2001 From: rxu <rxu@mail.ru> Date: Sun, 7 Jun 2020 13:35:38 +0700 Subject: [PATCH 3/5] [ticket/16512] Add user_ipwhois() test which internally uses make_clickable() PHPBB3-16512 --- tests/functions_user/whois_test.php | 51 +++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 tests/functions_user/whois_test.php diff --git a/tests/functions_user/whois_test.php b/tests/functions_user/whois_test.php new file mode 100644 index 0000000000..02cd3a5ebd --- /dev/null +++ b/tests/functions_user/whois_test.php @@ -0,0 +1,51 @@ +<?php +/** +* +* @package testing +* @copyright (c) 2020 phpBB Group +* @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2 +* +*/ + +require_once dirname(__FILE__) . '/../../phpBB/includes/functions_user.php'; + +class phpbb_functions_user_whois_test extends phpbb_test_case +{ + public function setUp() + { + global $config, $phpbb_dispatcher, $user, $request, $symfony_request, $phpbb_root_path, $phpEx; + + $user = $this->getMockBuilder('\phpbb\user') + ->setConstructorArgs([ + new \phpbb\language\language(new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx)), + '\phpbb\datetime', + ]) + ->getMock(); + $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); + $config = new \phpbb\config\config([]); + $request = $this->getMockBuilder('\phpbb\request\request') + ->getMock(); + $symfony_request = $this->getMockBuilder('\phpbb\symfony_request') + ->disableOriginalConstructor() + ->getMock(); + } + + public function ips_data() + { + return [ + ['2001:4860:4860::8888'], // Google public DNS + ['64.233.161.139'], // google.com + ]; + } + + /** + * @dataProvider ips_data + */ + public function test_ip_whois($ip) + { + $ip_whois = user_ipwhois($ip); + $this->assertNotContains('Query terms are ambiguous', $ip_whois); + $this->assertNotContains('no entries found', $ip_whois); + $this->assertNotContains('ERROR', $ip_whois); + } +} From 7ef87e3fbd6089cf265d8aa57ad78dc20e6df6ee Mon Sep 17 00:00:00 2001 From: rxu <rxu@mail.ru> Date: Fri, 14 Aug 2020 04:24:06 +0700 Subject: [PATCH 4/5] [ticket/16512] Adjust test to use better assertions PHPBB3-16512 --- tests/functions_user/whois_test.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/functions_user/whois_test.php b/tests/functions_user/whois_test.php index 02cd3a5ebd..b99854ba60 100644 --- a/tests/functions_user/whois_test.php +++ b/tests/functions_user/whois_test.php @@ -44,8 +44,8 @@ class phpbb_functions_user_whois_test extends phpbb_test_case public function test_ip_whois($ip) { $ip_whois = user_ipwhois($ip); - $this->assertNotContains('Query terms are ambiguous', $ip_whois); - $this->assertNotContains('no entries found', $ip_whois); - $this->assertNotContains('ERROR', $ip_whois); + $this->assertStringNotContainsString('Query terms are ambiguous', $ip_whois); + $this->assertStringNotContainsString('no entries found', $ip_whois); + $this->assertStringNotContainsString('ERROR', $ip_whois); } } From d6078821c50dae5941ecceaca17fba3d71b10085 Mon Sep 17 00:00:00 2001 From: rxu <rxu@mail.ru> Date: Tue, 6 Oct 2020 07:29:02 +0700 Subject: [PATCH 5/5] [ticket/16512] Minor code adjustments PHPBB3-16512 --- phpBB/includes/functions_content.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/functions_content.php b/phpBB/includes/functions_content.php index e82fc09f7b..bfd6b179cd 100644 --- a/phpBB/includes/functions_content.php +++ b/phpBB/includes/functions_content.php @@ -956,7 +956,7 @@ function make_clickable($text, $server_url = false, string $class = 'postlink') $element_exists = false; if (isset($magic_url_match_args[$server_url])) { - array_walk_recursive($magic_url_match_args[$server_url], function($value, $key) use (&$element_exists, $static_class) + array_walk_recursive($magic_url_match_args[$server_url], function($value) use (&$element_exists, $static_class) { if ($value == $static_class) { @@ -1010,7 +1010,7 @@ function make_clickable($text, $server_url = false, string $class = 'postlink') { if (preg_match($magic_args[0], $text, $matches)) { - // Only apply $class from the correcponding function call argument (excepting emails which never has a class) + // Only apply $class from the corresponding function call argument (excepting emails which never has a class) if ($magic_args[3] != $static_class && $magic_args[1] != MAGIC_URL_EMAIL) { continue;