Actually fix GH-9583

[Girgias]

The issue is that PS(mod)->s_validate_sid is always defined for user modules, thus we need to check that the actual callable is set Add another regression test to ensure current working behaviour is not broken (which was by the previous incorrect fix)

[cmb69]

That actually looks correct. While I don't like to refer to mod_user_names in session.c, that ship has obviously sailed long ago. However, the implementation of session_regenerate_id() might also need that hack:

php-src/ext/session/session.c

Lines 2250 to 2270 in 8e1cef4

	PS(id) = PS(mod)->s_create_sid(&PS(mod_data));
	if (!PS(id)) {
	PS(session_status) = php_session_none;
	if (!EG(exception)) {
	zend_throw_error(NULL, "Failed to create new session ID: %s (path: %s)", PS(mod)->s_name, PS(save_path));
	}
	RETURN_THROWS();
	}
	if (PS(use_strict_mode) && PS(mod)->s_validate_sid &&
	PS(mod)->s_validate_sid(&PS(mod_data), PS(id)) == SUCCESS) {
	zend_string_release_ex(PS(id), 0);
	PS(id) = PS(mod)->s_create_sid(&PS(mod_data));
	if (!PS(id)) {
	PS(mod)->s_close(&PS(mod_data));
	PS(session_status) = php_session_none;
	if (!EG(exception)) {
	zend_throw_error(NULL, "Failed to create session ID by collision: %s (path: %s)", PS(mod)->s_name, PS(save_path));
	}
	RETURN_THROWS();
	}
	}

Otherwise we would always create the session ID twice for mod_user.

[cmb69]

Oh, and please delete the comment /* This is not used yet */ just above the definition of PHP_FUNCTION(session_create_id) – obviously it is used now.

[Girgias]

That actually looks correct. While I don't like to refer to mod_user_names in session.c, that ship has obviously sailed long ago. However, the implementation of session_regenerate_id() might also need that hack:

php-src/ext/session/session.c

Lines 2250 to 2270 in 8e1cef4

	PS(id) = PS(mod)->s_create_sid(&PS(mod_data));
	if (!PS(id)) {
	PS(session_status) = php_session_none;
	if (!EG(exception)) {
	zend_throw_error(NULL, "Failed to create new session ID: %s (path: %s)", PS(mod)->s_name, PS(save_path));
	}
	RETURN_THROWS();
	}
	if (PS(use_strict_mode) && PS(mod)->s_validate_sid &&
	PS(mod)->s_validate_sid(&PS(mod_data), PS(id)) == SUCCESS) {
	zend_string_release_ex(PS(id), 0);
	PS(id) = PS(mod)->s_create_sid(&PS(mod_data));
	if (!PS(id)) {
	PS(mod)->s_close(&PS(mod_data));
	PS(session_status) = php_session_none;
	if (!EG(exception)) {
	zend_throw_error(NULL, "Failed to create session ID by collision: %s (path: %s)", PS(mod)->s_name, PS(save_path));
	}
	RETURN_THROWS();
	}
	}

Otherwise we would always create the session ID twice for mod_user.

I'm very confused by the whole check here... at it seems to be kinda weirdly implemented? I tried to fix it let me know what you think.

[cmb69]

Yes, looks good to me. Thank you!