session_regenerate_id multiple calls to create_sid and validateId

[shimscharf]

The following code:

<?php
class sessionLogger extends \SessionHandler{

	static public function debug ($val, $color = 'red')
	{
		echo "<div style='margin-left:300px; color:$color'>" . $val . '</div>';
	}
	
	public function create_sid (): string
	{	
		$sid = parent::create_sid();
		self::debug ('create_sid(): ' . $sid);
		return $sid;
	}
	
	public function open (string $path, string $session_name): bool
	{
		self::debug('Open(): ' . $session_name );
		return parent::open($path, $session_name);
	}
	
	public function read (string $session_id): string|false
	{
		self::debug ('Read(): ' . $session_id );
		$data = parent::read($session_id);
		return $data;
	}
	
	public function write (string $session_id, string $data): bool
	{
		self::debug ('Write(): ' . $session_id );
		return parent::write($session_id, $data);
	}
	
	public function close (): bool
	{
		self::debug('Close(): ' . session_id());
		return parent::close();
	}
	
	public function destroy (string $session_id): bool
	{
		self::debug ('Destroy(): ' . $session_id );
		return parent::destroy($session_id);
	}
	
	public function gc (int $max_life): int|false
	{
		self::debug ('Garbage Collection()' );
		return parent::gc($max_life);
	}
	
}

class customHandler extends sessionLogger implements SessionHandlerInterface{

	static public function debug ($val, $color = 'blue')
	{
		echo "<div style='margin-left:200px; color:$color'>" . $val . '</div>';
	}

	public function create_sid (): string
	{
		self::debug ('creating_sid ....');
		$sid = parent::create_sid();
		return $sid;
	}
	
	public function validateId (string $id): bool
	{
		self::debug ('validateID (' . $id . ')');
		return true;
	}
	
}



ini_set('session.use_strict_mode','1');
$handler = new customHandler;
session_set_save_handler ($handler , true);

echo '<p>session_start()</p>';
session_start();

echo '<p>session_regenerate_id(false)</p>';
session_regenerate_id(false);

echo '<p>end session</p>';

Resulted in this output:

session_start()
Open(): PHPSESSID
validateID (98danmo2v49m5a62b5tcb123ve)
Read(): 98danmo2v49m5a62b5tcb123ve

session_regenerate_id(false)
Write(): 98danmo2v49m5a62b5tcb123ve
Close(): 98danmo2v49m5a62b5tcb123ve
Open(): PHPSESSID
creating_sid ....
create_sid(): ko2kru80k50euoqjoutj0mlvm8
validateID (ko2kru80k50euoqjoutj0mlvm8)
creating_sid ....
create_sid(): 3vbdr6c18dl0u14qb9eelf27q2
validateID (3vbdr6c18dl0u14qb9eelf27q2)
creating_sid ....
create_sid(): iucfevbluanu07plr3p03msnls
validateID (iucfevbluanu07plr3p03msnls)
creating_sid ....
create_sid(): 2ch44nesf7ufh9tdahovuva52b
Read(): 2ch44nesf7ufh9tdahovuva52b

end session
Write(): 2ch44nesf7ufh9tdahovuva52b
Close(): 2ch44nesf7ufh9tdahovuva52b

But I expected this output instead:

session_start()
Open(): PHPSESSID
validateID (2ch44nesf7ufh9tdahovuva52b)
creating_sid ....
create_sid(): gunkvau9dptt1p6olj3pfiu3sh
Read(): gunkvau9dptt1p6olj3pfiu3sh

session_regenerate_id(false)
Write(): gunkvau9dptt1p6olj3pfiu3sh
Close(): gunkvau9dptt1p6olj3pfiu3sh
Open(): PHPSESSID
creating_sid ....
create_sid(): i8lufpuan3au3mhl1c1q61lcjs
validateID (i8lufpuan3au3mhl1c1q61lcjs)
Read(): i8lufpuan3au3mhl1c1q61lcjs

end session
Write(): i8lufpuan3au3mhl1c1q61lcjs
Close(): i8lufpuan3au3mhl1c1q61lcjs

Seems as though validateID and create_sid are called up to 3 times (as in the source code on line 2300:

php-src/ext/session/session.c

Line 2300 in 1fb40b5

while (limit-- && PS(mod)->s_validate_sid(&PS(mod_data), PS(id)) == SUCCESS) {

I'm pretty sure the test should be for FAILURE rather than SUCCESS. Keep trying if failed ....
If I return false from validateId function it runs as expected.

	public function validateId (string $id): bool
	{
		self::debug ('validateID (' . $id . ')');
		return false;;
	}

PHP Version

PHP 8.1.12

Operating System

Windows 11

[shimscharf]

Upon deeper inspection it looks like it got implemented originally in PHP 7.4.5 branch.

b510250

This fix is correct for session_create_id function, but NOT for session_regenerate_id where it was incorrectly applied.

The reason is that within session_create_id we are acting if a collision was detected, to reset value, then continue, whilst within the session_regenerate_id function we need to keep looping as long as the generated id is NOT valid.

[shimscharf]

An even shorter code example:

class customHandler extends \SessionHandler implements SessionHandlerInterface{

	static public function debug ($val, $color = 'blue')
	{
		echo "<div style='margin-left:200px; color:$color'>" . $val . '</div>';
	}

	public function create_sid (): string
	{
		self::debug ('creating_sid ....');
		$sid = parent::create_sid();
		return $sid;
	}

	public function validateId (string $id): bool
	{
		self::debug ('validateID (' . $id . ')');
		return true;
	}
	
}

ini_set('session.use_strict_mode','1');
$handler = new customHandler;
session_set_save_handler ($handler , true);

echo '<p>session_start()</p>';
session_start();

echo '<p>session_regenerate_id(false)</p>';
session_regenerate_id(false);

[shimscharf]

I was blinded by validateID wording --> it is supposed to test if session already exists, not if the ID generated is valid

[Girgias]

Sorry for not coming back sooner, I was planing to have a look at it tomorrow. But yes the wording of the method is extremely confusing. :-/