tonyair/silverstripe-framework
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-framework synced 2024-10-22 14:05:37 +02:00
Code Issues Projects Releases Wiki Activity

Suggested improvements / test case fixes

Browse Source
This commit is contained in:
Damian Mooyman 2014-09-26 16:01:23 +12:00
parent 3b9056fc01
commit 1e612607aa
13 changed files with 233 additions and 163 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
_config/cookie.yml Normal file
View File

@ -0,0 +1,5 @@
---
Name: cookie
---
Injector:
Cookie_Backend: CookieJar

49
control/Cookie.php
View File

@ -19,13 +19,6 @@ class Cookie {
* @return Cookie_Backend The cookie backend
*/
public static function get_inst() {
//if we don't have a CookieJar service yet, register it
if(!Injector::inst()->hasService('Cookie_Backend')) {
Injector::inst()->registerService(
Injector::inst()->create('CookieJar', $_COOKIE),
'Cookie_Backend'
);
}
return Injector::inst()->get('Cookie_Backend');
}
@ -47,13 +40,25 @@ class Cookie {
}
/**
* Get a cookie variable.
* Get the cookie value by name
*
* @param string
* @return mixed
* @param string $name The name of the cookie to get
* @param boolean $includeUnsent Include cookies we've yet to send when fetching values
*
* @return string|null The cookie value or null if unset
*/
public static function get($name) {
return self::get_inst()->get($name);
public static function get($name, $includeUnsent = true) {
return self::get_inst()->get($name, $includeUnsent);
}
/**
* Get all the cookies
*
* @param boolean $includeUnsent Include cookies we've yet to send
* @return array All the cookies
*/
public static function get_all($includeUnsent = true) {
return self::get_inst()->getAll($includeUnsent);
}
/**
@ -61,25 +66,7 @@ class Cookie {
* @param string
* @param string
*/
public static function force_expiry($name, $path = null, $domain = null, $secure = false, $httpOnly = false) {
public static function force_expiry($name, $path = null, $domain = null, $secure = false, $httpOnly = true) {
return self::get_inst()->forceExpiry($name, $path, $domain, $secure, $httpOnly);
}
/**
* @deprecated 3.2 Use the "Cookie.report_errors" config setting instead
* @param bool
*/
protected function set_report_errors($reportErrors) {
Deprecation::notice('3.2', 'Use the "Cookie.report_errors" config setting instead');
Config::inst()->update('Cookie', 'report_errors', $reportErrors);
}
/**
* @deprecated 3.2 Use the "Cookie.report_errors" config setting instead
* @return bool
*/
protected function report_errors() {
Deprecation::notice('3.2', 'Use the "Cookie.report_errors" config setting instead');
return Config::inst()->get('Cookie', 'report_errors');
}
}

17
control/CookieJar.php
View File

@ -42,10 +42,13 @@ class CookieJar implements Cookie_Backend {
* "existing" array. This allows us to distinguish between cookies we recieved
* or we set ourselves (and didn't get from the browser)
*
* @param array $cookies The existing cookies to load into the cookie jar
* @param array $cookies The existing cookies to load into the cookie jar.
* Omit this to default to $_COOKIE
*/
public function __construct(array $cookies = null) {
$this->current = $this->existing = !empty($cookies) ? $cookies : $this->existing;
public function __construct($cookies = array()) {
$this->current = $this->existing = func_num_args()
? ($cookies ?: array()) // Convert empty values to blank arrays
: $_COOKIE;
}
/**
@ -105,7 +108,6 @@ class CookieJar implements Cookie_Backend {
* Get all the cookies
*
* @param boolean $includeUnsent Include cookies we've yet to send
*
* @return array All the cookies
*/
public function getAll($includeUnsent = true) {
@ -121,8 +123,8 @@ class CookieJar implements Cookie_Backend {
* @param boolean $secure Can the cookie only be sent over SSL?
* @param boolean $httpOnly Prevent the cookie being accessible by JS
*/
public function forceExpiry($name, $path = null, $domain = null, $secure = false, $httpOnly = false) {
$this->set($name, false,-1, $path, $domain, $secure, $httpOnly);
public function forceExpiry($name, $path = null, $domain = null, $secure = false, $httpOnly = true) {
$this->set($name, false, -1, $path, $domain, $secure, $httpOnly);
}
/**
@ -137,11 +139,10 @@ class CookieJar implements Cookie_Backend {
* @param string $domain The domain to make the cookie available on
* @param boolean $secure Can the cookie only be sent over SSL?
* @param boolean $httpOnly Prevent the cookie being accessible by JS
*
* @return boolean If the cookie was set or not; doesn't mean it's accepted by the browser
*/
protected function outputCookie(
$name, $value, $expiry = 90, $path = null, $domain = null, $secure = false, $httpOnly = false
$name, $value, $expiry = 90, $path = null, $domain = null, $secure = false, $httpOnly = true
) {
// if headers aren't sent, we can set the cookie
if(!headers_sent($file, $line)) {

12
control/Cookie_Backend.php
View File

@ -14,7 +14,7 @@ interface Cookie_Backend {
*
* @param array $cookies The existing cookies to load into the cookie jar
*/
public function __construct(array $cookies = null);
public function __construct($cookies = array());
/**
* Set a cookie
@ -27,23 +27,25 @@ interface Cookie_Backend {
* @param boolean $secure Can the cookie only be sent over SSL?
* @param boolean $httpOnly Prevent the cookie being accessible by JS
*/
public function set($name, $value, $expiry = 90, $path = null, $domain = null, $secure = false, $httpOnly = false);
public function set($name, $value, $expiry = 90, $path = null, $domain = null, $secure = false, $httpOnly = true);
/**
* Get the cookie value by name
*
* @param string $name The name of the cookie to get
* @param boolean $includeUnsent Include cookies we've yet to send when fetching values
*
* @return string|null The cookie value or null if unset
*/
public function get($name);
public function get($name, $includeUnsent = true);
/**
* Get all the cookies
*
* @param boolean $includeUnsent Include cookies we've yet to send
* @return array All the cookies
*/
public function getAll();
public function getAll($includeUnsent = true);
/**
* Force the expiry of a cookie by name
@ -54,6 +56,6 @@ interface Cookie_Backend {
* @param boolean $secure Can the cookie only be sent over SSL?
* @param boolean $httpOnly Prevent the cookie being accessible by JS
*/
public function forceExpiry($name, $path = null, $domain = null, $secure = false, $httpOnly = false);
public function forceExpiry($name, $path = null, $domain = null, $secure = false, $httpOnly = true);
}

62
control/Director.php
View File

@ -207,17 +207,18 @@ class Director implements TemplateGlobalProvider {
* GET otherwise. Overwritten by $postVars['_method'] if present.
* @param string $body The HTTP body
* @param array $headers HTTP headers with key-value pairs
* @param Cookie_Backend $cookies to populate $_COOKIE
* @param array|Cookie_Backend $cookies to populate $_COOKIE
* @param HTTP_Request $request The {@see HTTP_Request} object generated as a part of this request
* @return SS_HTTPResponse
*
* @uses getControllerForURL() The rule-lookup logic is handled by this.
* @uses Controller::run() Controller::run() handles the page logic for a Director::direct() call.
*/
public static function test($url, $postVars = null, $session = null, $httpMethod = null, $body = null,
$headers = null, $cookies = null, &$request = null) {
public static function test($url, $postVars = null, $session = array(), $httpMethod = null, $body = null,
$headers = array(), $cookies = array(), &$request = null) {
Config::nest();
Injector::nest();
// These are needed so that calling Director::test() doesnt muck with whoever is calling it.
// Really, it's some inappropriate coupling and should be resolved by making less use of statics
@ -227,7 +228,9 @@ class Director implements TemplateGlobalProvider {
if(!$httpMethod) $httpMethod = ($postVars || is_array($postVars)) ? "POST" : "GET";
if(!$session) $session = Injector::inst()->create('Session', array());
if(!$cookies) $cookies = new CookieJar(null);
$cookieJar = $cookies instanceof Cookie_Backend
? $cookies
: Injector::inst()->createWithArgs('Cookie_Backend', array($cookies ?: array()));
// Back up the current values of the superglobals
$existingRequestVars = isset($_REQUEST) ? $_REQUEST : array();
@ -236,13 +239,35 @@ class Director implements TemplateGlobalProvider {
$existingSessionVars = isset($_SESSION) ? $_SESSION : array();
$existingCookies = isset($_COOKIE) ? $_COOKIE : array();
$existingServer = isset($_SERVER) ? $_SERVER : array();
$existingCookieJar = Cookie::get_inst();
$existingRequirementsBackend = Requirements::backend();
Config::inst()->update('Cookie', 'report_errors', false);
Requirements::set_backend(new Requirements_Backend());
// Set callback to invoke prior to return
$onCleanup = function() use(
$existingRequestVars, $existingGetVars, $existingPostVars, $existingSessionVars,
$existingCookies, $existingServer, $existingRequirementsBackend, $oldStage
) {
// Restore the superglobals
$_REQUEST = $existingRequestVars;
$_GET = $existingGetVars;
$_POST = $existingPostVars;
$_SESSION = $existingSessionVars;
$_COOKIE = $existingCookies;
$_SERVER = $existingServer;
Requirements::set_backend($existingRequirementsBackend);
// These are needed so that calling Director::test() doesnt muck with whoever is calling it.
// Really, it's some inappropriate coupling and should be resolved by making less use of statics
Versioned::reading_stage($oldStage);
Injector::unnest(); // Restore old CookieJar, etc
Config::unnest();
};
if (strpos($url, '#') !== false) {
$url = substr($url, 0, strpos($url, '#'));
}
@ -271,8 +296,8 @@ class Director implements TemplateGlobalProvider {
$_GET = (array)$getVars;
$_POST = (array)$postVars;
$_SESSION = $session ? $session->inst_getAll() : array();
$_COOKIE = $cookies->getAll(false);
Injector::inst()->registerService($cookies, 'CookieJar');
$_COOKIE = $cookieJar->getAll(false);
Injector::inst()->registerService($cookieJar, 'Cookie_Backend');
$_SERVER['REQUEST_URI'] = Director::baseURL() . $urlWithQuerystring;
$request = new SS_HTTPRequest($httpMethod, $url, $getVars, $postVars, $body);
@ -283,7 +308,7 @@ class Director implements TemplateGlobalProvider {
$model = DataModel::inst();
$output = Injector::inst()->get('RequestProcessor')->preRequest($request, $session, $model);
if ($output === false) {
// @TODO Need to NOT proceed with the request in an elegant manner
$onCleanup();
throw new SS_HTTPResponse_Exception(_t('Director.INVALID_REQUEST', 'Invalid request'), 400);
}
@ -303,27 +328,12 @@ class Director implements TemplateGlobalProvider {
$output = Injector::inst()->get('RequestProcessor')->postRequest($request, $result, $model);
if ($output === false) {
$onCleanup();
throw new SS_HTTPResponse_Exception("Invalid response");
}
// Restore the superglobals
$_REQUEST = $existingRequestVars;
$_GET = $existingGetVars;
$_POST = $existingPostVars;
$_SESSION = $existingSessionVars;
$_COOKIE = $existingCookies;
$_SERVER = $existingServer;
Injector::inst()->registerService($existingCookieJar, 'CookieJar');
Requirements::set_backend($existingRequirementsBackend);
// These are needed so that calling Director::test() doesnt muck with whoever is calling it.
// Really, it's some inappropriate coupling and should be resolved by making less use of statics
Versioned::reading_stage($oldStage);
Config::unnest();
// Return valid response
$onCleanup();
return $result;
}

12
control/Session.php
View File

@ -395,18 +395,11 @@ class Session {
public function inst_destroy($removeCookie = true) {
if(session_id()) {
if($removeCookie) {
$path = Config::inst()->get('Session', 'cookie_path');
if(!$path) $path = Director::baseURL();
$path = Config::inst()->get('Session', 'cookie_path') ?: Director::baseURL();
$domain = Config::inst()->get('Session', 'cookie_domain');
$secure = Config::inst()->get('Session', 'cookie_secure');
if($domain) {
Cookie::set(session_name(), '', null, $path, $domain, $secure, true);
} else {
Cookie::set(session_name(), '', null, $path, null, $secure, true);
}
unset($_COOKIE[session_name()]);
Cookie::force_expiry(session_name(), $path, $domain, $secure, true);
}
session_destroy();
@ -609,7 +602,6 @@ class Session {
*/
public static function destroy($removeCookie = true) {
self::current_session()->inst_destroy($removeCookie);
Cookie::force_expiry(session_name(), $path, null, $secure, true);
}
/**

10
control/injector/Injector.php
View File

@ -740,21 +740,13 @@ class Injector {
$this->inject($service);
}
/**
* Remove a service that has been registered
*
* @param string $service The service to clear
*/
public function clearService($service) {
unset($this->specs[$service], $this->serviceCache[$service]);
}
/**
* Register a service with an explicit name
*
* @deprecated since 3.1.1
*/
public function registerNamedService($name, $service) {
Deprecation::notice('3.2', 'registerNamedService is deprecated, use registerService instead');
Deprecation::notice('3.1.1', 'registerNamedService is deprecated, use registerService instead');
return $this->registerService($service, $name);
}

16
dev/TestSession.php
View File

@ -14,9 +14,13 @@ class TestSession {
private $session;
/**
* @var SS_HTTPResponse
* @var Cookie_Backend
*/
private $cookies;
/**
* @var SS_HTTPResponse
*/
private $lastResponse;
/**
@ -37,7 +41,7 @@ class TestSession {
public function __construct() {
$this->session = Injector::inst()->create('Session', array());
$this->cookies = new CookieJar();
$this->cookies = Injector::inst()->create('Cookie_Backend');
$this->controller = new Controller();
$this->controller->setSession($this->session);
$this->controller->pushCurrent();
@ -67,11 +71,11 @@ class TestSession {
$this->lastResponse = Director::test(
$url,
null,
$session ? $session : $this->session,
$session ?: $this->session,
null,
null,
$headers,
$cookies ? $cookies : $this->cookies
$cookies ?: $this->cookies
);
$this->lastUrl = $url;
if(!$this->lastResponse) user_error("Director::test($url) returned null", E_USER_WARNING);
@ -96,11 +100,11 @@ class TestSession {
$this->lastResponse = Director::test(
$url,
$data,
$session ? $session : $this->session,
$session ?: $this->session,
null,
$body,
$headers,
$cookies ? $cookies : $this->cookies
$cookies ?: $this->cookies
);
$this->lastUrl = $url;
if(!$this->lastResponse) user_error("Director::test($url) returned null", E_USER_WARNING);

87
docs/en/reference/cookies.md
View File

@ -6,28 +6,24 @@ Cookies can be set/get/expired using the `Cookie` class and its static methods
setting:
```php
Cookie::set('CookieName', 'CookieValue');
```
:::php
Cookie::set('CookieName', 'CookieValue');
getting:
```php
Cookie::get('CookieName'); //returns null if not set or the value if set
```
:::php
Cookie::get('CookieName'); //returns null if not set or the value if set
expiring / removing / clearing:
```php
Cookie::force_expiry('CookieName');
```
:::php
Cookie::force_expiry('CookieName');
## The `Cookie_Backend`
The `Cookie` class manipulates and sets cookies using a `Cookie_Backend`. The backend is in charge of the logic
that fetches, sets and expires cookies. By default we use a the `CookieJar` backend which uses PHP's
(`setcookie`)[http://www.php.net/manual/en/function.setcookie.php] function.
[setcookie](http://www.php.net/manual/en/function.setcookie.php) function.
The `CookieJar` keeps track of cookies that have been set by the current process as well as those that were recieved
from the browser.
@ -38,48 +34,42 @@ the `Injector`.
eg:
```php
$myCookies = array(
:::php
$myCookies = array(
'cookie1' => 'value1',
);
);
$newBackend = new CookieJar($myCookies);
$newBackend = new CookieJar($myCookies);
Injector::inst()->registerService($newBackend, 'CookieJar');
Injector::inst()->registerService($newBackend, 'Cookie_Backend');
Cookie::get('cookie1'); //will return 'value1'
```
Cookie::get('cookie1'); //will return 'value1'
### Resetting the Cookie_Backend state
Assuming that your appliation hasn't messed around with the `$_COOKIE` superglobal, you can reset the state of your
Assuming that your application hasn't messed around with the `$_COOKIE` superglobal, you can reset the state of your
`Cookie_Backend` by simply unregistering the `CookieJar` service with `Injector`. Next time you access `Cookie` it'll
create a new service for you using the `$_COOKIE` superglobal
create a new service for you using the `$_COOKIE` superglobal.
eg:
```php
:::php
Injector::inst()->unregisterNamedObject('Cookie_Backend');
Injector::inst()->unregisterNamedObject('CookieJar');
Cookie::get('cookiename'); // will return $_COOKIE['cookiename'] if set
Cookie::get('cookiename'); //will return $_COOKIE['cookiename'] if set
```
Alternatively, if you know that the superglobal has been changed (or you aren't sure it hasn't) you can attempt to use
the current `CookieJar` service to tell you what it was like when it was registered.
eg:
```php
:::php
//store the cookies that were loaded into the `CookieJar`
$recievedCookie = Cookie::get_inst()->getAll(false);
//store the cookies that were loaded into the `CookieJar`
$recievedCookie = Cookie::get_inst()->getAll(false);
//set a new `CookieJar`
Injector::inst()->registerService(new CookieJar($recievedCookie), 'CookieJar');
```
//set a new `CookieJar`
Injector::inst()->registerService(new CookieJar($recievedCookie), 'CookieJar');
### Using your own Cookie_Backend
@ -88,11 +78,14 @@ If you need to implement your own Cookie_Backend you can use the injector system
example:
```yml
Injector:
CookieJar:
:::yml
---
Name: mycookie
After: '#cookie'
---
Injector:
Cookie_Backend:
class: MyCookieJar
```
To be a valid backend your class must implement the `Cookie_Backend` interface.
@ -105,24 +98,20 @@ came from the browser as part of the request.
Using the `Cookie_Backend` we can do this like such:
```php
:::php
Cookie::set('CookieName', 'CookieVal');
Cookie::set('CookieName', 'CookieVal');
Cookie::get('CookieName'); //gets the cookie as we set it
Cookie::get('CookieName'); //gets the cookie as we set it
//will return the cookie as it was when it was sent in the request
Cookie::get('CookieName', false);
//will return the cookie as it was when it was sent in the request
Cookie::get_inst()->get('CookieName', false);
```
### Accessing all the cookies at once
One can also access all of the cookies in one go using the `Cookie_Backend`
```php
:::php
Cookie::get_inst()->getAll(); //returns all the cookies including ones set during the current process
Cookie::get_inst()->getAll(); //returns all the cookies including ones set during the current process
Cookie::get_inst()->getAll(false); //returns all the cookies in the request
```
Cookie::get_inst()->getAll(false); //returns all the cookies in the request

2
tests/control/CookieJarTest.php
View File

@ -27,7 +27,7 @@ class CookieJarTest extends SapphireTest {
$this->assertEquals($defaultCookies, $cookieJar->getAll(false));
//make sure there are no "phantom" cookies
$this->assertEquals($defaultCookies, $cookieJar->getAll(false));
$this->assertEquals($defaultCookies, $cookieJar->getAll(true));
//check an empty array is accepted
$cookieJar = new CookieJar(array());

108
tests/control/CookieTest.php
View File

@ -4,25 +4,16 @@ class CookieTest extends SapphireTest {
private $cookieInst;
public function setUpOnce() {
parent::setUpOnce();
Injector::nest();
}
public function tearDownOnce() {
parent::tearDownOnce();
//restore the cookie_backend
Injector::unnest();
}
public function setUp() {
parent::setUp();
Injector::nest();
Injector::inst()->registerService(new CookieJar($_COOKIE), 'Cookie_Backend');
}
public function tearDown() {
//restore the cookie_backend
Injector::unnest();
parent::tearDown();
Injector::inst()->unregisterNamedObject('Cookie_Backend');
}
/**
@ -101,4 +92,97 @@ class CookieTest extends SapphireTest {
}
/**
* Test that we can set and get cookies
*/
public function testSetAndGet() {
$this->assertEmpty(Cookie::get('testCookie'));
//set a test cookie
Cookie::set('testCookie', 'testVal');
//make sure it was set
$this->assertEquals('testVal', Cookie::get('testCookie'));
//make sure we can distinguise it from ones that were "existing"
$this->assertEmpty(Cookie::get('testCookie', false));
}
/**
* Test that we can distinguish between vars that were loaded on instantiation
* and those added later
*/
public function testExistingVersusNew() {
//load with a cookie
$cookieJar = new CookieJar(array(
'cookieExisting' => 'i woz here',
));
Injector::inst()->registerService($cookieJar, 'Cookie_Backend');
//set a new cookie
Cookie::set('cookieNew', 'i am new');
//check we can fetch new and old cookie values
$this->assertEquals('i woz here', Cookie::get('cookieExisting'));
$this->assertEquals('i woz here', Cookie::get('cookieExisting', false));
$this->assertEquals('i am new', Cookie::get('cookieNew'));
//there should be no original value for the new cookie
$this->assertEmpty(Cookie::get('cookieNew', false));
//change the existing cookie, can we fetch the new and old value
Cookie::set('cookieExisting', 'i woz changed');
$this->assertEquals('i woz changed', Cookie::get('cookieExisting'));
$this->assertEquals('i woz here', Cookie::get('cookieExisting', false));
//check we can get all cookies
$this->assertEquals(array(
'cookieExisting' => 'i woz changed',
'cookieNew' => 'i am new',
), Cookie::get_all());
//check we can get all original cookies
$this->assertEquals(array(
'cookieExisting' => 'i woz here',
), Cookie::get_all(false));
}
/**
* Check we can remove cookies and we can access their original values
*/
public function testForceExpiry() {
//load an existing cookie
$cookieJar = new CookieJar(array(
'cookieExisting' => 'i woz here',
));
Injector::inst()->registerService($cookieJar, 'Cookie_Backend');
//make sure it's available
$this->assertEquals('i woz here', Cookie::get('cookieExisting'));
//remove the cookie
Cookie::force_expiry('cookieExisting');
//check it's gone
$this->assertEmpty(Cookie::get('cookieExisting'));
//check we can get it's original value
$this->assertEquals('i woz here', Cookie::get('cookieExisting', false));
//check we can add a new cookie and remove it and it doesn't leave any phantom values
Cookie::set('newCookie', 'i am new');
//check it's set by not recieved
$this->assertEquals('i am new', Cookie::get('newCookie'));
$this->assertEmpty(Cookie::get('newCookie', false));
//remove it
Cookie::force_expiry('newCookie');
//check it's neither set nor reveived
$this->assertEmpty(Cookie::get('newCookie'));
$this->assertEmpty(Cookie::get('newCookie', false));
}
}

8
tests/control/DirectorTest.php
View File

@ -240,6 +240,7 @@ class DirectorTest extends SapphireTest {
unset($_SESSION['isLive']);
unset($_GET['isTest']);
unset($_GET['isDev']);
$_SESSION = $_SESSION ?: array();
// Test isDev=1
$_GET['isDev'] = '1';
@ -271,7 +272,10 @@ class DirectorTest extends SapphireTest {
$_POST = array('somekey' => 'postvalue');
$_COOKIE = array('somekey' => 'cookievalue');
$cookies = new CookieJar(array('somekey' => 'sometestcookievalue'));
$cookies = Injector::inst()->createWithArgs(
'Cookie_Backend',
array(array('somekey' => 'sometestcookievalue'))
);
$getresponse = Director::test('errorpage?somekey=sometestgetvalue', array('somekey' => 'sometestpostvalue'),
null, null, null, null, $cookies);
@ -298,7 +302,7 @@ class DirectorTest extends SapphireTest {
strtoupper($method),
null,
null,
new CookieJar($fixture)
Injector::inst()->createWithArgs('Cookie_Backend', array($fixture))
);
$this->assertInstanceOf('SS_HTTPResponse', $getresponse, 'Director::test() returns SS_HTTPResponse');

2
tests/injector/InjectorTest.php
View File

@ -558,7 +558,7 @@ class InjectorTest extends SapphireTest {
$injector = new Injector();
$service = new stdClass();
$injector->registerNamedService('NamedService', $service);
$injector->registerService($service, 'NamedService');
$this->assertEquals($service, $injector->get('NamedService'));
}