Browse Source

Merge pull request #14906 from ndm2/3.x-fix-output-buffers-not-being-closed

3.x - Fix output buffers not being closed when the view triggers an exception.
Mark Story 5 years ago
parent
commit
aa84771113

+ 25 - 2
src/View/View.php

@@ -30,6 +30,7 @@ use Cake\View\Exception\MissingElementException;
 use Cake\View\Exception\MissingHelperException;
 use Cake\View\Exception\MissingLayoutException;
 use Cake\View\Exception\MissingTemplateException;
+use Exception;
 use InvalidArgumentException;
 use LogicException;
 use RuntimeException;
@@ -814,8 +815,20 @@ class View implements EventDispatcherInterface
         if ($result) {
             return $result;
         }
+
+        $bufferLevel = ob_get_level();
         ob_start();
-        $block();
+
+        try {
+            $block();
+        } catch (Exception $exception) {
+            while (ob_get_level() > $bufferLevel) {
+                ob_end_clean();
+            }
+
+            throw $exception;
+        }
+
         $result = ob_get_clean();
 
         Cache::write($options['key'], $result, $options['config']);
@@ -1414,9 +1427,19 @@ class View implements EventDispatcherInterface
     protected function _evaluate($viewFile, $dataForView)
     {
         extract($dataForView);
+
+        $bufferLevel = ob_get_level();
         ob_start();
 
-        include func_get_arg(0);
+        try {
+            include func_get_arg(0);
+        } catch (Exception $exception) {
+            while (ob_get_level() > $bufferLevel) {
+                ob_end_clean();
+            }
+
+            throw $exception;
+        }
 
         return ob_get_clean();
     }

+ 61 - 4
tests/TestCase/View/ViewTest.php

@@ -1881,7 +1881,6 @@ TEXT;
             $this->View->render('extend_self');
             $this->fail('No exception');
         } catch (\LogicException $e) {
-            ob_end_clean();
             $this->assertContains('cannot have views extend themselves', $e->getMessage());
         }
     }
@@ -1898,7 +1897,6 @@ TEXT;
             $this->View->render('extend_loop');
             $this->fail('No exception');
         } catch (\LogicException $e) {
-            ob_end_clean();
             $this->assertContains('cannot have views extend in a loop', $e->getMessage());
         }
     }
@@ -1954,8 +1952,6 @@ TEXT;
             $this->View->render('extend_missing_element');
             $this->fail('No exception');
         } catch (\LogicException $e) {
-            ob_end_clean();
-            ob_end_clean();
             $this->assertContains('element', $e->getMessage());
         }
     }
@@ -1996,6 +1992,67 @@ TEXT;
     }
 
     /**
+     * Tests that the buffers that are opened when evaluating a template
+     * are being closed in case an exception happens.
+     *
+     * @return void
+     */
+    public function testBuffersOpenedDuringTemplateEvaluationAreBeingClosed()
+    {
+        $bufferLevel = ob_get_level();
+
+        $e = null;
+        try {
+            $this->View->element('exception_with_open_buffers');
+        } catch (\Exception $e) {
+        }
+
+        $this->assertNotNull($e);
+        $this->assertEquals('Exception with open buffers', $e->getMessage());
+        $this->assertEquals($bufferLevel, ob_get_level());
+    }
+
+    /**
+     * Tests that the buffers that are opened during block caching are
+     * being closed in case an exception happens.
+     *
+     * @return void
+     */
+    public function testBuffersOpenedDuringBlockCachingAreBeingClosed()
+    {
+        Cache::drop('test_view');
+        Cache::setConfig('test_view', [
+            'engine' => 'File',
+            'duration' => '+1 day',
+            'path' => CACHE . 'views/',
+            'prefix' => '',
+        ]);
+        Cache::clear(false, 'test_view');
+
+        $bufferLevel = ob_get_level();
+
+        $e = null;
+        try {
+            $this->View->cache(function () {
+                ob_start();
+
+                throw new \Exception('Exception with open buffers');
+            }, [
+                'key' => __FUNCTION__,
+                'config' => 'test_view',
+            ]);
+        } catch (\Exception $e) {
+        }
+
+        Cache::clear(false, 'test_view');
+        Cache::drop('test_view');
+
+        $this->assertNotNull($e);
+        $this->assertEquals('Exception with open buffers', $e->getMessage());
+        $this->assertEquals($bufferLevel, ob_get_level());
+    }
+
+    /**
      * Test memory leaks that existed in _paths at one point.
      *
      * @return void

+ 3 - 0
tests/test_app/TestApp/Template/Element/exception_with_open_buffers.ctp

@@ -0,0 +1,3 @@
+<?php
+$this->start('non closing block');
+throw new \Exception('Exception with open buffers');