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

Prevent session from starting during WordPress pseudo-cron procedures #210

Merged
merged 3 commits into from
Jul 18, 2020

Conversation

christianwach
Copy link
Member

@christianwach christianwach commented Jul 18, 2020

Overview

Fixes this issue on Lab.
There is a linked PR on CiviCRM Core.

Before

PHP warnings are written to the logs.

After

No PHP warnings are written to the logs.

Technical Details

There are a number of routes in WordPress that do not require sessions. This PR adds a check for the WordPress pseudo-cron route and skips starting the session in that situation.

totten and others added 3 commits July 2, 2020 21:36
This is a refinement of the idea in civicrm#135 to address other CLI use-cases.

Before
-------

If I have a `wpmaster` site on on `bknix-max` (php73), and if I run `cv upgrade:db -vv`, then there are several warnings like this:

```
PHP Warning:  session_start(): Cannot send session cookie - headers already sent by (output started at phar:///Users/totten/bknix/bin/cv/.box/src/Printer.php:83) in /Users/totten/bknix/build/wpmaster/web/wp-content/plugins/civicrm/civicrm.php on line 371
PHP Stack trace:
PHP   1. {main}() /Users/totten/bknix/bin/cv:0
PHP   2. require() /Users/totten/bknix/bin/cv:14
PHP   3. Civi\Cv\Application::main() phar:///Users/totten/bknix/bin/cv/bin/cv:27
PHP   4. Civi\Cv\Application->run() phar:///Users/totten/bknix/bin/cv/src/Application.php:15
PHP   5. Civi\Cv\Application->doRun() phar:///Users/totten/bknix/bin/cv/vendor/symfony/console/Application.php:124
PHP   6. Civi\Cv\Application->doRun() phar:///Users/totten/bknix/bin/cv/src/Application.php:46
PHP   7. Civi\Cv\Application->doRunCommand() phar:///Users/totten/bknix/bin/cv/vendor/symfony/console/Application.php:193
PHP   8. Civi\Cv\Command\UpgradeDbCommand->run() phar:///Users/totten/bknix/bin/cv/vendor/symfony/console/Application.php:850
PHP   9. Civi\Cv\Command\UpgradeDbCommand->execute() phar:///Users/totten/bknix/bin/cv/vendor/symfony/console/Command/Command.php:257
PHP  10. Civi\Cv\Command\UpgradeDbCommand->boot() phar:///Users/totten/bknix/bin/cv/src/Command/UpgradeDbCommand.php:39
PHP  11. call_user_func:{phar:///Users/totten/bknix/bin/cv/src/Util/BootTrait.php:39}() phar:///Users/totten/bknix/bin/cv/src/Util/BootTrait.php:39
PHP  12. Civi\Cv\Command\UpgradeDbCommand->_boot_full() phar:///Users/totten/bknix/bin/cv/src/Util/BootTrait.php:39
PHP  13. CRM_Utils_System::loadBootStrap() phar:///Users/totten/bknix/bin/cv/src/Util/BootTrait.php:99
PHP  14. CRM_Utils_System_WordPress->loadBootStrap() /Users/totten/bknix/build/wpmaster/web/wp-content/plugins/civicrm/civicrm/CRM/Utils/System.php:1521
PHP  15. require_once() /Users/totten/bknix/build/wpmaster/web/wp-content/plugins/civicrm/civicrm/CRM/Utils/System/WordPress.php:600
PHP  16. require_once() /Users/totten/bknix/build/wpmaster/web/wp-load.php:37
PHP  17. require_once() /Users/totten/bknix/build/wpmaster/web/wp-config.php:97
PHP  18. do_action() /Users/totten/bknix/build/wpmaster/web/wp-settings.php:392
PHP  19. WP_Hook->do_action() /Users/totten/bknix/build/wpmaster/web/wp-includes/plugin.php:478
PHP  20. WP_Hook->apply_filters() /Users/totten/bknix/build/wpmaster/web/wp-includes/class-wp-hook.php:312
PHP  21. CiviCRM_For_WordPress->setup_instance() /Users/totten/bknix/build/wpmaster/web/wp-includes/class-wp-hook.php:288
PHP  22. session_start() /Users/totten/bknix/build/wpmaster/web/wp-content/plugins/civicrm/civicrm.php:371
```

After
-----

It doesn't try to start session in CLI (where sessions don't make sense).
Do not start session on any CLI processes
@kcristiano
Copy link
Member

jenkins test this please

@kcristiano kcristiano changed the base branch from master to 5.28 July 18, 2020 13:54
@kcristiano
Copy link
Member

Did an r-run

session start errors are gone.

Ra through multiple contribution forms to ensure session was not broken on return from processing. PayPal Standard, Stripe and test processor used.

Works as expected.

@kcristiano kcristiano merged commit 3868182 into civicrm:5.28 Jul 18, 2020
@christianwach christianwach deleted the lab-wp-64 branch July 19, 2020 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants