Page MenuHomePhabricator
Files F15374279

D19051.diff
No OneTemporary
Actions

D19051.diff
View Options

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 @@
+<?php
+
+final class PhabricatorMacroTestCase
+ extends PhabricatorTestCase {
+
+ public function testMacroNames() {
+ $lit = "\xF0\x9F\x94\xA5";
+ $combining_diaeresis = "\xCC\x88";
+
+ $cases = array(
+ // Only 2 glyphs long.
+ "u{$combining_diaeresis}n" => 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));
+ }
+ }
+}

File Metadata

Mime Type
text/plain
Expires
Thu, Mar 13, 6:12 PM (1 w, 2 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7627956
Default Alt Text
D19051.diff (5 KB)

Event Timeline

Phabricator · Built by Phacility