Merge pull request #150 from lucasMesquitaBorges/fix-removing-trailin…

…g-slashes Fix removing trailing slashes
renatomarinho · Jun 1, 2021 · 6550a9d · 6550a9d
2 parents 5c807f0 + 22118ff
commit 6550a9d
Show file tree

Hide file tree

Showing 5 changed files with 83 additions and 19 deletions.
diff --git a/src/Entities/HtmlSpecs.php b/src/Entities/HtmlSpecs.php
@@ -0,0 +1,26 @@
+<?php
+
+namespace RenatoMarinho\LaravelPageSpeed\Entities;
+
+class HtmlSpecs
+{
+    public static function voidElements(): array
+    {
+        return [
+            'area',
+            'base',
+            'br',
+            'col',
+            'embed',
+            'hr',
+            'img',
+            'input',
+            'link',
+            'meta',
+            'param',
+            'source',
+            'track',
+            'wbr',
+        ];
+    }
+}
diff --git a/src/Middleware/PageSpeed.php b/src/Middleware/PageSpeed.php
@@ -3,6 +3,7 @@
 namespace RenatoMarinho\LaravelPageSpeed\Middleware;
 
 use Closure;
+use RenatoMarinho\LaravelPageSpeed\Entities\HtmlSpecs;
 use Symfony\Component\HttpFoundation\StreamedResponse;
 use Symfony\Component\HttpFoundation\BinaryFileResponse;
 
