symfony http-foundation changes and drush_print()

[douggreen]

Symfony's latest release checks for headers_sent() before creating a session. PHP print's and thus drush_print() cause this check to fail, and exceptions to be thrown. The Symfony commit that causes this change is symfony/http-foundation@ef7a168

I have errors on site-install. The #drush slack channel also mentions errors in drush updb from this.

Starting Drupal installation. This takes a while. Consider using the [ok]
--notify global option.
RuntimeException: Failed to set the session handler because headers  [error]
have already been sent by
"/path/to/drupal/vendor/drush/drush/includes/output.inc"
at line 41. in
/path/to/drupal/vendor/symfony/http-foundation/Session/Storage/NativeSessionStorage.php:395
Stack trace:
#0
/path/to/drupal/vendor/symfony/http-foundation/Session/Storage/NativeSessionStorage.php(120):
Symfony\Component\HttpFoundation\Session\Storage\NativeSessionStorage->setSaveHandler(Object(Drupal\Core\Session\WriteSafeSessionHandler))
#1
/path/to/drupaldocroot/core/lib/Drupal/Core/Session/SessionManager.php(91):
Symfony\Component\HttpFoundation\Session\Storage\NativeSessionStorage->__construct(Array,
Object(Drupal\Core\Session\WriteSafeSessionHandler),
Object(Drupal\Core\Session\MetadataBag))
#2
/path/to/drupaldocroot/core/lib/Drupal/Component/DependencyInjection/Container.php(284):
Drupal\Core\Session\SessionManager->__construct(Object(Symfony\Component\HttpFoundation\RequestStack),
Object(Drupal\Core\Database\Driver\mysql\Connection),
Object(Drupal\Core\Session\MetadataBag),
Object(Drupal\Core\Session\SessionConfiguration),
Object(Drupal\Core\Session\WriteSafeSessionHandler))
#3
/path/to/drupaldocroot/core/lib/Drupal/Component/DependencyInjection/Container.php(177):
Drupal\Component\DependencyInjection\Container->createService(Array,
'session_manager')
#4
/path/to/drupaldocroot/core/lib/Drupal/Component/DependencyInjection/Container.php(494):
Drupal\Component\DependencyInjection\Container->get('session_manager',
1)
#5
/path/to/drupaldocroot/core/lib/Drupal/Component/DependencyInjection/Container.php(236):
Drupal\Component\DependencyInjection\Container->resolveServicesAndParameters(Array)
#6
/path/to/drupaldocroot/core/lib/Drupal/Component/DependencyInjection/Container.php(177):
Drupal\Component\DependencyInjection\Container->createService(Array,
'session')
#7
/path/to/drupaldocroot/core/lib/Drupal.php(158):
Drupal\Component\DependencyInjection\Container->get('session')
#8
/path/to/drupaldocroot/core/includes/install.core.inc(1509):
Drupal::service('session')
#9
/path/to/drupaldocroot/core/includes/install.core.inc(672):
install_bootstrap_full(Array)
#10
/path/to/drupaldocroot/core/includes/install.core.inc(550):
install_run_task(Array, Array)
#11
/path/to/drupaldocroot/core/includes/install.core.inc(121):
install_run_tasks(Array)
#12
/path/to/drupal/vendor/drush/drush/includes/drush.inc(726):
install_drupal(Object(Composer\Autoload\ClassLoader), Array)
#13
/path/to/drupal/vendor/drush/drush/includes/drush.inc(711):
drush_call_user_func_array('install_drupal', Array)
#14
/path/to/drupal/vendor/drush/drush/commands/core/drupal/site_install.inc(82):
drush_op('install_drupal', Object(Composer\Autoload\ClassLoader),
Array)
#15
/path/to/drupal/vendor/drush/drush/commands/core/site_install.drush.inc(255):
drush_core_site_install_version('mysite_profil...', Array)
#16
/path/to/drupal/vendor/drush/drush/includes/command.inc(422):
drush_core_site_install('mysite_profil...')
#17
/path/to/drupal/vendor/drush/drush/includes/command.inc(231):
_drush_invoke_hooks(Array, Array)
#18
/path/to/drupal/vendor/drush/drush/includes/command.inc(199):
drush_command('mysite_profil...')
#19
/path/to/drupal/vendor/drush/drush/lib/Drush/Boot/BaseBoot.php(67):
drush_dispatch(Array)
#20
/path/to/drupal/vendor/drush/drush/includes/preflight.inc(66):
Drush\Boot\BaseBoot->bootstrap_and_dispatch()
#21
/path/to/drupal/vendor/drush/drush/drush.php(12):
drush_main()
#22 {main}

I don't know the right fix.

1. should you not use sessions in the cli, so should drupal avoid all session calls from the cli?
2. if you are going to use sessions in the cli, should the session object check if it’s the cli?
3. should we just fix drush to not write trigger this by outputting in a way that php doesn’t think the session has been written to?

I'm suggesting option 3 here in drush, which might be the biggest hack, but is the easiest solution. There are 9.x changes to drush_print() that have since been back out. Has this been looked at before? See https://github.com/drush-ops/drush/blame/master/includes/output.inc#L45

I have a different solution below, that fixes the Symfony http-foundation issue. I didn't create a proper PR yet, was hoping for some discussion.

-- drush/includes/output.bak   2017-11-13 11:41:14.000000000 -0500
+++ drush/includes/output.inc   2017-11-13 11:41:35.000000000 -0500
@@ -34,12 +34,10 @@ function drush_print($message = '', $ind
   if (($charset = drush_get_option('output_charset')) && function_exists('iconv')) {
     $msg = iconv('UTF-8', $charset, $msg);
   }
-  if (isset($handle)) {
-    fwrite($handle, $msg);
-  }
-  else {
-    print $msg;
+  if (!$handle) {
+    $handle = STDOUT;
   }
+  fwrite($handle, $msg);
 }
 
 /**

[greg-1-anderson]

It is unclear to me from the PHP documentation when the headers set via the header method are actually sent. From your experiences above, it appears that print sends all queued headers, but fwrite does not. Is that your experience? Do you know if this behavior is documented anywhere? It seems like it would ideally be the case that the CLI version of PHP should never send headers, and headers_sent() should always return false. Regrettably, this appears to not be the case. If we can avoid header problems by using fwrite consistently in Drush, then we should make that change to the Drush 8.x and master branches.

In the meantime, please consider using webflo/drupal-core-strict to avoid using new dependency versions before the Drupal core / Drush maintainers have had a chance to resolve problems.

[douggreen]

I don't know what is documented. This is what I'm seeing:

$ php --version
PHP 7.1.8 (cli) (built: Aug 17 2017 11:34:56) ( NTS )
Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.1.0, Copyright (c) 1998-2017 Zend Technologies
    with Xdebug v2.5.0, Copyright (c) 2002-2016, by Derick Rethans
$ drush ev 'print "headers_sent=" . headers_sent() . "\n"; print "headers_sent=" . headers_sent() . "\n";'
headers_sent=
headers_sent=1
$ drush ev 'fwrite(STDOUT, "headers_sent=" . headers_sent() . "\n"); print "headers_sent=" . headers_sent() . "\n";'
headers_sent=
headers_sent=

[greg-1-anderson]

Seems like a good indication that we should replace print with fwrite everywhere.

[cosmicdreams]

I have received this error in using both php 7.1 and php 5.6

[douggreen]

I'm not sure if @cosmicdreams is saying that http-foundation error is also broken in 5.6 (which I'd expect it to be) or if the proposed solution works in 5.6 (which I just tested, that it does):

$ php --version
PHP 5.6.31 (cli) (built: Aug 17 2017 11:33:10) 
Copyright (c) 1997-2016 The PHP Group
Zend Engine v2.6.0, Copyright (c) 1998-2016 Zend Technologies
$ drush ev 'print "headers_sent=" . headers_sent() . "\n"; print "headers_sent=" . headers_sent() . "\n";'
headers_sent=
headers_sent=1
$ drush ev 'fwrite(STDOUT, "headers_sent=" . headers_sent() . "\n"); print "headers_sent=" . headers_sent() . "\n";'
headers_sent=
headers_sent=

[douggreen]

See #3169

[weitzman]

I think this is fixed now?

[deviantintegral]

I'm not seeing this fix on the 8.x branch.

[greg-1-anderson]

No, I don't think this was back-ported. It would be a good idea to do so.

[m4olivei]

I found the trail of PR's hard to follow. Eventually I found this, which appears to be the active PR to backport this to the 8.x branch: #3171