From a8be733e5ffebb9c0573d7e932e19c86d8cc5f09 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 1 Dec 2014 18:48:20 -0800
Subject: [PATCH] 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
---
 src/__phutil_library_map__.php                |  2 +
 .../DiffusionCommitRemarkupRuleTestCase.php   | 60 +++++++++++++++++++
 .../rule/PhabricatorObjectRemarkupRule.php    |  2 +-
 3 files changed, 63 insertions(+), 1 deletion(-)
 create mode 100644 src/applications/diffusion/remarkup/__tests__/DiffusionCommitRemarkupRuleTestCase.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index fc92678d7f..61fe4b2ad1 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -454,6 +454,7 @@ phutil_register_library_map(array(
     '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 @@ phutil_register_library_map(array(
     'DiffusionCommitQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'DiffusionCommitRef' => 'Phobject',
     'DiffusionCommitRemarkupRule' => 'PhabricatorObjectRemarkupRule',
+    'DiffusionCommitRemarkupRuleTestCase' => 'PhabricatorTestCase',
     'DiffusionCommitTagsController' => 'DiffusionController',
     'DiffusionConduitAPIMethod' => 'ConduitAPIMethod',
     'DiffusionController' => 'PhabricatorController',
diff --git a/src/applications/diffusion/remarkup/__tests__/DiffusionCommitRemarkupRuleTestCase.php b/src/applications/diffusion/remarkup/__tests__/DiffusionCommitRemarkupRuleTestCase.php
new file mode 100644
index 0000000000..b8476791be
--- /dev/null
+++ b/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);
+    }
+  }
+
+}
diff --git a/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php b/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php
index 3f8688897e..7f211c3654 100644
--- a/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php
+++ b/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php
@@ -166,7 +166,7 @@ abstract class PhabricatorObjectRemarkupRule extends PhutilRemarkupRule {
     $prefix = preg_quote($prefix);
     $id = $this->getObjectIDPattern();
 
-    return '(\B{'.$prefix.'('.$id.')((?:[^}\\\\]|\\\\.)*)}\B)u';
+    return '(\B{'.$prefix.'('.$id.')([,\s](?:[^}\\\\]|\\\\.)*)?}\B)u';
   }
 
   private function getObjectReferencePattern() {