Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Slim 4.10.0 redirects with status code 200 causing blank page #3177

Closed
mjac opened this issue Mar 15, 2022 · 6 comments
Closed

Slim 4.10.0 redirects with status code 200 causing blank page #3177

mjac opened this issue Mar 15, 2022 · 6 comments

Comments

@mjac
Copy link

mjac commented Mar 15, 2022

Hi Slim Developers,

Since I upgraded to 4.10.0, my redirects had status code 200. This broke my site with blank pages everywhere but luckily I caught it testing on the staging server.

  - Upgrading slim/slim (4.9.0 => 4.10.0)

To fix the issue I now specify ->withStatus(302) when I do a redirect. See commented line below:

    public function redirectToPage(ResponseInterface $response, string $relativeUrl): ResponseInterface
    {
        $pageUrl = $this->urlFactory->domainUrl($relativeUrl);
        return $response->withHeader('Location', $pageUrl);  // ->withStatus(302); // required since Slim 4.10.0
    }

I'm sharing this as an issue in case other people experience this issue.

Thanks again for an excellent framework.

Note: here is my full list of upgrades in case it wasn't caused by Slim (I feel this is unlikely though as they are unrelated)

  - Upgrading cakephp/core (4.3.5 => 4.3.6)
  - Upgrading cakephp/database (4.3.5 => 4.3.6)
  - Upgrading cakephp/datasource (4.3.5 => 4.3.6)
  - Upgrading cakephp/utility (4.3.5 => 4.3.6)
  - Upgrading doctrine/instantiator (1.4.0 => 1.4.1)
  - Upgrading google/apiclient-services (v0.236.0 => v0.239.0)
  - Upgrading monolog/monolog (2.3.5 => 2.4.0)
  - Upgrading myclabs/deep-copy (1.10.2 => 1.11.0)
  - Upgrading phpmailer/phpmailer (v6.5.4 => v6.6.0)
  - Upgrading phpunit/php-code-coverage (9.2.13 => 9.2.15)
  - Upgrading phpunit/phpunit (9.5.16 => 9.5.18)
  - Upgrading sentry/sentry (3.3.7 => 3.4.0)
  - Upgrading slim/slim (4.9.0 => 4.10.0)
  - Upgrading symfony/http-client (v5.4.3 => v5.4.5)
  - Upgrading symfony/polyfill-ctype (v1.24.0 => v1.25.0)
  - Upgrading symfony/polyfill-mbstring (v1.24.0 => v1.25.0)
  - Upgrading symfony/polyfill-php73 (v1.24.0 => v1.25.0)
  - Upgrading symfony/polyfill-php80 (v1.24.0 => v1.25.0)
  - Upgrading symfony/polyfill-php81 (v1.24.0 => v1.25.0)
  - Upgrading symfony/polyfill-uuid (v1.24.0 => v1.25.0)
@mjac
Copy link
Author

mjac commented Mar 15, 2022

I downgraded to Slim 4.9.0 and that fixed the redirect issue on my local development environment.

@mjac
Copy link
Author

mjac commented Mar 15, 2022

To speed things up --

Caused by #1730

Commits 6aa522d and 1ma/jelly@c718132

Previous PRs #2309 #2345

@mjac
Copy link
Author

mjac commented Mar 15, 2022

Looks like you lost these tests from #2345

    public function testStatusIsSetTo302IfLocationIsSetWhenStatusis200()
    {
        $response = new Response();
        $response = $response->withHeader('Location', '/foo');

        $this->assertSame(302, $response->getStatusCode());
    }

    public function testStatusIsNotSetTo302IfLocationIsSetWhenStatusisNot200()
    {
        $response = new Response();
        $response = $response->withStatus(201)->withHeader('Location', '/foo');

        $this->assertSame(201, $response->getStatusCode());
    }

@mjac
Copy link
Author

mjac commented Mar 15, 2022

I will stick with 4.9.0 for now

@odan
Copy link
Contributor

odan commented Mar 15, 2022

The old behavior (before v4.10) was not correct because it was based on an implicit behavior of PHP. See here: #3139

@mjac
Copy link
Author

mjac commented Mar 15, 2022

I'm okay moving to the new version without the implicit 302, thanks for the link to the relevant PR. 👍

It might be worth mentioning with more prominence in the release notes as it could break a lot of production sites e.g. "The behaviour of all Location: redirects has changed you need to check all your redirects do not end up in a blank page". In my case login/logout etc were all broken and I only caught it because I was deep testing a feature.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants