Merge pull request #94 from iMattPro/sanitization

Try again at shortname cleanup

Author: iMattPro

controller/manifest.php

@@ -15,6 +15,7 @@
 use phpbb\language\language;
 use phpbb\path_helper;
 use phpbb\user;
+use phpbb\webpushnotifications\ext;
 use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Response;
 
@@ -63,9 +64,13 @@ public function handle(): JsonResponse
 		$board_path = $this->config['force_server_vars'] ? $this->config['script_path'] : $this->path_helper->get_web_root_path();
 		$board_url = generate_board_url();
 
+		// Emoji fixer-uppers
+		$sitename = ext::decode_entities($this->config['sitename'], ENT_QUOTES);
+		$pwa_short_name = ext::decode_entities($this->config['pwa_short_name'], ENT_QUOTES);
+
 		$manifest = [
-			'name'			=> $this->config['sitename'],
-			'short_name'	=> $this->config['pwa_short_name'] ?: utf8_substr(preg_replace('/[^\x21-\x7E]/', '', html_entity_decode($this->config['sitename'], ENT_QUOTES, 'UTF-8')), 0, 12),
+			'name'			=> $sitename,
+			'short_name'	=> $pwa_short_name ?: utf8_substr($sitename, 0, 12),
 			'display'		=> 'standalone',
 			'orientation'	=> 'portrait',
 			'dir'			=> $this->language->lang('DIRECTION'),

event/listener.php

@@ -17,6 +17,7 @@
 use phpbb\notification\manager;
 use phpbb\template\template;
 use phpbb\user;
+use phpbb\webpushnotifications\ext;
 use phpbb\webpushnotifications\form\form_helper;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 
@@ -85,6 +86,7 @@ public static function getSubscribedEvents()
 			'core.ucp_display_module_before'	=> 'load_language',
 			'core.acp_main_notice'				=> 'compatibility_notice',
 			'core.acp_board_config_edit_add'		=> 'acp_pwa_options',
+			'core.acp_board_config_emoji_enabled'=> 'acp_pwa_allow_emoji',
 			'core.validate_config_variable'		=> 'validate_pwa_options',
 			'core.help_manager_add_block_after'	=> 'wpn_faq',
 		];
@@ -143,7 +145,7 @@ public function pwa_manifest()
 		$this->template->assign_vars([
 			'U_MANIFEST_URL'	=> $this->controller_helper->route('phpbb_webpushnotifications_manifest_controller'),
 			'U_TOUCH_ICON'		=> $this->config['pwa_icon_small'],
-			'SHORT_SITE_NAME'	=> $this->config['pwa_short_name'] ?: $this->get_shortname($this->config['sitename']),
+			'SHORT_SITE_NAME'	=> $this->config['pwa_short_name'] ?: $this->trim_shortname($this->config['sitename']),
 		]);
 	}
 
@@ -170,6 +172,24 @@ public function acp_pwa_options($event)
 		}
 	}
 
+	/**
+	 * Allow PWA short name ACP field to accept emoji characters
+	 *
+	 * @param \phpbb\event\data $event
+	 * @return void
+	 */
+	public function acp_pwa_allow_emoji($event)
+	{
+		if (in_array('pwa_short_name', $event['config_name_ary'], true))
+		{
+			return;
+		}
+
+		$config_name_ary = $event['config_name_ary'];
+		$config_name_ary[] = 'pwa_short_name';
+		$event['config_name_ary'] = $config_name_ary;
+	}
+
 	/**
 	 * Return HTML for PWA icon name settings
 	 *
@@ -191,7 +211,7 @@ public function pwa_icon_name($value, $key)
 	 */
 	public function pwa_short_sitename($value, $key)
 	{
-		$placeholder = $this->get_shortname($this->config['sitename']);
+		$placeholder = $this->trim_shortname($this->config['sitename']);
 
 		return '<input id="' . $key . '" type="text" size="40" maxlength="12" name="config[' . $key . ']" value="' . $value . '" placeholder="' . $placeholder . '">';
 	}
@@ -223,17 +243,10 @@ public function validate_pwa_options($event)
 					return;
 				}
 
-				$short_name = $event['cfg_array']['pwa_short_name'];
-
-				// Do not allow multibyte characters or emoji
-				if (strlen($short_name) !== mb_strlen($short_name, 'UTF-8'))
-				{
-					$this->add_error($event, 'PWA_SHORT_NAME_INVALID');
-					return;
-				}
+				$short_name = ext::decode_entities($event['cfg_array']['pwa_short_name'], ENT_QUOTES);
 
 				// Do not allow strings longer than 12 characters
-				if (strlen($short_name) > 12)
+				if (utf8_strlen($short_name) > 12)
 				{
 					$this->add_error($event, 'PWA_SHORT_NAME_INVALID');
 					return;
@@ -346,13 +359,15 @@ protected function can_use_notifications()
 	}
 
 	/**
-	 * Get short name from a string (strip out multibyte characters and trim to 12 characters)
+	 * Trim short name from a string to 12 characters
 	 *
 	 * @param string $name
 	 * @return string 12 max characters string
 	 */
-	protected function get_shortname($name)
+	protected function trim_shortname($name)
 	{
-		return utf8_substr(preg_replace('/[^\x20-\x7E]/', '', $name), 0, 12);
+		$decoded = ext::decode_entities($name, ENT_QUOTES);
+		$trimmed = utf8_substr($decoded, 0, 12);
+		return htmlspecialchars($trimmed, ENT_QUOTES, 'UTF-8');
 	}
 }

