"session_id(): Cannot change session id"

Same here. I will take a closer look at this and get back to you here if I come with anything.

I am getting the same error. Thanks for any help.

Ok I did some research on this and I have a few things to report:

1) The latest version of drupal core (dev) does not generate the specific error (at least not on its own)

2) There are 2 variations of the error for me:

Warning: session_id(): Cannot change session id when headers already sent in drupal_session_initialize() (line 266 of /var/www/devevlopment/includes/session.inc).

and

Warning: session_set_save_handler(): Cannot change save handler when headers already sent in drupal_session_initialize() (line 242 of /var/www/development/includes/session.inc).

This warning usually means that there is something wrong with the application's code: some part of the code tried to do something with the session (change its id or handler respectively) after the session was created. This used to fail in pre PHP 7.2 but silently. Which means that the code was problematic but we were not able to see it.

Here is where it gets interesting though: the code that created the session and the code that is trying to change it is in the same file (includes/session.inc in the core). But even more interesting is the fact that drupal itself does not do it (tested it in a clean Drupal installation and everything).

After multiple trials and errors in the enviromnent where the error presented, I finally managed to isolate it to the submodule: actions_permissions of VBO. I will do some further research on this and possibly submit a patch to VBO for it (I don't think that changing the code in session.inc to check for the session is a proper solution as it will mask potential problems in the code of modules).

There have been a number of fixes relating to PHP 7.2 that have been committed to 7.x-dev but are not in 7.60; they should be in 7.61 when that is released (which looks like it should be later today). See https://groups.drupal.org/node/534483 and #2947772: Fully support PHP 7.2 in Drupal 7 for some more information on this.

It is still possible (indeed likely) that some individual contrib modules have issues with PHP 7.2.

@longwave this is not your typical "each()" problem that's why I tested it against the dev version and not 7.60. Drupal's dev core seems absolutely fine with PHP 7.2. This issue may be hiding an actual security risk for VBO or whichever module is trying to alter the session hence I would not think that it's a good idea to "patch" core in order to test whether there is a session before doing the session_id() or the session_set_save_handler(). I'm currently trying to find the offending piece of code and will report back here (or/and by creating a new issue in VBO) with my findings.

This does happen in tests within PHP 7.3, but not 7.2 with 7.x.

Moving over to PHP 7.3

Within our test runner the following error was reported related to sessions:

drupal_session_regenerate
exception: [Warning] Line 375 of includes/session.inc:
session_id(): Cannot change session id when session is active

exception: [Warning] Line 375 of includes/session.inc:
session_id(): Cannot change session id when session is active

(https://www.drupal.org/pift-ci-job/1115582)

Hi all,

I've been working on this for a couple of days without luck. Either I break the Session tests, either the User tests.
I'm unable to find a proper patch yet.

If there are some people that are willing to help, and I hope there are...here's a simple tool that might help you all, even those who do not have (or want to install) PHP 7.3.

You can find it at this URL: https://github.com/drupol/dockerdrupal7runnerngninxphpfpm

Basically, it's just a docker stack. With 3 commands, you can spawn a Drupal 7 site and work on it.

Let me know if it's helpful and if you need help to use it. The only requirement is docker.

Here are the commands that you need to do in order to get it on your local machine:

git clone https://github.com/drupol/dockerdrupal7runnerngninxphpfpm.git
cd dockerdrupal7runnerngninxphpfpm
docker-compose up -d
docker-compose exec -u www-data php composer install
docker-compose exec -u www-data php ./vendor/bin/run drupal:site-install

Let me know how it goes and... Let's fix this thing!

Drupal 8.7 branch is working on a fix, and from what I see, they have come up with a fix (albeit drastic changes). I will try to work on a fix, but it's unlikely that we will have a proper fix for this, tested them well, and drafted a release before December 3rd.

This patch resolves the PHP 7.3.0 warning message in drupal_session_regenerate() when a user with a pre-existing unauthenticated session logs in.

Nice!

Updating the patch. It includes small optimisations.

Oh I forgot to mention...

For people willing to test Drupal 7 with PHP 7.3 without installing anything, feel free to test Drupal 7 with PHP 7.3 using this small custom project: https://github.com/drupol/dockerdrupal7runnerngninxphpfpm

The only requirement is Docker.

Let me know how it goes.

@Pol I'm not sure why you want to make so many changes to session.inc, it's like a hornets nest made out of jenga blocks, which are made out of hacky PHP code :)

We can't do what you're doing because the insecure session ID needs to be different from the secure session ID.

Hi,

There are not so many changes, I'm just simplifying the code, reducing duplicates function calls and in the end, it's more readable.

Regarding the differents IDs, you're right, I will update it, it's weird that there is no tests that covers that (yet?)

This doesn't work - it's setting identical secure and insecure session cookies.

I don't think a patch to fix a bug re: PHP 7.3 is a place to refactor the code - that could happen in a separate patch.

Hello,

I think we should indeed fix the issue in one single commit (your patch is perfectly fine for that) and then only we can optimize the function a bit like the patch I propose.

Some progress ... almost there!

See the log: https://pastebin.com/9SLzf0mW

I'm having the OpenID module failing, I need to investigate, but I think it's related to the NGinX server configuration.

Besides that, I'm still investigating why the MySQL server has gone away, I'll keep you updated.

Want to help ? See this project: https://github.com/drupol/dockerdrupal7runnerngninxphpfpm and feel free to test and improve :)

Same issue with the refactor patch (using the same session ID for secure and insecure sessions); I'm undisplaying it as it would be good to get some reviews on the bugfix patch.

Updating issue summary as this issue is apparently being used for the new PHP 7.3 bug rather than the originally reported issue?

The bug my patch addresses is due to new behavior in PHP 7.3. If you have a problem with "session_id(): Cannot change session id" in earlier PHP versions it would be some other bug.

The original issue was "After moving from PHP 5.6 to PHP 7.2, this warning shows up in my logs (admin/reports/dblog):

Warning: session_id(): Cannot change session id when headers already sent in drupal_session_initialize()
(Line 266 of ... /includes/session.inc).

I couldn't find anything on this other than an issue in Durpal 8. Did anyone else run into this?"

Yes, my client too has tons of these errors after moving from PHP 5.6 to PHP 7.2., same line, 266 etc.

@BrianLP and @kari.kaariainen: Sorry that I hijacked the old issue... But over at https://www.drupal.org/project/drupal/issues/3012308#comment-12907469 @sjerdo asked that my patch be moved here, and @Fabianx had put [PHP 7.3] in the title, so I assumed it was being repurposed.

Your original issue re: PHP 7.2 could be a problem in custom or contributed code, or because you're using an old version of something. You will have to debug what is sending headers before drupal_session_initialize() runs, which happens pretty early in bootstrap. Headers shouldn't be sent until the next bootstrap step, DRUPAL_BOOTSTRAP_PAGE_HEADER. If you provide steps to reproduce after installing current version of Drupal 7, then other folks could help you investigate.

@BrianLP You can use this project and change the PHP version in this file to quickly spawn a Drupal with the desired PHP version.

"On my installation, almost every single click triggers these two warnings, resulting in a constant stream of error messages." Same for me.

@BrianLP No, they don't have Commerce. I also couldn't reproduce the errors using the same contrib modules and a fresh Drupal on an other server, so maybe it's the ad blocks or templates or something else. I'll try to figure it out.

EDIt: No, it wasn't the theme or anything else I could test now. I'll have to come back to this later. I hope it's not BOM (byte order mark) :)

The patch changes the strict necessary to fix the issue, it's fine for me, moving this forward.

Okay, moving this back to the PHP 7.2 scope -- sorry for the confusion

I think this should be "maintainer needs more info" as we don't have steps to reproduce.

I am also having these problems with Drupal 7 and php 7.2
The patch didnt help (#38). It caused new error logs.

Info: I uninstalled VBO but still having these issues.

Unless I'm mistaken, in Drupal 7, drupal_bootstrap runs _drupal_bootstrap_page_cache() which always sends out headers and then drupal_session_initialize() gets called which calls session_set_save_handler and session_id so every bootstrap call will generate this warning.

Did I miss something? How is this handled in Drupal 8?

@KiranJoshi Wouldn't that mean that every Drupal 7 generates the warnings? That is not the case.

I was able to reproduce this by enabling page cache, enabling statistics module access log, and as anonymous user loading cached pages that are longer than the length of the PHP output buffer, or of any length if PHP output_buffering is disabled. In this case, headers will have been already sent and then session will be initialized in statistics module hook_exit().

Disabling statistics module access log stops the flow of warnings for me.

I've never used statistics module so I'm not sure why it needs to generate a "fake" session ID when recording a view of a cached page that doesn't otherwise have a session.

Let's see what tests fail if we remove the session initialization here.

I was able to reproduce this on a test site.

@KiranJoshi I believe headers are sent all at once. So this warning isn't triggered unless the headers have finished sending, which could happen by calling flush() or by sending some output (more than the output buffer, or any amount if output buffering is disabled or you flush the output buffer).

We are using #48 on a client's live site without any problems. Even the Popular content block, that the Statistics module provides, seems to be working fine.

I believe this is because PHP session is controlled by the PHP server. If a script called a PHP Script running at different PHP server and you must use session, then you must use session_set_save_handler(). But, using session_set_save_handler() is slower and consume lots of traffic thus may create security issues. To return the session control to the different PHP server, you must create a new session_id.
Drupal must have been using this method to allow running scripts located at different PHP servers.

Apparently, since PHP 7.3 they are not allowing this anymore and produce warning. Usually a warning will become a fatal error in the future release. I am not aware of PHP's solution to this issue.

@Burhan I don't follow your comment, but I am pretty sure we have a solution (patch #48) it just needs to be reviewed.

patch #48 is working ok.

I am still getting the two messages for every anonymous user to hit the site in my prod environment even after applying patch #48.

Warning: session_id(): Cannot change session id when headers already sent in drupal_session_initialize() (line 266 of .../drupal7/includes/session.inc).
Warning: session_set_save_handler(): Cannot change save handler when headers already sent in drupal_session_initialize() (line 242 of .../drupal7/includes/session.inc).

I applied the patch #48 above to both a dev and then a prod site. I also applied https://www.drupal.org/files/issues/2019-01-11/3025335-3.patch. I increased PHP output_buffering to 8192. The dev site works without the errors (but unlike the prod site, does not have advagg or memcache, but not using it for sessions). The prod site still kicks off messages. Yes, I restarted everything, even rebooted (out of desperation).

I am using Debian 9 and Drupal 7.65, with PHP-FPM 7.2.16 - PHP7.2.16-1+0~20190307202415.17+stretch~1.gbpa7be82, with NGINX 1.10.3, everything up to date and patched.

@gorlov you will need to get a backtrace from inside drupal_session_initialize() and see what is calling it after the headers were sent.

You could also grep your code and see if anything is calling drupal_bootstrap (either DRUPAL_BOOTSTRAP_SESSION or a later bootstrap phase, which would bootstrap session automatically) or drupal_session_initialize, but getting a backtrace is probably easier.

(I suppose it's also possible you have some code causing headers to be sent too early?)

Using #48 on several production sites. Thank you!!

@mfb thanks for the insight.
The grep worked fine and identified the "better_statistics" module that calls drupal_bootstrap(DRUPAL_BOOTSTRAP_SESSION); Disabling the module stopped the errors. Odd that my dev site also had better statistics enabled but was not throwing off the errors. If I have time I will investigate why the dev site was not throwing the errors.

Submitted the same patch to that module :)

Updating credits. Moving this forward.

Thanks for the feedback all.

Okay, so the idea for the fix is good - however it will fail under the following circumstances [not the right variable names, but explaining the behavior instead]:

- page_cache_without_database = TRUE
- page_cache_execute_exit_hooks = TRUE

That means on page cache hits the response is send from the page cache (e.g. memcache), then statistics_exit() is executed.

Now when statistics tries to execute db_insert() it will fail.

As you can also see the db_insert() uses a session_id() for logging the access:

    db_insert('accesslog')
      ->fields(array(
        'title' => truncate_utf8(strip_tags(drupal_get_title()), 255),
        'path' => truncate_utf8($_GET['q'], 255),
        'url' => isset($_SERVER['HTTP_REFERER']) ? $_SERVER['HTTP_REFERER'] : '',
        'hostname' => ip_address(),
        'uid' => $user->uid,
        'sid' => session_id(),
        'timer' => (int) timer_read('page'),
        'timestamp' => REQUEST_TIME,
      ))
      ->execute();

So that is the reason for bootstrapping till SESSION.

Maybe it would be possible to bootstrap till database and then check if there is a session and if yes, output session_id and if not, not?

Unless I misremember the starting of the session would mean the ID changes everytime anyway [as the session header cannot be send to the anon user], so the field is meaningless.

Thanks for your valuable comment @fabian, we will update the patch asap.

As far as I can tell, it's fine as-is.

statistics_exit() is already bootstrapping to variables, so database is fully functional.

Inserting session_id() is fine - this will insert a session ID if there is one, or empty string otherwise.

Yep, I missed the earlier call to bootstrap to variables.

Good as is.

Fixed! Thanks all.

Does this mean that updating to 7.67 will fix this issue? Sorry for the rookie question.

Does this mean that updating to 7.67 will fix this issue? Sorry for the rookie question.

Doesn't feel like such a rookie question, especially given that updating to Drupal 7.67 did not resolve this problem; sites where this error crops up with PHP 7.2 or higher still need the patch. I think that's because 7.67 was a security release and these sorts of bug fixes were omitted, perhaps?

Yeah, It's not fixed for me. We updated to 7.67 and hope to see these errors no more. We aren't supposed to patch Core correct???

I don't think this commit was part of the 7.67 release.

marking it for 7.68

Thanks @joseph.oldstad! Looking forward to seeing this in the 7.68 release.

Yeah, It's not fixed for me. We updated to 7.67 and hope to see these errors no more. We aren't supposed to patch Core correct???

@ToneLoc, the usual saying is "don't hack core;" patching core doesn't feel quite like hacking core, especially when the patch will soon make it into a core release! (I'm applying the patch in a few spots myself.)

The last releases (7.67, 7.66, and 7.65) were all releases to fix security vulnerabilities. Since their descriptions say No other fixes are included. none of them contained the changes introduced in this issue.

I hope it shows up in the next non-security release.

@ToneLoc We are thankful to having drush make and have 27 patches to drupal 7 core and I don't think we're unusual. Adding the patch_status module we can see on the available updates report what patches are applied to core and contrib modules and the related issue queues.

PS. Sorry if this seems off topic to some people but you may as well learn something more while you wait. We are all rookies at some time :-)

@ToneLoc ,
here is what a drush make file with patches for 'core' looks like.
as mentioned by izmeez , there is a module called patch_status that when correctly configured can show which patches are applied to core.

However, my goal isn't to patch core, my goal is to improve core for everyone, not just myself.

There is a new core maintainer @mcdruid , he and @FabianX are going to take the bull by the horns (with the support of the community).

Adding a note that this made it into 7.68, so one less patch in our core update workflow.

https://www.drupal.org/project/drupal/releases/7.68

Yeah!

Any updates on this? Updated core 7.69 and having Php 7.3.6 running and still having this issue. It spams my log files!

@silentbob see #61 for debugging instructions.

@silentbob , I am willing to guess that your build procedure probably is erroneous or that you did not upgrade your core correctly to 7.69.

is it possible that your build procedure reversed the fix?

do a diff of the related include files between 7.69 vanilla and your environments include files, see if they are different.

@oseph.olstad I made a diff and unfortunately no difference. Update was made with "drush up drupal"

Memcache?

@silentbob It is in the 7.69 release. The patch is simply the deletion of two lines. Take a look at the file and see if the lines have been deleted.

diff --git a/modules/statistics/statistics.module b/modules/statistics/statistics.module
index 356a0e2513..7db0237a9f 100644
--- a/modules/statistics/statistics.module
+++ b/modules/statistics/statistics.module
@@ -74,8 +74,6 @@ function statistics_exit() {
     }
   }
   if (variable_get('statistics_enable_access_log', 0)) {
-    drupal_bootstrap(DRUPAL_BOOTSTRAP_SESSION);
-
     // For anonymous users unicode.inc will not have been loaded.
     include_once DRUPAL_ROOT . '/includes/unicode.inc';
     // Log this page access.

@izmeez this how this function is looking on my site in statistics.module. the two lines are not there

function statistics_exit() {
  global $user;

  // When serving cached pages with the 'page_cache_without_database'
  // configuration, system variables need to be loaded. This is a major
  // performance decrease for non-database page caches, but with Statistics
  // module, it is likely to also have 'statistics_enable_access_log' enabled,
  // in which case we need to bootstrap to the session phase anyway.
  drupal_bootstrap(DRUPAL_BOOTSTRAP_VARIABLES);

  if (variable_get('statistics_count_content_views', 0) && !variable_get('statistics_count_content_views_ajax', 0)) {
    // We are counting content views.
    if (arg(0) == 'node' && is_numeric(arg(1)) && arg(2) == NULL) {
      // A node has been viewed, so update the node's counters.
      db_merge('node_counter')
        ->key(array('nid' => arg(1)))
        ->fields(array(
          'daycount' => 1,
          'totalcount' => 1,
          'timestamp' => REQUEST_TIME,
        ))
        ->expression('daycount', 'daycount + 1')
        ->expression('totalcount', 'totalcount + 1')
        ->execute();
    }
  }
  if (variable_get('statistics_enable_access_log', 0)) {
    // For anonymous users unicode.inc will not have been loaded.
    include_once DRUPAL_ROOT . '/includes/unicode.inc';
    // Log this page access.
    db_insert('accesslog')
      ->fields(array(
        'title' => truncate_utf8(strip_tags(drupal_get_title()), 255),
        'path' => truncate_utf8($_GET['q'], 255),
        'url' => isset($_SERVER['HTTP_REFERER']) ? $_SERVER['HTTP_REFERER'] : '',
        'hostname' => ip_address(),
        'uid' => $user->uid,
        'sid' => session_id(),
        'timer' => (int) timer_read('page'),
        'timestamp' => REQUEST_TIME,
      ))
      ->execute();
  }
}

@silentbob It doesn't look like this is your issue. The previous suggestion that you look at comment #61 might help as it did as reported in comment #64. You may have another module that needs the line removed
- drupal_bootstrap(DRUPAL_BOOTSTRAP_SESSION);

@izmeez I found this line also in better_statistics.module
do you think its safe to delete ?

/**
 * Implements hook_exit().
 *
 * Gathers additional data and inserts our data into accesslog.
 */
function better_statistics_exit() {
  // If statistics access log is set to run, run our code.
  if (variable_get('statistics_enable_access_log', 0)) {

    // Force a recheck on cache status, just in case this function was called
    // before the X-Drupal-Cache header was set.
    $cache_status = better_statistics_served_from_cache(TRUE);

    // @see statistics_exit()
    drupal_bootstrap(DRUPAL_BOOTSTRAP_SESSION);
    include_once DRUPAL_ROOT . '/includes/unicode.inc';

    // Load all relevant module.statistics.inc files for hook invocations.
    better_statistics_load_active_incs();

    // Allow all modules to react before logging begins.
    module_invoke_all('better_statistics_prelog');

    // Log the request if the request is loggable.
    if (better_statistics_request_is_loggable()) {
      // Get all declared fields and their computed values.
      $fields = better_statistics_get_fields_data();

      // Allow modules to log statistics data.
      module_invoke_all('better_statistics_log', $fields);
    }
  }

if found the issue is same here: https://www.drupal.org/project/better_statistics/issues/3043654

Yes, that's the one referred to in comment #64 as related. Maybe that is the issue you are having.

@izmeez Thanks a lot. Now my logs look much cleaner!

@silentbob That's good, so this issue is truly closed - may it rest in peace.