FIX Return an emtpy string instead of false from HybridSession::read()

[emteknetnz]

Issue #82

Reverts change from https://github.com/silverstripe/silverstripe-hybridsessions/pull/79/files#diff-ca4ddcb65cd9d12ea026a139173f0ece0c7ac066adb9b027c2f4f802bd42f936R78

[michalkleiner]

Did you mean ^3 (and the ^5 was because of CMS 5)?

Suggested change

	* Install with composer using `composer require silverstripe/hybridsessions:^5`
	* Install with composer using `composer require silverstripe/hybridsessions:^3`

[emteknetnz]

Have removed the version as per Guys review below

[michalkleiner]

Suggested change

	// The "session" still exists in memory until a new record will be created
	// The "session" still exists in memory until a new record is created

[emteknetnz]

Updated

[GuySartorelli]

Also add a comment above return true; in open() something along the lines of

Our interpretation of "or creates a new one" in https://www.php.net/manual/en/sessionhandlerinterface.open.php allows for the session to have been created in memory, to be stored by the appropriate handler on session_write_close(). If the session fails to be written, we return false in write(), so we will be alerted to there being some error.

Alternatively, we could return false from open() if all of the stores return false which would indicate that none of the stores are going to be able to write the session when the session ends. This would also require changing open() of the child handlers to return false if they won't be able to store stuff (db not available, and the checks that are in write() for the cookie handler)

Either one of these - a comment explaining our thinking and interpretation of the docs (or returning false if all handlers return false from open) will be fine.

[GuySartorelli]

Suggested change

	* Install with composer using `composer require silverstripe/hybridsessions:^5`
	* Install with composer using `composer require silverstripe/hybridsessions`

Composer knows what version is right. Specifying here just gives us another thing we have to change next major.

[emteknetnz]

Updated

[GuySartorelli]

Suggested change

	// Return a blank string if no record was found in any store
	// The "session" still exists in memory until a new record will be created
	// in the first writable store in write()
	// This prevents `PHP Warning: session_start(): Failed to read session data: user`
	// when session_start() is called within SilverStripe\Control\Session::start()
	return '';
	// Return a blank string if no record was found in any store
	// The "session" still exists in memory until a new record will be created
	// in the first writable store in write()
	// Our interpretation then of "If the record was not found" in https://www.php.net/manual/en/sessionhandlerinterface.read.php
	// is that we have "found" the NEW session in memory.
	// This prevents `PHP Warning: session_start(): Failed to read session data: user`
	// when session_start() is called within SilverStripe\Control\Session::start()
	return '';

[emteknetnz]

Updated

[emteknetnz]

@GuySartorelli I've added the comment to open() rather than changing return types