Merge pull request #945 from thephpleague/fix-attribute-parsing

Fix attribute parsing
thephpleague · Oct 30, 2022 · 5d77bca · 5d77bca
2 parents 32891d1 + 9dfa625
commit 5d77bca
Show file tree

Hide file tree

Showing 6 changed files with 54 additions and 28 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,10 @@ Updates should follow the [Keep a CHANGELOG](https://keepachangelog.com/) princi
 
 ## [Unreleased][unreleased]
 
+### Fixed
+
+- Fixed unquoted attribute parsing when closing curly brace is followed by certain characters (like a `.`) (#943)
+
 ## [2.3.5] - 2022-07-29
 
 ### Fixed

diff --git a/src/Extension/Attributes/Util/AttributesHelper.php b/src/Extension/Attributes/Util/AttributesHelper.php
@@ -23,7 +23,8 @@
  */
 final class AttributesHelper
 {
-    private const REGEX = '/^\s*([.#][_a-z0-9-]+|' . RegexHelper::PARTIAL_ATTRIBUTENAME . RegexHelper::PARTIAL_ATTRIBUTEVALUESPEC . ')(?<!})\s*/i';
+    private const SINGLE_ATTRIBUTE = '\s*([.#][_a-z0-9-]+|' . RegexHelper::PARTIAL_ATTRIBUTENAME . RegexHelper::PARTIAL_ATTRIBUTEVALUESPEC . ')\s*';
+    private const ATTRIBUTE_LIST   = '/^{:?(' . self::SINGLE_ATTRIBUTE . ')+}/i';
 
     /**
      * @return array<string, mixed>
@@ -32,20 +33,33 @@ public static function parseAttributes(Cursor $cursor): array
     {
         $state = $cursor->saveState();
         $cursor->advanceToNextNonSpaceOrNewline();
-        if ($cursor->getCurrentCharacter() !== '{') {
+
+        // Quick check to see if we might have attributes
+        if ($cursor->getCharacter() !== '{') {
             $cursor->restoreState($state);
 
             return [];
         }
 
-        $cursor->advanceBy(1);
-        if ($cursor->getCurrentCharacter() === ':') {
-            $cursor->advanceBy(1);
+        // Attempt to match the entire attribute list expression
+        // While this is less performant than checking for '{' now and '}' later, it simplifies
+        // matching individual attributes since they won't need to look ahead for the closing '}'
+        // while dealing with the fact that attributes can technically contain curly braces.
+        // So we'll just match the start and end braces up front.
+        $attributeExpression = $cursor->match(self::ATTRIBUTE_LIST);
+        if ($attributeExpression === null) {
+            $cursor->restoreState($state);
+
+            return [];
         }
 
+        // Trim the leading '{' or '{:' and the trailing '}'
+        $attributeExpression = \ltrim(\substr($attributeExpression, 1, -1), ':');
+        $attributeCursor     = new Cursor($attributeExpression);
+
         /** @var array<string, mixed> $attributes */
         $attributes = [];
-        while ($attribute = \trim((string) $cursor->match(self::REGEX))) {
+        while ($attribute = \trim((string) $attributeCursor->match('/^' . self::SINGLE_ATTRIBUTE . '/i'))) {
             if ($attribute[0] === '#') {
                 $attributes['id'] = \substr($attribute, 1);
 
@@ -75,18 +89,6 @@ public static function parseAttributes(Cursor $cursor): array
             }
         }
 
-        if ($cursor->match('/}/') === null) {
-            $cursor->restoreState($state);
-
-            return [];
-        }
-
-        if ($attributes === []) {
-            $cursor->restoreState($state);
-
-            return [];
-        }
-
         if (isset($attributes['class'])) {
             $attributes['class'] = \implode(' ', (array) $attributes['class']);
         }

diff --git a/tests/functional/AbstractLocalDataTest.php b/tests/functional/AbstractLocalDataTest.php
@@ -76,7 +76,7 @@ protected function loadTests(string $dir, string $pattern = '*', string $inputFo
             $parsed   = (new FrontMatterParser(new SymfonyYamlFrontMatterParser()))->parse($input);
             $html     = \file_get_contents($dir . '/' . $testName . $outputFormat);
 
-            yield [$parsed->getContent(), $html, (array) $parsed->getFrontMatter(), $testName];
+            yield $testName => [$parsed->getContent(), $html, (array) $parsed->getFrontMatter(), $testName];
         }
     }
 }
diff --git a/tests/functional/Extension/Attributes/data/special_attributes.html b/tests/functional/Extension/Attributes/data/special_attributes.html
@@ -9,3 +9,6 @@ <h2 class="main shine" id="the-site">The Site</h2>
 <p>some } brackets</p>
 <p>some { } brackets</p>
 <p>A link inside of an emphasis tag: <em><a target="_blank" href="http://url.com" rel="noopener noreferrer">link</a></em>.</p>
+<p>Attributes without quote and non-whitespace char <a target="_blank" href="http://url.com" rel="noopener noreferrer">link</a></p>
+<p>Attributes without quote and non-whitespace char and a dot <a target="_blank" href="http://url.com" rel="noopener noreferrer">link</a>.</p>
+<p>Multiple attributes without quote and non-whitespace char and a dot <a class="class" id="id" target="_blank" href="http://url.com" rel="noopener noreferrer">link</a>.</p>
diff --git a/tests/functional/Extension/Attributes/data/special_attributes.md b/tests/functional/Extension/Attributes/data/special_attributes.md
@@ -1,7 +1,7 @@
 Header 1            {#header1}
 ========
 
-## Header 2 ##      
+## Header 2 ##
 {#header2}
 
 ## The Site    {.main}
@@ -22,3 +22,9 @@ some } brackets
 some { } brackets
 
 A link inside of an emphasis tag: *[link](http://url.com){target="_blank"}*.
+
+Attributes without quote and non-whitespace char [link](http://url.com){target=_blank}
+
+Attributes without quote and non-whitespace char and a dot [link](http://url.com){target=_blank}.
+
+Multiple attributes without quote and non-whitespace char and a dot [link](http://url.com){#id .class target=_blank}.
diff --git a/tests/unit/Extension/Attributes/Util/AttributesHelperTest.php b/tests/unit/Extension/Attributes/Util/AttributesHelperTest.php
@@ -29,19 +29,20 @@ final class AttributesHelperTest extends TestCase
      *
      * @param array<string, mixed> $expectedResult
      */
-    public function testParseAttributes(Cursor $input, array $expectedResult): void
+    public function testParseAttributes(Cursor $input, array $expectedResult, string $expectedRemainder = ''): void
     {
         $this->assertSame($expectedResult, AttributesHelper::parseAttributes($input));
+        $this->assertSame($expectedRemainder, $input->getRemainder());
     }
 
     /**
      * @return iterable<Cursor|array<string, mixed>>
      */
     public function dataForTestParseAttributes(): iterable
     {
-        yield [new Cursor(''), []];
-        yield [new Cursor('{}'), []];
-        yield [new Cursor('{ }'), []];
+        yield [new Cursor(''), [], ''];
+        yield [new Cursor('{}'), [], '{}'];
+        yield [new Cursor('{ }'), [], '{ }'];
 
         // Examples with colons
         yield [new Cursor('{:title="My Title"}'), ['title' => 'My Title']];
@@ -53,6 +54,10 @@ public function dataForTestParseAttributes(): iterable
         yield [new Cursor('{: #custom-id #another-id }'), ['id' => 'another-id']];
         yield [new Cursor('{: .class1 .class2 }'), ['class' => 'class1 class2']];
         yield [new Cursor('{: #custom-id .class1 .class2 title="My Title" }'), ['id' => 'custom-id', 'class' => 'class1 class2', 'title' => 'My Title']];
+        yield [new Cursor('{:target=_blank}'), ['target' => '_blank']];
+        yield [new Cursor('{: target=_blank}'), ['target' => '_blank']];
+        yield [new Cursor('{: target=_blank }'), ['target' => '_blank']];
+        yield [new Cursor('{:   target=_blank   }'), ['target' => '_blank']];
 
         // Examples without colons
         yield [new Cursor('{title="My Title"}'), ['title' => 'My Title']];
@@ -64,6 +69,10 @@ public function dataForTestParseAttributes(): iterable
         yield [new Cursor('{ #custom-id #another-id }'), ['id' => 'another-id']];
         yield [new Cursor('{ .class1 .class2 }'), ['class' => 'class1 class2']];
         yield [new Cursor('{ #custom-id .class1 .class2 title="My Title" }'), ['id' => 'custom-id', 'class' => 'class1 class2', 'title' => 'My Title']];
+        yield [new Cursor('{target=_blank}'), ['target' => '_blank']];
+        yield [new Cursor('{ target=_blank}'), ['target' => '_blank']];
+        yield [new Cursor('{target=_blank }'), ['target' => '_blank']];
+        yield [new Cursor('{   target=_blank   }'), ['target' => '_blank']];
 
         // Stuff at the beginning
         yield [new Cursor(' {: #custom-id }'), ['id' => 'custom-id']];
@@ -75,19 +84,21 @@ public function dataForTestParseAttributes(): iterable
         yield [new Cursor('      {: #custom-id }'), ['id' => 'custom-id']];
 
         // Stuff on the end
-        yield [new Cursor('{: #custom-id } '), ['id' => 'custom-id']];
+        yield [new Cursor('{: #custom-id } '), ['id' => 'custom-id'], ' '];
 
         // Note that this method doesn't abort if non-attribute things are found at the end - that should be checked elsewhere
-        yield [new Cursor('{: #custom-id } foo'), ['id' => 'custom-id']];
+        yield [new Cursor('{: #custom-id } foo'), ['id' => 'custom-id'], ' foo'];
+        yield [new Cursor('{: #custom-id }.'), ['id' => 'custom-id'], '.'];
 
         // Missing curly brace on end
-        yield [new Cursor('{: #custom-id'), []];
+        yield [new Cursor('{: #custom-id'), [], '{: #custom-id'];
 
         // Two sets of attributes in one string - we stop after the first one
-        yield [new Cursor('{: #id1 } {: #id2 }'), ['id' => 'id1']];
+        yield [new Cursor('{: #id1 } {: #id2 }'), ['id' => 'id1'], ' {: #id2 }'];
 
         // Curly braces inside of values
         yield [new Cursor('{: data-json="{1,2,3}" }'), ['data-json' => '{1,2,3}']];
+        yield [new Cursor('{data-json={1,2,3}} test'), ['data-json' => '{1,2,3}'], ' test'];
     }
 
     /**