From bd61d6261bf257b4f8b22195d0581e8f3c7e4bcd Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Tue, 26 Jan 2021 09:15:25 +0100 Subject: [PATCH 1/2] keep previous cookies we recently fixed that we lose previous cookies. but the fix was keeping the messages in the session and only setting the cookie on the last redirect, which means the messages are lost if the session is destroyed (e.g. redirect to page outside firewall) changed to merge the flash messages cookie from requests into the flashes. and using array_merge_recursive to keep multiple messages with the same key. --- CHANGELOG.md | 11 ++++++++++ src/EventListener/FlashMessageListener.php | 20 ++++++++++--------- .../FlashMessageListenerTest.php | 11 ++-------- 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c7cd9e2a..e80d1e85 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,17 @@ Changelog ========= +2.9.2 +----- + +### Fixed + +* 2.9.1 fixed overwriting flash messages on multiple redirects, but introduced + a risk to lose flash messages when redirecting to a path that is outside the + firewall or destroys the session. + This version hopefully fixes both cases. Existing flash messages in a request + cookie are merged with new flash messages from the session. + 2.9.1 ----- diff --git a/src/EventListener/FlashMessageListener.php b/src/EventListener/FlashMessageListener.php index 336157ac..1347a319 100644 --- a/src/EventListener/FlashMessageListener.php +++ b/src/EventListener/FlashMessageListener.php @@ -87,14 +87,6 @@ public function onKernelResponse(FlashMessageResponseEvent $event) return; } - $response = $event->getResponse(); - - // If the response is a redirect, we should wait until the final response - // is reached - if ($response->isRedirect()) { - return; - } - $flashBag = $this->session->getFlashBag(); $flashes = $flashBag->all(); @@ -102,11 +94,21 @@ public function onKernelResponse(FlashMessageResponseEvent $event) return; } + $response = $event->getResponse(); + $cookies = $response->headers->getCookies(ResponseHeaderBag::COOKIES_ARRAY); $host = (null === $this->options['host']) ? '' : $this->options['host']; if (isset($cookies[$host][$this->options['path']][$this->options['name']])) { $rawCookie = $cookies[$host][$this->options['path']][$this->options['name']]->getValue(); - $flashes = array_merge($flashes, json_decode($rawCookie)); + $flashes = array_merge_recursive($flashes, json_decode($rawCookie, true)); + } + + // Preserve existing flash message cookie from previous redirect if there was one. + // This covers multiple redirects where each redirect adds flash messages. + $request = $event->getRequest(); + if ($request->cookies->has($this->options['name'])) { + $rawCookie = $request->cookies->get($this->options['name']); + $flashes = array_merge_recursive($flashes, json_decode($rawCookie, true)); } $cookie = new Cookie( diff --git a/tests/Functional/EventListener/FlashMessageListenerTest.php b/tests/Functional/EventListener/FlashMessageListenerTest.php index 799658c4..6eac1a51 100644 --- a/tests/Functional/EventListener/FlashMessageListenerTest.php +++ b/tests/Functional/EventListener/FlashMessageListenerTest.php @@ -13,7 +13,6 @@ use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; -use Symfony\Component\HttpFoundation\Cookie; class FlashMessageListenerTest extends WebTestCase { @@ -33,7 +32,6 @@ public function testFlashMessageCookieIsSet() $found = false; foreach ($cookies as $cookie) { - /** @var Cookie $cookie */ if ('flash_cookie_name' !== $cookie->getName()) { continue; } @@ -45,9 +43,7 @@ public function testFlashMessageCookieIsSet() $found = true; } - if (!$found) { - $this->fail('Cookie flash_cookie_name not found in the cookie response header: '.implode(',', $cookies)); - } + $this->assertTrue($found, 'Cookie "flash_cookie_name" not found in response cookies'); } public function testFlashMessageCookieIsSetOnRedirect() @@ -65,7 +61,6 @@ public function testFlashMessageCookieIsSetOnRedirect() $found = false; foreach ($cookies as $cookie) { - /** @var Cookie $cookie */ if ('flash_cookie_name' !== $cookie->getName()) { continue; } @@ -77,8 +72,6 @@ public function testFlashMessageCookieIsSetOnRedirect() $found = true; } - if (!$found) { - $this->fail('Cookie flash_cookie_name not found in the cookie response header: '.implode(',', $cookies)); - } + $this->assertTrue($found, 'Cookie "flash_cookie_name" not found in response cookies'); } } From 497086347ea620725395ea2a9d1194ce61ea1d9c Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Tue, 26 Jan 2021 09:17:17 +0100 Subject: [PATCH 2/2] backport github workflows to 2.9 branch --- .github/workflows/php.yml | 59 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 .github/workflows/php.yml diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml new file mode 100644 index 00000000..ac1159ee --- /dev/null +++ b/.github/workflows/php.yml @@ -0,0 +1,59 @@ +name: CI + +env: + SYMFONY_PHPUNIT_DIR: "$HOME/symfony-bridge/.phpunit" + +on: + push: + branches: + - master + pull_request: + +jobs: + build: + runs-on: ubuntu-latest + strategy: + matrix: + include: + # Test the latest stable release + - php-version: '7.2' + - php-version: '7.3' + - php-version: '7.4' + # Test Symfony LTS versions. Read more at https://github.com/symfony/lts + - php-version: '7.4' + dependencies: 'symfony/lts:^3' + - php-version: '7.4' + symfony-version: '^4' + - php-version: '7.4' + symfony-version: '^5' + # Test latest unreleased versions + - php-version: '7.4' + symfony-version: '^5' + stability: 'dev' + name: PHP ${{ matrix.php-version }} Test on Symfony ${{ matrix.symfony-version }} ${{ matrix.dependencies}} ${{ matrix.stability }} ${{ matrix.composer-flag }} + steps: + - name: Pull the code + uses: actions/checkout@v2 + - name: Install PHP and Composer + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php-version }} + tools: composer:v2 + - name: Check PHP Version + run: php -v + - name: Stability + run: composer config minimum-stability ${{ matrix.stability }} + if: ${{ matrix.stability }} + - name: Additional require + run: composer require --no-update ${{ matrix.dependencies }} + if: ${{ matrix.dependencies }} + - name: Symfony version + run: composer require --no-update symfony/flex && composer config extra.symfony.require ${{ matrix.symfony-version}} + if: ${{ matrix.symfony-version }} + - name: Composer update + run: composer update ${{ matrix.composer-flag }} --prefer-dist --no-interaction + - name: Composer validate + run: composer validate --strict --no-check-lock + - name: Run tests + run: php vendor/bin/simple-phpunit -v +