Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[Blueprints] Prevent WSOD when autologin is enabled and a plugin logs a notice #2079

Merged
merged 1 commit into from
Dec 17, 2024

Conversation

adamziel
Copy link
Collaborator

This PR ensures the autologin step won't cause a perpetual white screen of death when one of the plugins displays a notice.

The autologin flow consists of a bunch of checks, setcookie() call, a header('Location') call, and an exit;. However, when the headers are already sent, the cookies won't be set, the redirect won't work, but the script will still. This, effectively, prevents loading any WordPress page.

This PR adds a headers_sent() check to stop the autologin step early in those scenarios when we can't finish the login.

Testing instructions

  • CI has a new E2E test – confirm everything is green
  • Try this Blueprint (courtesy of @janw-me). It should open a white screen with errors. Now change the landing page to "/". It should open the homepage with a few errors at the top. Before this PR, you'd see a blank page instead of the homepage.
{
  "plugins": [
    "pronamic-ideal"
  ],
  "preferredVersions": {
    "php": "8.2",
    "wp": "6.7"
  },
  "features": {
    "networking": true
  },
  "login": true,
  "landingPage": "/wp-admin/update-core.php",
  "steps": [
    {
      "step": "defineWpConfigConsts",
      "consts": {
        "WP_DEBUG": true,
        "WP_DEBUG_DISPLAY": true
      }
    },
    {
      "step": "setSiteLanguage",
      "language": "nl_NL"
    }
  ]
}

… a notice

This PR ensures the autologin step won't cause a perpetual white screen
of death when one of the plugins displays a notice.

The autologin flow consists of a bunch of checks, setcookie() call, a
header('Location') call, and an `exit;`. However, when the headers are
already sent, the cookies won't be set, the redirect won't work, but
the script will still. This, effectively, prevents loading any WordPress
page.

This PR adds a `headers_sent()` check to stop the autologin step early
in those scenarios when we can't finish the login.

  ## Testing instructions

* CI has a new E2E test – confirm everything is green
* Try this Blueprint. It should open a white screen with errors. Now
  change the landing page to "/". It should open the homepage with a few
  errors at the top. Before this PR, you'd see a blank page instead of
  the homepage.

```json
{
  "plugins": [
    "pronamic-ideal"
  ],
  "preferredVersions": {
    "php": "8.2",
    "wp": "6.7"
  },
  "features": {
    "networking": true
  },
  "login": true,
  "landingPage": "/wp-admin/update-core.php",
  "steps": [
    {
      "step": "defineWpConfigConsts",
      "consts": {
        "WP_DEBUG": true,
        "WP_DEBUG_DISPLAY": true
      }
    },
    {
      "step": "setSiteLanguage",
      "language": "nl_NL"
    }
  ]
}

```
@adamziel adamziel added [Type] Bug An existing feature does not function as intended [Package][@wp-playground] Blueprints labels Dec 13, 2024
Copy link
Collaborator

@bgrgicak bgrgicak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

I observed this last week in a few plugins while running the top 1000 plugin compatibility tests.

@adamziel
Copy link
Collaborator Author

adamziel commented Dec 16, 2024

Thanks for the review! It seems like an important test for this is still failing, though. Let's fix it before merging.

@bgrgicak
Copy link
Collaborator

I reran the test and it works, I also tried it locally and it passed.

@adamziel adamziel merged commit 1bed672 into trunk Dec 17, 2024
10 checks passed
@adamziel adamziel deleted the prevent-autologin-white-screen-of-death branch December 17, 2024 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package][@wp-playground] Blueprints [Type] Bug An existing feature does not function as intended
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants