dev/drupal#169 - Fix for session_id() change in Drupal 9.2

[demeritcowboy]

Overview

https://lab.civicrm.org/dev/drupal/-/issues/169

It's described well in the ticket but briefly Drupal 9.2 now tries to avoid making sessions for anonymous, so using session_id() is unreliable and can return the empty string.

Before

Clicking on a link to contribution page as anonymous if you're not already clicking around the site will then fail when you try to submit the form with "Your browser session has expired".

After

Ok

Technical Details

It's best described in the ticket and https://www.drupal.org/node/3006306, which is also where I stole the session id generation code from.

There are other uses of session_id() in core but I've left them out here, also as described in the ticket. They aren't important.

Comments

I haven't tested this on Drupal 8 - it's technically end-of-life but of course there might still be sites using it. However the functions involved all seem to be identical.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[seamuslee001]

This seems fine to me adding merge ready just pinging a few other Drupal folks who are likely to be interested here

@petednz @homotechsual @jackrabbithanna @KarinG

[petednz]

@stesi561 @jitendrapurohit any thoughts. this is urgent and jonG is asking it be in a points release

[demeritcowboy]

Yeah I should probably rebase this for backport (5.44 first). And if people are upgrading drupal 8 because of end-of-life then 9.2 would be what they'd upgrade to.

[seamuslee001]

@kcristiano are you able to test this on WordPress just to check there is no breakage there (shouldn't be)

[seamuslee001]

I tested this on a client staging site on 9.2 and that seemed to work fine and also on a local buildkit on 8.9,

[totten]

I'm +1 for backport to 5.43.2

[kcristiano]

@seamuslee001 I've done a number of r-run on WordPress with this Patch applied to 5.43.1. I realize now that it's targeted to 5.44 but I don't think that should make a difference.

Basic CiviCRM Contribution pages work as expected as an anonymous user. I also did a few tests on client staging sites with a mix of Contribution pages and WP forms using the API. They also worked as expected.

In all cases a session was set properly.

[KarinG]

What about backporting to ESR 5.39? Or perhaps most ESR sites are on D7 still?

[seamuslee001]

@KarinG I think that would be a sensible option but will leave that to the ESR manager @totten to manage but we should get a stable cut first.

Based on the comments here and the testing on the ticket I'm going to merge this and put up a stable backport PR now

[totten]

I'm not sure about the breakdown of CMS's among ESR users. My expectation is that ESR folks tend to hang-back a bit on all platform updates. But maybe this is coming up because Drupal 9.1 goes EOL on December 8 - creating an incentive for 9.2 or 9.3 upgrades in the next few weeks. And once Dec 8 hits, the natural combination will be Civi 5.39 and Drupal 9.2+.

So, yeah, I'm down with backporting to ESR - but maybe we let the patches gel a few more days on 5.44/5.43 first.