session.lazy_write only implemented if user defines custom handler with updateTimestamp method - Incorrect behavior

[shimscharf]

Description

The following code:

php-src/ext/session/session.c

Lines 515 to 522 in 21e01f3

	if (PS(lazy_write) && PS(session_vars)
	&& PS(mod)->s_update_timestamp
	&& PS(mod)->s_update_timestamp != php_session_update_timestamp
	&& zend_string_equals(val, PS(session_vars))
	) {
	ret = PS(mod)->s_update_timestamp(&PS(mod_data), PS(id), val, PS(gc_maxlifetime));
	handler_function_name = handler_class_name != NULL ? "updateTimestamp" : "update_timestamp";
	} else {

Will not call updateTimestamp unless PS(mod)->s_update_timestamp exists & is different to the inbuilt updateTimestamp function (php_session_update_timestamp defined in

php-src/ext/session/session.c

Line 1105 in 21e01f3

PHPAPI zend_result php_session_update_timestamp(PS_UPDATE_TIMESTAMP_ARGS) {

). It will always call the standard inbuilt file write handler instead.

Given that session.lazy_write defaults to 1 --> True, the expectation is that the session file will not be overwritten if there isn't a change in session value. But this will always happen unless there is a defined custom handler.

PHP Version

PHP 8.2

Operating System

Ubuntu 22.04 LTS

[devnexen]

Thanks for your report I ll try to see if this is justified ASAP.

[devnexen]

While your analysis makes sense (and is true), to me it qualifies more as a feature rather than bug, it might cause an unwarranted behaviour break/change if we change from PHP 8.2 and it had been like this since ~8 years.

[shimscharf]

But the reality is that we are actually just cheating the public --> the expectation as per the php.ini setting session.lazy_write which defaults to 1 (true), is that this will actually not write every time.
It does write every time, but no one has tested it properly to date somehow. Everyone who doesn't understand, just assumes it works for them as expected, whilst everyone who writes their own custom handler, takes care of it themselves usually.

[shimscharf]

I honestly think, implementing it correctly, where it actually only updates the timestamp. can only improve performance, without any detriment.
This was discussed during the implementation discussions:
https://wiki.php.net/rfc/session-read_only-lazy_write

There's no reason for this feature to be optional, as explained above - it is a performance improvement and the patch already calls write() if updateTimestamp() doesn't exist. If this RFC is accepted, it would make even less sense to keep it optional. Worst thing that could happen is to fallback to the old behavior where session data is written at all times.

It was caught and implemented correctly in phpredis:

phpredis/phpredis#2079

[shimscharf]

At the very least it's a huge bug in the documentation and expectation

[devnexen]

huge bug might be a bit much but documentation can deserve a bit more clarification indeed.