Hey!

I think@BradJonesLLC has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

Link bug#45755

@nicolas-grekas Any idea about this? Thanks a lot in advance. Best regards

I don't think changing a superglobal to accommodate some code is the correct approach.
Can you figure out another way that doesn't involve mutating$_COOKIE?

Thanks for your feedback. I understand your concern. I will try to find a better way!

@nicolas-grekas I propose a new way that does not involve mutating a superglobal: I use the native PHP functions to generate a new session ID. Therefore, session_start will work normally.

Best regards

NB: the Appveyor build failed with "Build execution time has reached the maximum allowed time for your plan (20 minutes)."

I don't think this failure is relevant. I could not find how to retry the build.

@nicolas-grekas thanks for your review, I applied the requested changes.

I will attempt to write a test.

Best regards

@nicolas-grekas I added a test. It fails without my patch and passes with it.

Best regards

Thank you@peter17.

After that update we got a problem in one of our projects regenerating the session on each page view. The new implemented regex fails each time because the newly created value in the session cookie is "wrong" too.

The basic problem comes from the nelmio security plugin (signed_cookie feature). This feature extends the session cookie with a "." and an additional hash value. Especially the "." causes the regex to fail.

I will create a ticket in the nelmio project - but leave this comment for others with similar problems

@Flo-JB thanks for reporting this and for opening the issue in NelmioSecurityBundle

What I don't understand is that the current issue was about avoiding an exception whensession_start() is called with an invalid session id. Does this mean that previously, with NelmioSecurityBundle, the exception was thrown by Symfony and catched by NelmioSecurityBundle?

I personally didn't notice any Symfony exception before.

With the signd_cookie-feature enabled our session cookies get modified to s.th. like that:
4snfl14u0ooups9ifmhd9cjsk9.1ffb65204121792f16e1a5f3cc3f2cb6e5806157a508dc6df9c07a3d4c181b45

I didn't delve deeper in Nelmios code but I think they just append this verification hash after the session was started (In our case with a valid session_id) and put the modified value in the session cookie.

Your check fails right now because you take the whole cookie contents (sessionid + separator + hash) without separating the session id out from that mixed value.

This also causes issues for Drupal fwiw -https://www.drupal.org/project/drupal/issues/3285696

I'm not sure that the premise of this issue about which characters are allowed in a session ID or indeed used in a session ID is correct. Seehttps://www.php.net/manual/en/function.session-id.php it says:

Depending on the session handler, not all characters are allowed within the session id. For example, the file session handler only allows characters in the range a-z A-Z 0-9 , (comma) and - (minus)!

So we've limited the characters to the file session handler but we might not be using that.

This gets even more interesting when the session ID as a cookie value - the legal characters there are, according tohttps://curl.se/rfc/cookie_spec.html, do not include a comma!