@@ -109,10 +110,26 @@ protected function shouldProcessPageSpeed($request, $response)
      */
     protected function matchAllHtmlTag(array $tags, string $buffer): array
     {
-        $tags = '('.implode('|', $tags).')';
+        $voidTags = array_intersect($tags, HtmlSpecs::voidElements());
+        $normalTags = array_diff($tags, $voidTags);
 
-        preg_match_all("/\<\s*{$tags}[^>]*\>((.|\n)*?)\<\s*\/\s*{$tags}\>/", $buffer, $matches);
-        return $matches;
+        return array_merge(
+            $this->matchTags($voidTags, '/\<\s*(%tags)[^>]*\>/', $buffer),
+            $this->matchTags($normalTags, '/\<\s*(%tags)[^>]*\>((.|\n)*?)\<\s*\/\s*(%tags)\>/', $buffer)
+        );
+    }
+
+    protected function matchTags(array $tags, string $pattern, string $buffer): array
+    {
+        if (empty($tags)) {
+            return [];
+        }
+
+        $normalizedPattern = str_replace('%tags', implode('|', $tags), $pattern);
+
+        preg_match_all($normalizedPattern, $buffer, $matches);
+
+        return $matches[0];
     }
 
     /**
@@ -127,12 +144,11 @@ protected function matchAllHtmlTag(array $tags, string $buffer): array
      */
     protected function replaceInsideHtmlTags(array $tags, string $regex, string $replace, string $buffer): string
     {
-        foreach ($this->matchAllHtmlTag($tags, $buffer)[0] as $tagMatched) {
-            preg_match_all($regex, $tagMatched, $tagContentsMatchedToReplace);
+        foreach ($this->matchAllHtmlTag($tags, $buffer) as $tagMatched) {
+            preg_match_all($regex, $tagMatched, $contentsMatched);
 
-            foreach ($tagContentsMatchedToReplace[0] as $tagContentReplace) {
-                $buffer = str_replace($tagContentReplace, $replace, $buffer);
-            }
+            $tagAfterReplace = str_replace($contentsMatched[0], $replace, $tagMatched);
+            $buffer = str_replace($tagMatched, $tagAfterReplace, $buffer);
         }
 
         return $buffer;

diff --git a/src/Middleware/RemoveQuotes.php b/src/Middleware/RemoveQuotes.php
@@ -2,10 +2,14 @@
 
 namespace RenatoMarinho\LaravelPageSpeed\Middleware;
 
+use RenatoMarinho\LaravelPageSpeed\Entities\HtmlSpecs;
+
 class RemoveQuotes extends PageSpeed
 {
     public function apply($buffer)
     {
+        $buffer = $this->replaceInsideHtmlTags(HtmlSpecs::voidElements(), '/\/>/', '>', $buffer);
+
         $replace = [
             '/ src="(.\S*?)"/' => ' src=$1',
             '/ width="(.\S*?)"/' => ' width=$1',
@@ -16,7 +20,6 @@ public function apply($buffer)
             '/ border="(.\S*?)"/' => ' border=$1',
             '/ crossorigin="(.\S*?)"/' => ' crossorigin=$1',
             '/ type="(.\S*?)"/' => ' type=$1',
-            '/\/>/' => '>',
         ];
 
         return $this->replace($replace, $buffer);

diff --git a/tests/Boilerplate/index.html b/tests/Boilerplate/index.html
@@ -41,6 +41,12 @@
         <div style="background-image: url('img/test-bg.jpg');">
             <h1>Test Background Image</h1>
         </div>
+
+        <svg width="325" height="200" xmlns="http://www.w3.org/2000/svg">
+            <path d="M 80 80 A 45 45, 0, 0, 0, 125 125 L 125 80 Z" fill="green"/>
+            <path d="M 230 80 A 45 45, 0, 1, 0, 275 125 L 275 80 Z" fill="red"/>
+        </svg>
+
         <img src="http://emblemsbf.com/img/18346.jpg" width="250" style="height:300px; padding:10px" />
 
         <img src="tile whitespace.png" width="250" style="height:300px; padding:10px"/>

diff --git a/tests/Middleware/RemoveQuotesTest.php b/tests/Middleware/RemoveQuotesTest.php
@@ -7,22 +7,35 @@
 
 class RemoveQuotesTest extends TestCase
 {
+    protected $response;
+
+    public function setUp(): void
+    {
+        parent::setUp();
+
+        $this->response = $this->middleware->handle($this->request, $this->getNext());
+    }
+
     protected function getMiddleware()
     {
         $this->middleware = new RemoveQuotes();
     }
 
     public function testRemoveQuotes()
     {
-        $response = $this->middleware->handle($this->request, $this->getNext());
-
-        $this->assertStringContainsString('<link rel="apple-touch-icon" href="icon.png">', $response->getContent());
-        $this->assertStringContainsString('<meta charset=utf-8>', $response->getContent());
-        $this->assertStringContainsString('<meta name=viewport content="width=device-width, initial-scale=1">', $response->getContent());
-        $this->assertStringContainsString('<img src=http://emblemsbf.com/img/18346.jpg width=250 style="height:300px; padding:10px" >', $response->getContent());
-        $this->assertStringContainsString('<img src=/images/1000coin.png>', $response->getContent());
-        $this->assertStringContainsString('<vue-component :src="\'src\'" :type="\'type\'" :width="200"></vue-component>', $response->getContent());
-        $this->assertStringContainsString('<img src="tile whitespace.png" width=250 style="height:300px; padding:10px">', $response->getContent());
-        $this->assertStringContainsString('<input type=text name="name with spaces" value="name with spaces" width=100%>', $response->getContent());
+        $this->assertStringContainsString('<link rel="apple-touch-icon" href="icon.png">', $this->response->getContent());
+        $this->assertStringContainsString('<meta charset=utf-8>', $this->response->getContent());
+        $this->assertStringContainsString('<meta name=viewport content="width=device-width, initial-scale=1">', $this->response->getContent());
+        $this->assertStringContainsString('<img src=http://emblemsbf.com/img/18346.jpg width=250 style="height:300px; padding:10px" >', $this->response->getContent());
+        $this->assertStringContainsString('<img src=/images/1000coin.png>', $this->response->getContent());
+        $this->assertStringContainsString('<vue-component :src="\'src\'" :type="\'type\'" :width="200"></vue-component>', $this->response->getContent());
+        $this->assertStringContainsString('<img src="tile whitespace.png" width=250 style="height:300px; padding:10px">', $this->response->getContent());
+        $this->assertStringContainsString('<input type=text name="name with spaces" value="name with spaces" width=100%>', $this->response->getContent());
+    }
+
+    public function testWontRemoveTrailingSlashesOfNonVoidElements()
+    {
+        $this->assertStringContainsString('<path d="M 80 80 A 45 45, 0, 0, 0, 125 125 L 125 80 Z" fill="green"/>', $this->response->getContent());
+        $this->assertStringContainsString('<path d="M 230 80 A 45 45, 0, 1, 0, 275 125 L 275 80 Z" fill="red"/>', $this->response->getContent());
     }
 }