ext.php

@@ -124,4 +124,17 @@ protected function result()
 
 		return false;
 	}
+
+	/**
+	 * Decode entities, used primarily to fix emoji for display
+	 *
+	 * @param string $text
+	 * @param int $flags Uses ENT_NOQUOTES to leave single and double quotes encoded by default
+	 * @param string $encoding
+	 * @return string Decoded string
+	 */
+	public static function decode_entities($text, $flags = ENT_NOQUOTES, $encoding = 'UTF-8')
+	{
+		return html_entity_decode($text, $flags, $encoding);
+	}
 }

language/en/webpushnotifications_common_acp.php

@@ -41,7 +41,7 @@
 	'PWA_SETTINGS'				=> 'Progressive web application options',
 	'PWA_SHORT_NAME'			=> 'Short site name',
 	'PWA_SHORT_NAME_EXPLAIN'	=> 'Your site name in 12 characters or fewer, which may be used as a label for an icon on a mobile device’s home screen. (If this field is left empty, the first 12 characters of the <samp>Site name</samp> will be used.)',
-	'PWA_SHORT_NAME_INVALID'	=> '“Short site name” contains illegal characters or exceeds the 12 character limit.',
+	'PWA_SHORT_NAME_INVALID'	=> '“Short site name” exceeds the 12 character limit.',
 	'PWA_ICON_SMALL'			=> 'Small mobile device icon',
 	'PWA_ICON_SMALL_EXPLAIN'	=> 'File name of a 192px x 192px PNG image. This file must be uploaded to your board’s <samp>icons</samp> directory.',
 	'PWA_ICON_LARGE'			=> 'Large mobile device icon',

tests/event/listener_test.php

@@ -149,6 +149,7 @@ public function test_getSubscribedEvents()
 			'core.ucp_display_module_before',
 			'core.acp_main_notice',
 			'core.acp_board_config_edit_add',
+			'core.acp_board_config_emoji_enabled',
 			'core.validate_config_variable',
 			'core.help_manager_add_block_after',
 		], array_keys(\phpbb\webpushnotifications\event\listener::getSubscribedEvents()));
@@ -376,7 +377,6 @@ public function test_acp_pwa_options($mode, $display_vars, $expected_keys)
 		$keys = array_keys($display_vars['vars']);
 
 		self::assertEquals($expected_keys, $keys);
-
 	}
 
 	public function validate_pwa_options_data()
@@ -420,6 +420,21 @@ public function validate_pwa_options_data()
 			[
 				'pwa_options:string',
 				['pwa_short_name' => 'foo❤️'],
+				[],
+			],
+			[
+				'pwa_options:string',
+				['pwa_short_name' => 'Фаны phpBB'],
+				[],
+			],
+			[
+				'pwa_options:string',
+				['pwa_short_name' => 'Фаны phpBB Board'],
+				['PWA_SHORT_NAME_INVALID'],
+			],
+			[
+				'pwa_options:string',
+				['pwa_short_name' => 'foo❤️bar foo bar'],
 				['PWA_SHORT_NAME_INVALID'],
 			],
 			[
@@ -472,6 +487,24 @@ public function test_validate_pwa_options($validate, $cfg_array, $expected_error
 		self::assertEquals($expected_error, $error);
 	}
 
+	public function test_acp_pwa_allow_emoji()
+	{
+		$config_name_ary = ['foo'];
+		$expected = ['foo', 'pwa_short_name'];
+
+		$this->set_listener();
+
+		$dispatcher = new \phpbb\event\dispatcher();
+		$dispatcher->addListener('core.acp_board_config_emoji_enabled', [$this->listener, 'acp_pwa_allow_emoji']);
+
+		$event_data = ['config_name_ary'];
+		$event_data_after = $dispatcher->trigger_event('core.acp_board_config_emoji_enabled', compact($event_data));
+
+		extract($event_data_after, EXTR_OVERWRITE);
+
+		self::assertEquals($expected, $config_name_ary);
+	}
+
 	public function test_wpn_faq()
 	{
 		$this->language->add_lang('webpushnotifications_faq', 'phpbb/webpushnotifications');

ucp/controller/webpush.php

@@ -16,6 +16,7 @@
 use phpbb\exception\http_exception;
 use phpbb\language\language;
 use phpbb\notification\manager;
+use phpbb\webpushnotifications\ext;
 use phpbb\webpushnotifications\form\form_helper;
 use phpbb\webpushnotifications\json\sanitizer as json_sanitizer;
 use phpbb\path_helper;
@@ -240,8 +241,8 @@ private function get_notification_data(string $notification_data): string
 
 		return json_encode([
 			'heading'	=> $this->config['sitename'],
-			'title'		=> strip_tags(html_entity_decode($notification->get_title(), ENT_NOQUOTES, 'UTF-8')),
-			'text'		=> strip_tags(html_entity_decode($notification->get_reference(), ENT_NOQUOTES, 'UTF-8')),
+			'title'		=> strip_tags(ext::decode_entities($notification->get_title())),
+			'text'		=> strip_tags(ext::decode_entities($notification->get_reference())),
 			'url'		=> htmlspecialchars_decode($notification->get_url()),
 			'avatar'	=> $this->prepare_avatar($notification->get_avatar()),
 		]);