Fix an issue with tail parsing in object embeds in remarkup

Summary:
Fixes T6619. In `{Xnnn key=value, key=value}` we did not require a separator between the object and the key-value part. This could lead to `{rX11aaa}` being parsed as `{rX11 aaa}`, i.e. a reference to `rX11` with parameter `aaa` set.

Instead, require a space or comma before we'll parse key-value parts of embedded objects.

Test Plan:
Added and executed unit tests.

{F242002}

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T6619

Differential Revision: https://secure.phabricator.com/D10915

Author: epriestley

src/__phutil_library_map__.php

@@ -454,6 +454,7 @@
     'DiffusionCommitQuery' => 'applications/diffusion/query/DiffusionCommitQuery.php',
     'DiffusionCommitRef' => 'applications/diffusion/data/DiffusionCommitRef.php',
     'DiffusionCommitRemarkupRule' => 'applications/diffusion/remarkup/DiffusionCommitRemarkupRule.php',
+    'DiffusionCommitRemarkupRuleTestCase' => 'applications/diffusion/remarkup/__tests__/DiffusionCommitRemarkupRuleTestCase.php',
     'DiffusionCommitTagsController' => 'applications/diffusion/controller/DiffusionCommitTagsController.php',
     'DiffusionConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionConduitAPIMethod.php',
     'DiffusionController' => 'applications/diffusion/controller/DiffusionController.php',
@@ -3458,6 +3459,7 @@
     'DiffusionCommitQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'DiffusionCommitRef' => 'Phobject',
     'DiffusionCommitRemarkupRule' => 'PhabricatorObjectRemarkupRule',
+    'DiffusionCommitRemarkupRuleTestCase' => 'PhabricatorTestCase',
     'DiffusionCommitTagsController' => 'DiffusionController',
     'DiffusionConduitAPIMethod' => 'ConduitAPIMethod',
     'DiffusionController' => 'PhabricatorController',

src/applications/diffusion/remarkup/__tests__/DiffusionCommitRemarkupRuleTestCase.php

@@ -0,0 +1,60 @@
+<?php
+
+final class DiffusionCommitRemarkupRuleTestCase extends PhabricatorTestCase {
+
+  public function testProjectObjectRemarkup() {
+    $cases = array(
+      '{rP12f3f6d3a9ef9c7731051815846810cb3c4cd248}' => array(
+        'embed' => array(
+          array(
+            'offset' => 1,
+            'id' => 'rP12f3f6d3a9ef9c7731051815846810cb3c4cd248',
+          ),
+        ),
+        'ref' => array(
+          array(
+            'offset' => 1,
+            'id' => 'rP12f3f6d3a9ef9c7731051815846810cb3c4cd248',
+          ),
+        ),
+      ),
+      '{rP1234, key=value}' => array(
+        'embed' => array(
+          array(
+            'offset' => 1,
+            'id' => 'rP1234',
+            'tail' => ', key=value',
+          ),
+        ),
+        'ref' => array(
+          array(
+            'offset' => 1,
+            'id' => 'rP1234',
+          ),
+        ),
+      ),
+      '{rP1234 key=value}' => array(
+        'embed' => array(
+          array(
+            'offset' => 1,
+            'id' => 'rP1234',
+            'tail' => ' key=value',
+          ),
+        ),
+        'ref' => array(
+          array(
+            'offset' => 1,
+            'id' => 'rP1234',
+          ),
+        ),
+      ),
+    );
+
+    foreach ($cases as $input => $expect) {
+      $rule = new DiffusionCommitRemarkupRule();
+      $matches = $rule->extractReferences($input);
+      $this->assertEqual($expect, $matches, $input);
+    }
+  }
+
+}

src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php

@@ -166,7 +166,7 @@ private function getObjectEmbedPattern() {
     $prefix = preg_quote($prefix);
     $id = $this->getObjectIDPattern();
 
-    return '(\B{'.$prefix.'('.$id.')((?:[^}\\\\]|\\\\.)*)}\B)u';
+    return '(\B{'.$prefix.'('.$id.')([,\s](?:[^}\\\\]|\\\\.)*)?}\B)u';
   }
 
   private function getObjectReferencePattern() {