Session::regenerateSessionId may cause warnings

[lekoala]

Affected Version

4.5.3

Description

Recently found this in my logs in production:

error-log.WARNING: E_WARNING: session_regenerate_id(): Cannot regenerate session id - session is not active {"code":2,"message":"session_regenerate_id(): Cannot regenerate session id - session is not active","file":".../vendor/silverstripe/framework/src/Control/Session.php","line":652}

This happens when sending password reset links

Steps to Reproduce

Get a password reset link, open an incognito window, paste the reset link (in my case, it's hidden by a tracking link from the mail api, so it triggers a redirect) : you get the warning

It may well be due to the incognito window freshly opened or the redirection from the mail api. In any case, it shouldn't happen

Possible ways to solve it:

• prepend @ in front of the call (like in vendor\silverstripe\framework\src\Security\MemberAuthenticator\SessionAuthenticationHandler.php)
• check if session is active, something like this seem to work fine for me in my current hotfix

if (!headers_sent() && session_status() == PHP_SESSION_ACTIVE) {
  session_regenerate_id(true);
}

[kinglozzer]

Gah, we’ve already fixed this once (#9259), obviously not properly :(

Fancy submitting a PR @lekoala?

[dhensby]

@kinglozzer your changes are in the 4.5.3 tag (https://github.com/silverstripe/silverstripe-framework/blob/4.5.3/src/Control/Session.php) - so I guess it means there's still an edge case to be dealt with?

[kinglozzer]

Yeah I think I fixed one trigger for this bug, but not the root cause

[lekoala]

Fancy submitting a PR @lekoala?

PR done! #9499

[kinglozzer]

PR merged so this can be closed :D