diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3203,6 +3203,7 @@ 'PhabricatorMacroQuery' => 'applications/macro/query/PhabricatorMacroQuery.php', 'PhabricatorMacroReplyHandler' => 'applications/macro/mail/PhabricatorMacroReplyHandler.php', 'PhabricatorMacroSearchEngine' => 'applications/macro/query/PhabricatorMacroSearchEngine.php', + 'PhabricatorMacroTestCase' => 'applications/macro/xaction/__tests__/PhabricatorMacroTestCase.php', 'PhabricatorMacroTransaction' => 'applications/macro/storage/PhabricatorMacroTransaction.php', 'PhabricatorMacroTransactionComment' => 'applications/macro/storage/PhabricatorMacroTransactionComment.php', 'PhabricatorMacroTransactionQuery' => 'applications/macro/query/PhabricatorMacroTransactionQuery.php', @@ -8751,6 +8752,7 @@ 'PhabricatorMacroQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorMacroReplyHandler' => 'PhabricatorApplicationTransactionReplyHandler', 'PhabricatorMacroSearchEngine' => 'PhabricatorApplicationSearchEngine', + 'PhabricatorMacroTestCase' => 'PhabricatorTestCase', 'PhabricatorMacroTransaction' => 'PhabricatorModularTransaction', 'PhabricatorMacroTransactionComment' => 'PhabricatorApplicationTransactionComment', 'PhabricatorMacroTransactionQuery' => 'PhabricatorApplicationTransactionQuery', diff --git a/src/applications/macro/markup/PhabricatorImageMacroRemarkupRule.php b/src/applications/macro/markup/PhabricatorImageMacroRemarkupRule.php --- a/src/applications/macro/markup/PhabricatorImageMacroRemarkupRule.php +++ b/src/applications/macro/markup/PhabricatorImageMacroRemarkupRule.php @@ -8,7 +8,7 @@ public function apply($text) { return preg_replace_callback( - '@^\s*([a-zA-Z0-9:_\-]+)$@m', + '@^\s*([a-zA-Z0-9:_\x7f-\xff-]+)$@m', array($this, 'markupImageMacro'), $text); } diff --git a/src/applications/macro/xaction/PhabricatorMacroNameTransaction.php b/src/applications/macro/xaction/PhabricatorMacroNameTransaction.php --- a/src/applications/macro/xaction/PhabricatorMacroNameTransaction.php +++ b/src/applications/macro/xaction/PhabricatorMacroNameTransaction.php @@ -52,12 +52,16 @@ new PhutilNumber($max_length))); } - if (!preg_match('/^[a-z0-9:_-]{3,}\z/', $new_value)) { - $errors[] = $this->newInvalidError( - pht('Macro name "%s" be at least three characters long and contain '. - 'only lowercase letters, digits, hyphens, colons and '. - 'underscores.', - $new_value)); + if (!self::isValidMacroName($new_value)) { + // This says "emoji", but the actual rule we implement is "all other + // unicode characters are also fine". + $errors[] = $this->newInvalidError( + pht( + 'Macro name "%s" be: at least three characters long; and contain '. + 'only lowercase letters, digits, hyphens, colons, underscores, '. + 'and emoji; and not be composed entirely of latin symbols.', + $new_value), + $xaction); } // Check name is unique when updating / creating @@ -78,4 +82,35 @@ return $errors; } + public static function isValidMacroName($name) { + if (preg_match('/^[:_-]+\z/', $name)) { + return false; + } + + // Accept trivial macro names. + if (preg_match('/^[a-z0-9:_-]{3,}\z/', $name)) { + return true; + } + + // Reject names with fewer than 3 glyphs. + $length = phutil_utf8v_combined($name); + if (count($length) < 3) { + return false; + } + + // Check character-by-character for any symbols that we don't want. + $characters = phutil_utf8v($name); + foreach ($characters as $character) { + if (ord($character[0]) > 0x7F) { + continue; + } + + if (preg_match('/^[^a-z0-9:_-]/', $character)) { + return false; + } + } + + return true; + } + } diff --git a/src/applications/macro/xaction/__tests__/PhabricatorMacroTestCase.php b/src/applications/macro/xaction/__tests__/PhabricatorMacroTestCase.php new file mode 100644 --- /dev/null +++ b/src/applications/macro/xaction/__tests__/PhabricatorMacroTestCase.php @@ -0,0 +1,46 @@ + false, + "{$lit}{$lit}" => false, + + // Too short. + 'a' => false, + '' => false, + + // Bad characters. + 'yes!' => false, + "{$lit} {$lit} {$lit}" => false, + "aaa\nbbb" => false, + 'aaa~' => false, + 'aaa`' => false, + + // Special rejections for only latin symbols. + '---' => false, + '___' => false, + '-_-' => false, + ':::' => false, + '-_:' => false, + + "{$lit}{$lit}{$lit}" => true, + 'bwahahaha' => true, + "u{$combining_diaeresis}nt" => true, + 'a-a-a-a' => true, + ); + + foreach ($cases as $input => $expect) { + $this->assertEqual( + $expect, + PhabricatorMacroNameTransaction::isValidMacroName($input), + pht('Validity of macro "%s"', $input)); + } + } +}