From 8ff496f070e9927bd017b80d3e41d4d81b21b9b4 Mon Sep 17 00:00:00 2001 From: Paul Croker Date: Sun, 9 Jun 2013 11:44:39 +1000 Subject: [PATCH 01/16] Add __get and __isset property overloading with tests. --- Slim/Helper/Set.php | 14 ++++++++++++++ tests/Helper/SetTest.php | 25 +++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/Slim/Helper/Set.php b/Slim/Helper/Set.php index 167fca059..88a2187f3 100644 --- a/Slim/Helper/Set.php +++ b/Slim/Helper/Set.php @@ -139,6 +139,20 @@ public function remove($key) unset($this->data[$this->normalizeKey($key)]); } + /** + * Property Overloading + */ + + public function __get($key) + { + return $this->get($key); + } + + public function __isset($key) + { + return $this->has($key); + } + /** * Clear all values */ diff --git a/tests/Helper/SetTest.php b/tests/Helper/SetTest.php index 0d33c0a44..6d339c358 100644 --- a/tests/Helper/SetTest.php +++ b/tests/Helper/SetTest.php @@ -166,4 +166,29 @@ public function testGetIterator() $this->property->setValue($this->bag, $data); $this->assertInstanceOf('\ArrayIterator', $this->bag->getIterator()); } + + public function testPropertyOverloadGet() + { + $data = array( + 'abc' => '123', + 'foo' => 'bar' + ); + $this->property->setValue($this->bag, $data); + + $this->assertEquals('123', $this->bag->abc); + $this->assertEquals('bar', $this->bag->foo); + } + + public function testPropertyOverloadingIsset() + { + $data = array( + 'abc' => '123', + 'foo' => 'bar' + ); + $this->property->setValue($this->bag, $data); + + $this->assertTrue(isset($this->bag->abc)); + $this->assertTrue(isset($this->bag->foo)); + $this->assertFalse(isset($this->bag->foobar)); + } } From 24c056ef4bdb01bf8b15207265b422f5856f60c0 Mon Sep 17 00:00:00 2001 From: Paul Croker Date: Tue, 11 Jun 2013 18:10:46 +1000 Subject: [PATCH 02/16] Add remaining magic. Also remembered PSR-2. --- Slim/Helper/Set.php | 36 ++++++++++++------- tests/Helper/SetTest.php | 76 ++++++++++++++++++++++++++-------------- 2 files changed, 72 insertions(+), 40 deletions(-) diff --git a/Slim/Helper/Set.php b/Slim/Helper/Set.php index 88a2187f3..47e77551d 100644 --- a/Slim/Helper/Set.php +++ b/Slim/Helper/Set.php @@ -139,19 +139,29 @@ public function remove($key) unset($this->data[$this->normalizeKey($key)]); } - /** - * Property Overloading - */ - - public function __get($key) - { - return $this->get($key); - } - - public function __isset($key) - { - return $this->has($key); - } + /** + * Property Overloading + */ + + public function __get($key) + { + return $this->get($key); + } + + public function __set($key, $value) + { + $this->set($key, $value); + } + + public function __isset($key) + { + return $this->has($key); + } + + public function __unset($key) + { + return $this->remove($key); + } /** * Clear all values diff --git a/tests/Helper/SetTest.php b/tests/Helper/SetTest.php index 6d339c358..222f6e2f1 100644 --- a/tests/Helper/SetTest.php +++ b/tests/Helper/SetTest.php @@ -43,7 +43,7 @@ public function testSet() { $this->bag->set('foo', 'bar'); $this->assertArrayHasKey('foo', $this->property->getValue($this->bag)); - $bag = $this->property->getValue($this->bag); + $bag = $this->property->getValue($this->bag); $this->assertEquals('bar', $bag['foo']); } @@ -67,7 +67,7 @@ public function testAdd() )); $this->assertArrayHasKey('abc', $this->property->getValue($this->bag)); $this->assertArrayHasKey('foo', $this->property->getValue($this->bag)); - $bag = $this->property->getValue($this->bag); + $bag = $this->property->getValue($this->bag); $this->assertEquals('123', $bag['abc']); $this->assertEquals('bar', $bag['foo']); } @@ -121,7 +121,7 @@ public function testArrayAccessSet() ); $this->property->setValue($this->bag, $data); $this->bag['foo'] = 'changed'; - $bag = $this->property->getValue($this->bag); + $bag = $this->property->getValue($this->bag); $this->assertEquals('changed', $bag['foo']); } @@ -167,28 +167,50 @@ public function testGetIterator() $this->assertInstanceOf('\ArrayIterator', $this->bag->getIterator()); } - public function testPropertyOverloadGet() - { - $data = array( - 'abc' => '123', - 'foo' => 'bar' - ); - $this->property->setValue($this->bag, $data); - - $this->assertEquals('123', $this->bag->abc); - $this->assertEquals('bar', $this->bag->foo); - } - - public function testPropertyOverloadingIsset() - { - $data = array( - 'abc' => '123', - 'foo' => 'bar' - ); - $this->property->setValue($this->bag, $data); - - $this->assertTrue(isset($this->bag->abc)); - $this->assertTrue(isset($this->bag->foo)); - $this->assertFalse(isset($this->bag->foobar)); - } + public function testPropertyOverloadGet() + { + $data = array( + 'abc' => '123', + 'foo' => 'bar' + ); + $this->property->setValue($this->bag, $data); + + $this->assertEquals('123', $this->bag->abc); + $this->assertEquals('bar', $this->bag->foo); + } + + public function testPropertyOverloadSet() + { + $this->bag->foo = 'bar'; + $this->assertArrayHasKey('foo', $this->property->getValue($this->bag)); + $this->assertEquals('bar', $this->bag->foo); + } + + public function testPropertyOverloadingIsset() + { + $data = array( + 'abc' => '123', + 'foo' => 'bar' + ); + $this->property->setValue($this->bag, $data); + + $this->assertTrue(isset($this->bag->abc)); + $this->assertTrue(isset($this->bag->foo)); + $this->assertFalse(isset($this->bag->foobar)); + } + + public function testPropertyOverloadingUnset() + { + $data = array( + 'abc' => '123', + 'foo' => 'bar' + ); + $this->property->setValue($this->bag, $data); + + $this->assertTrue(isset($this->bag->abc)); + unset($this->bag->abc); + $this->assertFalse(isset($this->bag->abc)); + $this->assertArrayNotHasKey('abc', $this->property->getValue($this->bag)); + $this->assertArrayHasKey('foo', $this->property->getValue($this->bag)); + } } From d273a5f0ab5d06f976850f4f3d4699460b95c143 Mon Sep 17 00:00:00 2001 From: Paul Croker Date: Tue, 11 Jun 2013 18:14:14 +1000 Subject: [PATCH 03/16] Fix missed line. --- tests/Helper/SetTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Helper/SetTest.php b/tests/Helper/SetTest.php index 222f6e2f1..d7efe4d4a 100644 --- a/tests/Helper/SetTest.php +++ b/tests/Helper/SetTest.php @@ -210,7 +210,7 @@ public function testPropertyOverloadingUnset() $this->assertTrue(isset($this->bag->abc)); unset($this->bag->abc); $this->assertFalse(isset($this->bag->abc)); - $this->assertArrayNotHasKey('abc', $this->property->getValue($this->bag)); + $this->assertArrayNotHasKey('abc', $this->property->getValue($this->bag)); $this->assertArrayHasKey('foo', $this->property->getValue($this->bag)); } } From 28eff874bc82bb8d51a41cecbce1ab9308e7f853 Mon Sep 17 00:00:00 2001 From: Roberts Date: Sun, 30 Jun 2013 22:15:14 +0300 Subject: [PATCH 04/16] Pretty exceptions only in debug --- Slim/Slim.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Slim/Slim.php b/Slim/Slim.php index e9f790e98..afea1ef1b 100644 --- a/Slim/Slim.php +++ b/Slim/Slim.php @@ -1234,7 +1234,10 @@ public function run() set_error_handler(array('\Slim\Slim', 'handleErrors')); //Apply final outer middleware layers - $this->add(new \Slim\Middleware\PrettyExceptions()); + if($this->config('debug')){ + //Apply pretty exceptions only in debug to avoid accidental information leakage in production + $this->add(new \Slim\Middleware\PrettyExceptions()); + } //Invoke middleware and application stack $this->middleware[0]->call(); From a23f9f83513cd46189e94358cb4425473ec0c081 Mon Sep 17 00:00:00 2001 From: Gabriel Manricks Date: Sun, 14 Jul 2013 18:04:09 +0300 Subject: [PATCH 05/16] Cleaned up output on tests --- tests/SlimTest.php | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/tests/SlimTest.php b/tests/SlimTest.php index 7e996fc84..5a251d57b 100644 --- a/tests/SlimTest.php +++ b/tests/SlimTest.php @@ -1211,7 +1211,9 @@ public function testNotFound() */ public function testSlimError() { - $s = new \Slim\Slim(); + $s = new \Slim\Slim(array( + "log.enabled" => false + )); $s->get('/bar', function () use ($s) { $s->error(); }); @@ -1292,7 +1294,8 @@ public function DISABLEDtestTriggeredErrorsAreConvertedToErrorExceptions() public function testErrorWithMultipleApps() { $s1 = new \Slim\Slim(array( - 'debug' => false + 'debug' => false, + 'log.enabled' => false )); $s2 = new \Slim\Slim(); $s1->get('/bar', function () use ($s1) { @@ -1373,8 +1376,8 @@ public function testErrorHandler() { * Slim should throw a Slim_Exception_Stop if error callback is not callable */ public function testErrorHandlerIfNotCallable() { - $this->setExpectedException('\Slim\Exception\Stop'); - $s = new \Slim\Slim(); + $this->setExpectedException('\Slim\Exception\Stop'); + $s = new \Slim\Slim(array("log.enabled" => false)); $errCallback = 'foo'; $s->error($errCallback); } @@ -1393,7 +1396,7 @@ public function testNotFoundHandler() { * Slim should throw a Slim_Exception_Stop if NotFound callback is not callable */ public function testNotFoundHandlerIfNotCallable() { - $this->setExpectedException('\Slim\Exception\Stop'); + $this->setExpectedException('\Slim\Exception\Stop'); $s = new \Slim\Slim(); $notFoundCallback = 'foo'; $s->notFound($notFoundCallback); @@ -1489,13 +1492,13 @@ public function testHookClear() $this->assertEquals(array(array()), $app->getHooks('test.hook.one')); } - /** + /** * Test late static binding * * Pre-conditions: * Slim app is extended by Derived class and instantiated; * Derived class overrides the 'getDefaultSettings' function and adds an extra default config value - * Test that the new config value exists + * Test that the new config value exists * * Post-conditions: * Config value exists and is equal to expected value From 1d4645509e8bb82df936cefed8124a53c73f6eda Mon Sep 17 00:00:00 2001 From: = Date: Thu, 8 Aug 2013 08:12:58 -0400 Subject: [PATCH 06/16] Force Travis CI rebuild --- README.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.markdown b/README.markdown index b4bc7c9b0..6bd9e2851 100644 --- a/README.markdown +++ b/README.markdown @@ -65,7 +65,7 @@ should contain this code: #### Nginx -Your nginx configuration file should contain this code (along with other settings you may need) in your `location` block: +The nginx configuration file should contain this code (along with other settings you may need) in your `location` block: try_files $uri $uri/ /index.php?$args; From 2b7a9d80eea998688c54617be3284401271b0af9 Mon Sep 17 00:00:00 2001 From: Greg Poole Date: Wed, 4 Sep 2013 14:16:59 +1000 Subject: [PATCH 07/16] Change to MethodOverride middleware to use correct environment variable - Updated tests to check correctly named HTTP_X_HTTP_METHOD_OVERRIDE (previously expected HTTP_ to be stripped, but it isn't) - Updated MethodOverride to use correct name - Added test which will fail if the HTTP_ is ever stripped off this variable in future --- Slim/Middleware/MethodOverride.php | 4 ++-- tests/EnvironmentTest.php | 13 +++++++++++++ tests/Middleware/MethodOverrideTest.php | 2 +- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/Slim/Middleware/MethodOverride.php b/Slim/Middleware/MethodOverride.php index f43b41f0b..5e4473e2b 100644 --- a/Slim/Middleware/MethodOverride.php +++ b/Slim/Middleware/MethodOverride.php @@ -76,10 +76,10 @@ public function __construct($settings = array()) public function call() { $env = $this->app->environment(); - if (isset($env['X_HTTP_METHOD_OVERRIDE'])) { + if (isset($env['HTTP_X_HTTP_METHOD_OVERRIDE'])) { // Header commonly used by Backbone.js and others $env['slim.method_override.original_method'] = $env['REQUEST_METHOD']; - $env['REQUEST_METHOD'] = strtoupper($env['X_HTTP_METHOD_OVERRIDE']); + $env['REQUEST_METHOD'] = strtoupper($env['HTTP_X_HTTP_METHOD_OVERRIDE']); } elseif (isset($env['REQUEST_METHOD']) && $env['REQUEST_METHOD'] === 'POST') { // HTML Form Override $req = new \Slim\Http\Request($env); diff --git a/tests/EnvironmentTest.php b/tests/EnvironmentTest.php index e86b24ac5..e2f12d6fa 100644 --- a/tests/EnvironmentTest.php +++ b/tests/EnvironmentTest.php @@ -295,6 +295,19 @@ public function testSetsSpecialHeaders() $this->assertEquals('XmlHttpRequest', $env['HTTP_X_REQUESTED_WITH']); } + /** + * Tests X-HTTP-Method-Override is allowed through unmolested. + * + * Pre-conditions: + * X_HTTP_METHOD_OVERRIDE is sent in client HTTP request; + * X_HTTP_METHOD_OVERRIDE is not empty; + */ + public function testSetsHttpMethodOverrideHeader() { + $_SERVER['HTTP_X_HTTP_METHOD_OVERRIDE'] = 'DELETE'; + $env = \Slim\Environment::getInstance(true); + $this->assertEquals('DELETE', $env['HTTP_X_HTTP_METHOD_OVERRIDE']); + } + /** * Test detects HTTPS * diff --git a/tests/Middleware/MethodOverrideTest.php b/tests/Middleware/MethodOverrideTest.php index c2c5857f3..7dd14d745 100644 --- a/tests/Middleware/MethodOverrideTest.php +++ b/tests/Middleware/MethodOverrideTest.php @@ -134,7 +134,7 @@ public function testOverrideMethodAsHeader() 'CONTENT_TYPE' => 'application/json', 'CONENT_LENGTH' => 0, 'slim.input' => '', - 'X_HTTP_METHOD_OVERRIDE' => 'DELETE' + 'HTTP_X_HTTP_METHOD_OVERRIDE' => 'DELETE' )); $app = new CustomAppMethod(); $mw = new \Slim\Middleware\MethodOverride(); From 222d1258382fa5ec881742c203665b27b848c921 Mon Sep 17 00:00:00 2001 From: Pascal Borreli Date: Sat, 24 Aug 2013 16:53:14 +0100 Subject: [PATCH 08/16] Fixed typos --- Slim/Http/Response.php | 2 +- Slim/Http/Util.php | 3 +-- Slim/Slim.php | 21 ++++++++++++--------- tests/Http/RequestTest.php | 6 +++--- tests/Middleware/ContentTypesTest.php | 6 +++--- tests/Middleware/MethodOverrideTest.php | 4 ++-- tests/RouteTest.php | 2 +- 7 files changed, 23 insertions(+), 21 deletions(-) diff --git a/Slim/Http/Response.php b/Slim/Http/Response.php index 1d3b729d5..278bc3121 100644 --- a/Slim/Http/Response.php +++ b/Slim/Http/Response.php @@ -480,7 +480,7 @@ public function count() } /** - * DEPRECATION WARNING! IteratorAggreate interface will be removed from \Slim\Http\Response. + * DEPRECATION WARNING! IteratorAggregate interface will be removed from \Slim\Http\Response. * Iterate `headers` or `cookies` properties directly. * * Get Iterator diff --git a/Slim/Http/Util.php b/Slim/Http/Util.php index e85a9a43e..bae44175c 100644 --- a/Slim/Http/Util.php +++ b/Slim/Http/Util.php @@ -389,7 +389,7 @@ public static function deleteCookieHeader(&$header, $name, $value = array()) /** * Parse cookie header * - * This method will parse the HTTP requst's `Cookie` header + * This method will parse the HTTP request's `Cookie` header * and extract cookies into an associative array. * * @param string @@ -431,5 +431,4 @@ private static function getIv($expires, $secret) return pack("h*", $data1.$data2); } - } diff --git a/Slim/Slim.php b/Slim/Slim.php index b5d6e8926..fd70e451c 100644 --- a/Slim/Slim.php +++ b/Slim/Slim.php @@ -231,12 +231,14 @@ public function __set($name, $value) $this->container[$name] = $value; } - public function __isset($name){ - return isset($this->container[$name]); + public function __isset($name) + { + return isset($this->container[$name]); } - public function __unset($name){ - unset($this->container[$name]); + public function __unset($name) + { + unset($this->container[$name]); } /** @@ -524,7 +526,7 @@ public function options() * declarations in the callback will be prepended by the group(s) * that it is in * - * Accepts the same paramters as a standard route so: + * Accepts the same parameters as a standard route so: * (pattern, middleware1, middleware2, ..., $callback) */ public function group() @@ -870,6 +872,7 @@ public function setCookie($name, $value, $time = null, $path = null, $domain = n * the current request will not be available until the next request. * * @param string $name + * @param bool $deleteIfInvalid * @return string|null */ public function getCookie($name, $deleteIfInvalid = true) @@ -1242,9 +1245,9 @@ public function run() set_error_handler(array('\Slim\Slim', 'handleErrors')); //Apply final outer middleware layers - if($this->config('debug')){ - //Apply pretty exceptions only in debug to avoid accidental information leakage in production - $this->add(new \Slim\Middleware\PrettyExceptions()); + if ($this->config('debug')) { + //Apply pretty exceptions only in debug to avoid accidental information leakage in production + $this->add(new \Slim\Middleware\PrettyExceptions()); } //Invoke middleware and application stack @@ -1384,6 +1387,6 @@ protected function defaultNotFound() protected function defaultError($e) { $this->getLog()->error($e); - echo self::generateTemplateMarkup('Error', '

A website error has occured. The website administrator has been notified of the issue. Sorry for the temporary inconvenience.

'); + echo self::generateTemplateMarkup('Error', '

A website error has occurred. The website administrator has been notified of the issue. Sorry for the temporary inconvenience.

'); } } diff --git a/tests/Http/RequestTest.php b/tests/Http/RequestTest.php index 7a837a7d0..574fe1ff0 100644 --- a/tests/Http/RequestTest.php +++ b/tests/Http/RequestTest.php @@ -682,7 +682,7 @@ public function testGetHostWithPort() /** * Test get host with port doesn't duplicate port numbers */ - public function testGetHostDoesntDulplicatePort() + public function testGetHostDoesntDuplicatePort() { $env = \Slim\Environment::mock(array( 'HTTP_HOST' => 'slimframework.com:80', @@ -884,7 +884,7 @@ public function testGetIpWithForwardedFor() } /** - * Test get refererer + * Test get referer */ public function testGetReferrer() { @@ -897,7 +897,7 @@ public function testGetReferrer() } /** - * Test get refererer + * Test get referer */ public function testGetReferrerWhenNotExists() { diff --git a/tests/Middleware/ContentTypesTest.php b/tests/Middleware/ContentTypesTest.php index 6f9fcfe87..d1a566b93 100644 --- a/tests/Middleware/ContentTypesTest.php +++ b/tests/Middleware/ContentTypesTest.php @@ -107,7 +107,7 @@ public function testParsesXmlWithError() \Slim\Environment::mock(array( 'REQUEST_METHOD' => 'POST', 'CONTENT_TYPE' => 'application/xml', - 'CONENT_LENGTH' => 68, + 'CONTENT_LENGTH' => 68, 'slim.input' => '1Clive Cussler' //<-- This should be incorrect! )); $s = new \Slim\Slim(); @@ -126,7 +126,7 @@ public function testParsesCsv() \Slim\Environment::mock(array( 'REQUEST_METHOD' => 'POST', 'CONTENT_TYPE' => 'text/csv', - 'CONENT_LENGTH' => 44, + 'CONTENT_LENGTH' => 44, 'slim.input' => "John,Doe,000-111-2222\nJane,Doe,111-222-3333" )); $s = new \Slim\Slim(); @@ -148,7 +148,7 @@ public function testParsesRequestBodyWithMediaType() \Slim\Environment::mock(array( 'REQUEST_METHOD' => 'POST', 'CONTENT_TYPE' => 'application/json; charset=ISO-8859-4', - 'CONENT_LENGTH' => 13, + 'CONTENT_LENGTH' => 13, 'slim.input' => '{"foo":"bar"}' )); $s = new \Slim\Slim(); diff --git a/tests/Middleware/MethodOverrideTest.php b/tests/Middleware/MethodOverrideTest.php index 7dd14d745..af22ff185 100644 --- a/tests/Middleware/MethodOverrideTest.php +++ b/tests/Middleware/MethodOverrideTest.php @@ -64,7 +64,7 @@ public function testOverrideMethodAsPost() \Slim\Environment::mock(array( 'REQUEST_METHOD' => 'POST', 'CONTENT_TYPE' => 'application/x-www-form-urlencoded', - 'CONENT_LENGTH' => 11, + 'CONTENT_LENGTH' => 11, 'slim.input' => '_METHOD=PUT' )); $app = new CustomAppMethod(); @@ -132,7 +132,7 @@ public function testOverrideMethodAsHeader() \Slim\Environment::mock(array( 'REQUEST_METHOD' => 'POST', 'CONTENT_TYPE' => 'application/json', - 'CONENT_LENGTH' => 0, + 'CONTENT_LENGTH' => 0, 'slim.input' => '', 'HTTP_X_HTTP_METHOD_OVERRIDE' => 'DELETE' )); diff --git a/tests/RouteTest.php b/tests/RouteTest.php index dec3f9efa..31d8c4010 100644 --- a/tests/RouteTest.php +++ b/tests/RouteTest.php @@ -487,7 +487,7 @@ public function testDispatch() /** * Test dispatch with middleware */ - public function testDispatchWithMiddlware() + public function testDispatchWithMiddleware() { $this->expectOutputString('First! Second! Hello josh'); $route = new \Slim\Route('/hello/:name', function ($name) { echo "Hello $name"; }); From 1a30b092861d391e6822cfc999b2913d9064a0a6 Mon Sep 17 00:00:00 2001 From: SeanMooney Date: Mon, 19 Aug 2013 14:04:13 +0100 Subject: [PATCH 09/16] closes Issue #598 --- Slim/Http/Util.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Slim/Http/Util.php b/Slim/Http/Util.php index bae44175c..6aad66b3e 100644 --- a/Slim/Http/Util.php +++ b/Slim/Http/Util.php @@ -170,7 +170,7 @@ public static function decrypt($data, $key, $iv, $settings = array()) //Decrypt value mcrypt_generic_init($module, $key, $iv); $decryptedData = @mdecrypt_generic($module, $data); - $res = str_replace("\x0", '', $decryptedData); + $res = rtrim($decryptedData, "\0"); mcrypt_generic_deinit($module); return $res; From 2be864c28f35f70d60a2e682a7ca28f31456106a Mon Sep 17 00:00:00 2001 From: inf3rno Date: Wed, 4 Sep 2013 01:52:49 +0200 Subject: [PATCH 10/16] Preventing XEE attacks by temporarily disabling entity loader for the time parsing external XML. --- Slim/Middleware/ContentTypes.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Slim/Middleware/ContentTypes.php b/Slim/Middleware/ContentTypes.php index 809b500c0..e47ff5a43 100644 --- a/Slim/Middleware/ContentTypes.php +++ b/Slim/Middleware/ContentTypes.php @@ -137,7 +137,10 @@ protected function parseXml($input) { if (class_exists('SimpleXMLElement')) { try { - return new \SimpleXMLElement($input); + $backup = libxml_disable_entity_loader(true); + $result = new \SimpleXMLElement($input); + libxml_disable_entity_loader($backup); + return $result; } catch (\Exception $e) { // Do nothing } From f16cfd6e96521fb3a64a12a9a7d26ba364578d32 Mon Sep 17 00:00:00 2001 From: Jeremy Kendall Date: Thu, 19 Sep 2013 16:21:10 -0500 Subject: [PATCH 11/16] Removes encryption and decryption, adds some error handling Removes encryption and decryption from SessionCookie. The encryption and decrption will be handled elsewhere and can be enabled or disabled via the cookies.encrypt config setting. This fixes flash messaging/SessionCookie decoding failure when cookies.encrypt = true --- Slim/Middleware/SessionCookie.php | 58 ++-- tests/Middleware/SessionCookieTest.php | 370 ++++++++++++++++++++++++- 2 files changed, 395 insertions(+), 33 deletions(-) diff --git a/Slim/Middleware/SessionCookie.php b/Slim/Middleware/SessionCookie.php index d13dd94cd..cff435328 100644 --- a/Slim/Middleware/SessionCookie.php +++ b/Slim/Middleware/SessionCookie.php @@ -40,15 +40,11 @@ * and instead serializes/unserializes the $_SESSION global * variable to/from an HTTP cookie. * - * If a secret key is provided with this middleware, the HTTP - * cookie will be checked for integrity to ensure the client-side - * cookie is not changed. - * * You should NEVER store sensitive data in a client-side cookie - * in any format, encrypted or not. If you need to store sensitive - * user information in a session, you should rely on PHP's native - * session implementation, or use other middleware to store - * session data in a database or alternative server-side cache. + * in any format, encrypted (with cookies.encrypt) or not. If you + * need to store sensitive user information in a session, you should + * rely on PHP's native session implementation, or use other middleware + * to store session data in a database or alternative server-side cache. * * Because this class stores serialized session data in an HTTP cookie, * you are inherently limited to 4 Kb. If you attempt to store @@ -79,9 +75,6 @@ public function __construct($settings = array()) 'secure' => false, 'httponly' => false, 'name' => 'slim_session', - 'secret' => 'CHANGE_ME', - 'cipher' => MCRYPT_RIJNDAEL_256, - 'cipher_mode' => MCRYPT_MODE_CBC ); $this->settings = array_merge($defaults, $settings); if (is_string($this->settings['expires'])) { @@ -127,14 +120,14 @@ protected function loadSession() session_start(); } - $value = \Slim\Http\Util::decodeSecureCookie( - $this->app->request()->cookies($this->settings['name']), - $this->settings['secret'], - $this->settings['cipher'], - $this->settings['cipher_mode'] - ); + $value = $this->app->getCookie($this->settings['name']); + if ($value) { - $_SESSION = unserialize($value); + try { + $_SESSION = unserialize($value); + } catch (\Exception $e) { + $this->app->getLog()->error('Error unserializing session cookie value! ' . $e->getMessage()); + } } else { $_SESSION = array(); } @@ -145,17 +138,12 @@ protected function loadSession() */ protected function saveSession() { - $value = \Slim\Http\Util::encodeSecureCookie( - serialize($_SESSION), - $this->settings['expires'], - $this->settings['secret'], - $this->settings['cipher'], - $this->settings['cipher_mode'] - ); + $value = serialize($_SESSION); + if (strlen($value) > 4096) { $this->app->getLog()->error('WARNING! Slim\Middleware\SessionCookie data size is larger than 4KB. Content save failed.'); } else { - $this->app->response()->setCookie( + $this->app->setCookie( $this->settings['name'], array( 'value' => $value, @@ -174,31 +162,49 @@ protected function saveSession() * Session Handler *******************************************************************************/ + /** + * @codeCoverageIgnore + */ public function open($savePath, $sessionName) { return true; } + /** + * @codeCoverageIgnore + */ public function close() { return true; } + /** + * @codeCoverageIgnore + */ public function read($id) { return ''; } + /** + * @codeCoverageIgnore + */ public function write($id, $data) { return true; } + /** + * @codeCoverageIgnore + */ public function destroy($id) { return true; } + /** + * @codeCoverageIgnore + */ public function gc($maxlifetime) { return true; diff --git a/tests/Middleware/SessionCookieTest.php b/tests/Middleware/SessionCookieTest.php index 472c01c01..e32860419 100644 --- a/tests/Middleware/SessionCookieTest.php +++ b/tests/Middleware/SessionCookieTest.php @@ -45,8 +45,9 @@ public function setUp() * Test session cookie is set and constructed correctly * * We test for two things: - * 1) That the HTTP cookie is added to the `Set-Cookie:` response header; - * 2) That the HTTP cookie is constructed in the expected format; + * + * 1) That the HTTP cookie exists with the correct name; + * 2) That the HTTP cookie's value is the expected value; */ public function testSessionCookieIsCreated() { @@ -56,6 +57,7 @@ public function testSessionCookieIsCreated() )); $app = new \Slim\Slim(); $app->get('/foo', function () { + $_SESSION['foo'] = 'bar'; echo "Success"; }); $mw = new \Slim\Middleware\SessionCookie(array('expires' => '10 years')); @@ -64,16 +66,18 @@ public function testSessionCookieIsCreated() $mw->call(); list($status, $header, $body) = $app->response()->finalize(); $this->assertTrue($app->response->cookies->has('slim_session')); + $cookie = $app->response->cookies->get('slim_session'); + $this->assertEquals(serialize(array('foo' => 'bar')), $cookie['value']['value']); } /** - * Test $_SESSION is populated from HTTP cookie + * Test $_SESSION is populated from an encrypted HTTP cookie * - * The HTTP cookie in this test was created using the previous test; the encrypted cookie contains - * the serialized array ['foo' => 'bar']. The middleware secret, cipher, and cipher mode are assumed - * to be the default values. + * The encrypted cookie contains the serialized array ['foo' => 'bar']. The + * global secret, cipher, and cipher mode are assumed to be the default + * values. */ - public function testSessionIsPopulatedFromCookie() + public function testSessionIsPopulatedFromEncryptedCookie() { \Slim\Environment::mock(array( 'SCRIPT_NAME' => '/index.php', @@ -81,6 +85,9 @@ public function testSessionIsPopulatedFromCookie() 'HTTP_COOKIE' => 'slim_session=1644004961%7CLKkYPwqKIMvBK7MWl6D%2BxeuhLuMaW4quN%2F512ZAaVIY%3D%7Ce0f007fa852c7101e8224bb529e26be4d0dfbd63', )); $app = new \Slim\Slim(); + // The cookie value in the test is encrypted, so cookies.encrypt must + // be set to true + $app->config('cookies.encrypt', true); $app->get('/foo', function () { echo "Success"; }); @@ -91,6 +98,64 @@ public function testSessionIsPopulatedFromCookie() $this->assertEquals(array('foo' => 'bar'), $_SESSION); } + /** + * Test $_SESSION is populated from an unencrypted HTTP cookie + * + * The unencrypted cookie contains the serialized array ['foo' => 'bar']. + * The global cookies.encrypt setting is set to false + */ + public function testSessionIsPopulatedFromUnencryptedCookie() + { + \Slim\Environment::mock(array( + 'SCRIPT_NAME' => '/index.php', + 'PATH_INFO' => '/foo', + 'HTTP_COOKIE' => 'slim_session=a%3A1%3A%7Bs%3A3%3A%22foo%22%3Bs%3A3%3A%22bar%22%3B%7D', + )); + $app = new \Slim\Slim(); + // The cookie value in the test is unencrypted, so cookies.encrypt must + // be set to false + $app->config('cookies.encrypt', false); + $app->get('/foo', function () { + echo "Success"; + }); + $mw = new \Slim\Middleware\SessionCookie(array('expires' => '10 years')); + $mw->setApplication($app); + $mw->setNextMiddleware($app); + $mw->call(); + $this->assertEquals(array('foo' => 'bar'), $_SESSION); + } + + public function testUnserializeErrorsAreCaughtAndLogged() + { + \Slim\Environment::mock(array( + 'SCRIPT_NAME' => '/index.php', + 'PATH_INFO' => '/foo', + 'HTTP_COOKIE' => 'slim_session=1644004961%7CLKkYPwqKIMvBK7MWl6D%2BxeuhLuMaW4quN%2F512ZAaVIY%3D%7Ce0f007fa852c7101e8224bb529e26be4d0dfbd63', + )); + + $logWriter = $this->getMockBuilder('Slim\LogWriter') + ->disableOriginalConstructor() + ->getMock(); + + $logWriter->expects($this->once()) + ->method('write') + ->with('Error unserializing session cookie value! unserialize(): Error at offset 0 of 96 bytes', \Slim\Log::ERROR); + + $app = new \Slim\Slim(array( + 'log.writer' => $logWriter + )); + // Unserializing an encrypted value fails if cookie not decrpyted + $app->config('cookies.encrypt', false); + $app->get('/foo', function () { + echo "Success"; + }); + $mw = new \Slim\Middleware\SessionCookie(array('expires' => '10 years')); + $mw->setApplication($app); + $mw->setNextMiddleware($app); + $mw->call(); + $this->assertEquals(array(), $_SESSION); + } + /** * Test $_SESSION is populated as empty array if no HTTP cookie */ @@ -110,4 +175,295 @@ public function testSessionIsPopulatedAsEmptyIfNoCookie() $mw->call(); $this->assertEquals(array(), $_SESSION); } + + public function testSerializingTooLongValueWritesLogAndDoesntCreateCookie() + { + \Slim\Environment::mock(array( + 'SCRIPT_NAME' => '/index.php', + 'PATH_INFO' => '/foo' + )); + + $logWriter = $this->getMockBuilder('Slim\LogWriter') + ->disableOriginalConstructor() + ->getMock(); + + $logWriter->expects($this->once()) + ->method('write') + ->with('WARNING! Slim\Middleware\SessionCookie data size is larger than 4KB. Content save failed.', \Slim\Log::ERROR); + + $app = new \Slim\Slim(array( + 'log.writer' => $logWriter + )); + + $app->get('/foo', function () { + $_SESSION['test'] = $this->getTooLongValue(); + echo "Success"; + }); + + $mw = new \Slim\Middleware\SessionCookie(array('expires' => '10 years')); + $mw->setApplication($app); + $mw->setNextMiddleware($app); + $mw->call(); + list($status, $header, $body) = $app->response()->finalize(); + $this->assertFalse($app->response->cookies->has('slim_session')); + } + + /** + * Generated by http://www.random.org/strings/ + */ + protected function getTooLongValue() + { + return << Date: Thu, 19 Sep 2013 17:22:00 -0500 Subject: [PATCH 12/16] Fixes \Slim\Slim::setCookie() method signature error. --- Slim/Middleware/SessionCookie.php | 14 ++++++-------- tests/Middleware/SessionCookieTest.php | 2 +- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/Slim/Middleware/SessionCookie.php b/Slim/Middleware/SessionCookie.php index cff435328..e40fe1f2d 100644 --- a/Slim/Middleware/SessionCookie.php +++ b/Slim/Middleware/SessionCookie.php @@ -145,14 +145,12 @@ protected function saveSession() } else { $this->app->setCookie( $this->settings['name'], - array( - 'value' => $value, - 'domain' => $this->settings['domain'], - 'path' => $this->settings['path'], - 'expires' => $this->settings['expires'], - 'secure' => $this->settings['secure'], - 'httponly' => $this->settings['httponly'] - ) + $value, + $this->settings['expires'], + $this->settings['path'], + $this->settings['domain'], + $this->settings['secure'], + $this->settings['httponly'] ); } session_destroy(); diff --git a/tests/Middleware/SessionCookieTest.php b/tests/Middleware/SessionCookieTest.php index e32860419..b8b4868cc 100644 --- a/tests/Middleware/SessionCookieTest.php +++ b/tests/Middleware/SessionCookieTest.php @@ -67,7 +67,7 @@ public function testSessionCookieIsCreated() list($status, $header, $body) = $app->response()->finalize(); $this->assertTrue($app->response->cookies->has('slim_session')); $cookie = $app->response->cookies->get('slim_session'); - $this->assertEquals(serialize(array('foo' => 'bar')), $cookie['value']['value']); + $this->assertEquals(serialize(array('foo' => 'bar')), $cookie['value']); } /** From 13c38a38e30b27f4d7c5deeac6135cfccc3fca90 Mon Sep 17 00:00:00 2001 From: Jeremy Kendall Date: Thu, 19 Sep 2013 17:40:48 -0500 Subject: [PATCH 13/16] Whitespace fixes --- Slim/Middleware/SessionCookie.php | 8 ++++---- tests/Middleware/SessionCookieTest.php | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Slim/Middleware/SessionCookie.php b/Slim/Middleware/SessionCookie.php index e40fe1f2d..e0e7dafc5 100644 --- a/Slim/Middleware/SessionCookie.php +++ b/Slim/Middleware/SessionCookie.php @@ -41,9 +41,9 @@ * variable to/from an HTTP cookie. * * You should NEVER store sensitive data in a client-side cookie - * in any format, encrypted (with cookies.encrypt) or not. If you - * need to store sensitive user information in a session, you should - * rely on PHP's native session implementation, or use other middleware + * in any format, encrypted (with cookies.encrypt) or not. If you + * need to store sensitive user information in a session, you should + * rely on PHP's native session implementation, or use other middleware * to store session data in a database or alternative server-side cache. * * Because this class stores serialized session data in an HTTP cookie, @@ -64,7 +64,7 @@ class SessionCookie extends \Slim\Middleware /** * Constructor * - * @param array $settings + * @param array $settings */ public function __construct($settings = array()) { diff --git a/tests/Middleware/SessionCookieTest.php b/tests/Middleware/SessionCookieTest.php index b8b4868cc..042703ded 100644 --- a/tests/Middleware/SessionCookieTest.php +++ b/tests/Middleware/SessionCookieTest.php @@ -73,8 +73,8 @@ public function testSessionCookieIsCreated() /** * Test $_SESSION is populated from an encrypted HTTP cookie * - * The encrypted cookie contains the serialized array ['foo' => 'bar']. The - * global secret, cipher, and cipher mode are assumed to be the default + * The encrypted cookie contains the serialized array ['foo' => 'bar']. The + * global secret, cipher, and cipher mode are assumed to be the default * values. */ public function testSessionIsPopulatedFromEncryptedCookie() @@ -101,7 +101,7 @@ public function testSessionIsPopulatedFromEncryptedCookie() /** * Test $_SESSION is populated from an unencrypted HTTP cookie * - * The unencrypted cookie contains the serialized array ['foo' => 'bar']. + * The unencrypted cookie contains the serialized array ['foo' => 'bar']. * The global cookies.encrypt setting is set to false */ public function testSessionIsPopulatedFromUnencryptedCookie() From 31eee4986042fa94ed753d83555953825b2b8d90 Mon Sep 17 00:00:00 2001 From: Jeremy Kendall Date: Fri, 20 Sep 2013 12:49:54 -0500 Subject: [PATCH 14/16] Fixes travisCI PHP 5.3 test failure (build 299). --- tests/Middleware/SessionCookieTest.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/Middleware/SessionCookieTest.php b/tests/Middleware/SessionCookieTest.php index 042703ded..53a4e3fdc 100644 --- a/tests/Middleware/SessionCookieTest.php +++ b/tests/Middleware/SessionCookieTest.php @@ -195,8 +195,10 @@ public function testSerializingTooLongValueWritesLogAndDoesntCreateCookie() 'log.writer' => $logWriter )); - $app->get('/foo', function () { - $_SESSION['test'] = $this->getTooLongValue(); + $tooLongValue = $this->getTooLongValue(); + + $app->get('/foo', function () use ($tooLongValue) { + $_SESSION['test'] = $tooLongValue; echo "Success"; }); From e8d0be97eb881f674727e324f1c51643181a5e9d Mon Sep 17 00:00:00 2001 From: Filipe Rodrigues Date: Wed, 7 Aug 2013 12:55:37 +0100 Subject: [PATCH 15/16] Add test for `urlFor` with regular expressions syntax. --- tests/RouterTest.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/RouterTest.php b/tests/RouterTest.php index 4f61c4b33..088c02f8a 100644 --- a/tests/RouterTest.php +++ b/tests/RouterTest.php @@ -222,14 +222,22 @@ public function testGetMatchedRoutes() public function testUrlFor() { $router = new \Slim\Router(); + $route1 = new \Slim\Route('/hello/:first/:last', function () {}); - $route1 = $route1->via('GET')->name('hello'); + $route1 = $route1->via('GET')->name('hello'); + + $route2 = new \Slim\Route('/path/(:foo\.:bar)', function () {}); + $route2 = $route2->via('GET')->name('regexRoute'); $routes = new \ReflectionProperty($router, 'namedRoutes'); $routes->setAccessible(true); - $routes->setValue($router, array('hello' => $route1)); + $routes->setValue($router, array( + 'hello' => $route1, + 'regexRoute' => $route2 + )); $this->assertEquals('/hello/Josh/Lockhart', $router->urlFor('hello', array('first' => 'Josh', 'last' => 'Lockhart'))); + $this->assertEquals('/path/Hello.Josh', $router->urlFor('regexRoute', array('foo' => 'Hello', 'bar' => 'Josh'))); } public function testUrlForIfNoSuchRoute() From 6ddc9485dda54f3478d88752bc2d643d143b6d24 Mon Sep 17 00:00:00 2001 From: Filipe Rodrigues Date: Wed, 7 Aug 2013 12:58:53 +0100 Subject: [PATCH 16/16] Removes remnant of escaped special characters. Before `urlFor` would return '/path/Hello\.Josh' for a pattern '/path/(:foo\.:bar)' with parameters `('foo' => 'Hello', 'bar' => 'Josh')`, when the expected return value is '/path/Hello.Josh' (this test was introduced in commit cc722cc). --- Slim/Router.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Slim/Router.php b/Slim/Router.php index 2f47fb895..8f4c4fce2 100644 --- a/Slim/Router.php +++ b/Slim/Router.php @@ -192,8 +192,8 @@ public function urlFor($name, $params = array()) } $pattern = preg_replace($search, $params, $this->getNamedRoute($name)->getPattern()); - //Remove remnants of unpopulated, trailing optional pattern segments - return preg_replace('#\(/?:.+\)|\(|\)#', '', $pattern); + //Remove remnants of unpopulated, trailing optional pattern segments, escaped special characters + return preg_replace('#\(/?:.+\)|\(|\)|\\\\#', '', $pattern); } /**