Skip to content

Commit

Permalink
Merge pull request #83 from creative-commoners/pulls/3.0/emtpy-data
Browse files Browse the repository at this point in the history
FIX Return an emtpy string instead of false from HybridSession::read()
  • Loading branch information
GuySartorelli authored Feb 15, 2023
2 parents 773e9b0 + 7aede67 commit 1b6897d
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 6 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ multi-server issues like asset storage and databases).

## Installation

* Install with composer using `composer require silverstripe/hybridsessions:*`
* Install with composer using `composer require silverstripe/hybridsessions`
* /dev/build?flush=all to setup the necessary tables
* In order to initiate the session handler is is necessary to add a snippet of code to your
\_config.php, along with a private key used to encrypt user cookies.

in `mysite/_config.php`
in `app/src/_config.php`

```php
// Ensure that you define a sufficiently indeterminable value for SS_SESSION_KEY in your `.env`
Expand Down
14 changes: 11 additions & 3 deletions src/HybridSession.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@ public function open(string $save_path, string $name): bool
foreach ($this->getHandlers() as $handler) {
$handler->open($save_path, $name);
}

// 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.
return true;
}

Expand All @@ -74,8 +77,13 @@ public function read(string $session_id): string|false
return $data;
}
}

return false;
// Return a blank string if no record was found in any store
// The "session" still exists in memory until a new record is 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 '';
}

public function write(string $session_id, string $session_data): bool
Expand Down
2 changes: 1 addition & 1 deletion tests/HybridSessionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public function testReadReturnsEmptyStringWithNoHandlers()
{
$this->handler->expects($this->once())->method('read')->with('foosession')->willReturn(false);
$this->instance->setHandlers([$this->handler]);
$this->assertFalse($this->instance->read('foosession'));
$this->assertSame('', $this->instance->read('foosession'));
}

public function testReadReturnsHandlerDelegateResult()
Expand Down

0 comments on commit 1b6897d

Please sign in to comment.