From e4d06faef2367df1503294d052cdf93d9fdb54a9 Mon Sep 17 00:00:00 2001 From: Samir Shah Date: Mon, 15 Jul 2013 15:29:37 +0300 Subject: [PATCH 01/14] Format the Last-Modified timestamp strictly according to the HTTP Specification. PHP's DATE_RFC1123 does not generate correct timestamps when the server is not operating on GMT. --- Slim/Slim.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Slim/Slim.php b/Slim/Slim.php index 86f7537cd..ff8e5e90a 100644 --- a/Slim/Slim.php +++ b/Slim/Slim.php @@ -754,7 +754,7 @@ public function render($template, $data = array(), $status = null) public function lastModified($time) { if (is_integer($time)) { - $this->response->headers->set('Last-Modified', date(DATE_RFC1123, $time)); + $this->response->headers->set('Last-Modified', gmdate('D, d M Y H:i:s T', $time)); if ($time === strtotime($this->request->headers->get('IF_MODIFIED_SINCE'))) { $this->halt(304); } From 9610a4a6e43c7c3a9402c7067d008047a90e2a11 Mon Sep 17 00:00:00 2001 From: = Date: Mon, 15 Jul 2013 18:43:48 -0400 Subject: [PATCH 02/14] Let Resource Locator store and return Closure values --- Slim/Helper/Set.php | 13 +++++++++++++ tests/Helper/SetTest.php | 12 ++++++++++++ 2 files changed, 25 insertions(+) diff --git a/Slim/Helper/Set.php b/Slim/Helper/Set.php index 8a8a56a03..0e7e531fc 100644 --- a/Slim/Helper/Set.php +++ b/Slim/Helper/Set.php @@ -207,4 +207,17 @@ public function singleton($key, $value) return $object; }); } + + /** + * Keep closure as value (i.e. DO NOT invoke it, just return the closure itself) + * @param string $key The value or object name + * @param Closure $callable A closure to keep from being invoked and evaluated + * @return Closure + */ + public function keep($key, \Closure $callable) + { + $this->set($key, function ($c) use ($callable) { + return $callable; + }); + } } diff --git a/tests/Helper/SetTest.php b/tests/Helper/SetTest.php index 674e2b167..8651eeaee 100644 --- a/tests/Helper/SetTest.php +++ b/tests/Helper/SetTest.php @@ -180,4 +180,16 @@ public function testGetIterator() $this->property->setValue($this->bag, $data); $this->assertInstanceOf('\ArrayIterator', $this->bag->getIterator()); } + + public function testKeep() + { + $callable = function () { + return 'foo'; + }; + $this->bag->keep('foo', $callable); + $value = $this->bag['foo']; + + $this->assertInstanceOf('Closure', $value); + $this->assertEquals('foo', $value()); + } } From baa1585005dcb347d8c4eee783ee0057f414eb38 Mon Sep 17 00:00:00 2001 From: = Date: Mon, 15 Jul 2013 18:53:31 -0400 Subject: [PATCH 03/14] Let legacy View::setData not invoke stored Closure values --- Slim/View.php | 17 ++++++++++++++++- tests/ViewTest.php | 15 +++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/Slim/View.php b/Slim/View.php index 48f2dcb2d..c25563b5f 100644 --- a/Slim/View.php +++ b/Slim/View.php @@ -103,6 +103,16 @@ public function set($key, $value) $this->data->set($key, $value); } + /** + * Set view data value as Closure with key + * @param string $key + * @param mixed $value + */ + public function keep($key, Closure $value) + { + $this->data->keep($key, $value); + } + /** * Return view data * @return array @@ -158,7 +168,12 @@ public function setData() if (count($args) === 1 && is_array($args[0])) { $this->data->replace($args[0]); } elseif (count($args) === 2) { - $this->data->set($args[0], $args[1]); + // Ensure original behavior is maintained. DO NOT invoke stored Closures. + if (is_object($args[1]) && method_exists($args[1], '__invoke')) { + $this->data->keep($args[0], $args[1]); + } else { + $this->data->set($args[0], $args[1]); + } } else { throw new \InvalidArgumentException('Cannot set View data with provided arguments. Usage: `View::setData( $key, $value );` or `View::setData([ key => value, ... ]);`'); } diff --git a/tests/ViewTest.php b/tests/ViewTest.php index 2f1b1e9e3..1f4255ad7 100644 --- a/tests/ViewTest.php +++ b/tests/ViewTest.php @@ -72,6 +72,21 @@ public function testSetDataKeyValue() $this->assertEquals(array('foo' => 'bar'), $prop->getValue($view)->all()); } + public function testSetDataKeyValueAsClosure() + { + $view = new \Slim\View(); + $prop = new \ReflectionProperty($view, 'data'); + $prop->setAccessible(true); + + $view->setData('fooClosure', function () { + return 'foo'; + }); + + $value = $prop->getValue($view)->get('fooClosure'); + $this->assertInstanceOf('Closure', $value); + $this->assertEquals('foo', $value()); + } + public function testSetDataArray() { $view = new \Slim\View(); From 8c6b0b0afa4880440ad378af8e6b41a05e1509ad Mon Sep 17 00:00:00 2001 From: = Date: Mon, 15 Jul 2013 22:06:38 -0400 Subject: [PATCH 04/14] Revise how we store Closure values in Set --- Slim/Helper/Set.php | 9 ++++----- Slim/View.php | 2 +- tests/Helper/SetTest.php | 9 ++++----- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/Slim/Helper/Set.php b/Slim/Helper/Set.php index 0e7e531fc..540931d6b 100644 --- a/Slim/Helper/Set.php +++ b/Slim/Helper/Set.php @@ -209,15 +209,14 @@ public function singleton($key, $value) } /** - * Keep closure as value (i.e. DO NOT invoke it, just return the closure itself) - * @param string $key The value or object name + * Protect closure from being directly invoked * @param Closure $callable A closure to keep from being invoked and evaluated * @return Closure */ - public function keep($key, \Closure $callable) + public function protect(\Closure $callable) { - $this->set($key, function ($c) use ($callable) { + return function () use ($callable) { return $callable; - }); + }; } } diff --git a/Slim/View.php b/Slim/View.php index c25563b5f..e5be0abe3 100644 --- a/Slim/View.php +++ b/Slim/View.php @@ -170,7 +170,7 @@ public function setData() } elseif (count($args) === 2) { // Ensure original behavior is maintained. DO NOT invoke stored Closures. if (is_object($args[1]) && method_exists($args[1], '__invoke')) { - $this->data->keep($args[0], $args[1]); + $this->data->set($args[0], $this->data->protect($args[1])); } else { $this->data->set($args[0], $args[1]); } diff --git a/tests/Helper/SetTest.php b/tests/Helper/SetTest.php index 8651eeaee..a465e1e2e 100644 --- a/tests/Helper/SetTest.php +++ b/tests/Helper/SetTest.php @@ -181,15 +181,14 @@ public function testGetIterator() $this->assertInstanceOf('\ArrayIterator', $this->bag->getIterator()); } - public function testKeep() + public function testProtect() { $callable = function () { return 'foo'; }; - $this->bag->keep('foo', $callable); - $value = $this->bag['foo']; + $result = $this->bag->protect($callable); - $this->assertInstanceOf('Closure', $value); - $this->assertEquals('foo', $value()); + $this->assertInstanceOf('\Closure', $result); + $this->assertSame($callable, $result()); } } From cccd25ee8c7204098dbf7647c2fb305bd7bc01db Mon Sep 17 00:00:00 2001 From: = Date: Mon, 15 Jul 2013 22:27:41 -0400 Subject: [PATCH 05/14] Add contributing file --- CONTRIBUTING.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 000000000..9bbb6b17c --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,20 @@ +# How to Contribute + +## Pull Requests + +1. Fork the Slim Framework repository +2. Create a new branch for each feature or improvement +3. Send a pull request from each feature branch to the **develop** branch + +It is very important to separate new features or improvements into separate feature branches, and to send a +pull request for each branch. This allows me to review and pull in new features or improvements individually. + +## Style Guide + +All pull requests must adhere to the [PSR-2 standard](https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-2-coding-style-guide.md). + +## Unit Testing + +All pull requests must be accompanied by passing unit tests and complete code coverage. The Slim Framework uses phpunit for testing. + +[Learn about PHPUnit](https://github.com/sebastianbergmann/phpunit/) From 02106a6df8eaec6ee7c33d7ec8f27f9d44db3fb5 Mon Sep 17 00:00:00 2001 From: = Date: Mon, 15 Jul 2013 22:37:10 -0400 Subject: [PATCH 06/14] Update README --- README.markdown | 1 + 1 file changed, 1 insertion(+) diff --git a/README.markdown b/README.markdown index a772db216..a6b04c0f1 100644 --- a/README.markdown +++ b/README.markdown @@ -14,6 +14,7 @@ Thank you for choosing the Slim Framework for your next project. I think you're * Route parameters with wildcards and conditions * Route redirect, halt, and pass * Route middleware +* Resource Locator and DI container * Template rendering with custom views * Flash messages * Secure cookies with AES-256 encryption From 10c27e42af22c82b831af89ae6b4bc1b809222ea Mon Sep 17 00:00:00 2001 From: Samir Shah Date: Tue, 16 Jul 2013 06:12:26 +0300 Subject: [PATCH 07/14] Format Expires header according to HTTP specification. --- Slim/Slim.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Slim/Slim.php b/Slim/Slim.php index ff8e5e90a..53ba054cc 100644 --- a/Slim/Slim.php +++ b/Slim/Slim.php @@ -820,7 +820,7 @@ public function expires($time) if (is_string($time)) { $time = strtotime($time); } - $this->response->headers->set('Expires', gmdate(DATE_RFC1123, $time)); + $this->response->headers->set('Expires', gmdate('D, d M Y H:i:s T', $time)); } /******************************************************************************** From f0484b1e4dd8b74fb326a7a86b32e3189e07b69c Mon Sep 17 00:00:00 2001 From: Addvilz Date: Tue, 16 Jul 2013 10:55:06 +0300 Subject: [PATCH 08/14] Missing __isset and __unset --- Slim/Slim.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Slim/Slim.php b/Slim/Slim.php index 53ba054cc..61a0f6eff 100644 --- a/Slim/Slim.php +++ b/Slim/Slim.php @@ -230,6 +230,10 @@ public function __set($name, $value) { $this->container[$name] = $value; } + + public function __isset($name){ + return isset($this->container[$name]); + } /** * Get application instance by name From bab9b63b67f7ad3637adcebfabdcc0e142e0b886 Mon Sep 17 00:00:00 2001 From: Addvilz Date: Tue, 16 Jul 2013 10:56:07 +0300 Subject: [PATCH 09/14] __unset --- Slim/Slim.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Slim/Slim.php b/Slim/Slim.php index 61a0f6eff..a1dc86330 100644 --- a/Slim/Slim.php +++ b/Slim/Slim.php @@ -234,6 +234,10 @@ public function __set($name, $value) public function __isset($name){ return isset($this->container[$name]); } + + public function __unset($name){ + unset($this->container[$name]); + } /** * Get application instance by name From c98c45e8e6cf82938a0bfd0c720ee84ca81d6d1d Mon Sep 17 00:00:00 2001 From: Roberts Date: Sun, 30 Jun 2013 22:15:14 +0300 Subject: [PATCH 10/14] 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 a1dc86330..b5d6e8926 100644 --- a/Slim/Slim.php +++ b/Slim/Slim.php @@ -1242,7 +1242,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 9bc4f15cce4c4c3857f6236736b9d6e1c083e519 Mon Sep 17 00:00:00 2001 From: Tapan Shah Date: Sun, 14 Jul 2013 14:22:04 -0400 Subject: [PATCH 11/14] Updated the Install docs URL --- README.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.markdown b/README.markdown index a6b04c0f1..b4bc7c9b0 100644 --- a/README.markdown +++ b/README.markdown @@ -30,7 +30,7 @@ Thank you for choosing the Slim Framework for your next project. I think you're You may install the Slim Framework with Composer (recommended) or manually. -[Read how to install Slim](http://docs.slimframework.com/pages/getting-started-install) +[Read how to install Slim](http://docs.slimframework.com/#Installation) ### System Requirements From 0c04ab416469b899b20936315d8a12be7e9e6049 Mon Sep 17 00:00:00 2001 From: Samir Shah Date: Thu, 18 Jul 2013 08:43:47 +0300 Subject: [PATCH 12/14] Modify tests for Last-Modified and Expires headers to enforce RFC compliance. --- tests/SlimTest.php | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/tests/SlimTest.php b/tests/SlimTest.php index 638da7c84..60c9ef51c 100644 --- a/tests/SlimTest.php +++ b/tests/SlimTest.php @@ -611,7 +611,7 @@ public function testLastModifiedMatch() 'REQUEST_METHOD' => 'GET', 'SCRIPT_NAME' => '/foo', //<-- Physical 'PATH_INFO' => '/bar', //<-- Virtual - 'HTTP_IF_MODIFIED_SINCE' => 'Sun, 03 Oct 2010 17:00:52 -0400', + 'HTTP_IF_MODIFIED_SINCE' => 'Sun, 03 Oct 2010 21:00:52 GMT', )); $s = new \Slim\Slim(); $s->get('/bar', function () use ($s) { @@ -629,7 +629,7 @@ public function testLastModifiedDoesNotMatch() \Slim\Environment::mock(array( 'SCRIPT_NAME' => '/foo', //<-- Physical 'PATH_INFO' => '/bar', //<-- Virtual - 'IF_MODIFIED_SINCE' => 'Sun, 03 Oct 2010 17:00:52 -0400', + 'IF_MODIFIED_SINCE' => 'Sun, 03 Oct 2010 21:00:52 GMT', )); $s = new \Slim\Slim(); $s->get('/bar', function () use ($s) { @@ -653,6 +653,25 @@ public function testLastModifiedOnlyAcceptsIntegers() $s->call(); } + /** + * Test Last Modified header format + */ + public function testLastModifiedHeaderFormat() + { + \Slim\Environment::mock(array( + 'SCRIPT_NAME' => '/foo', //<-- Physical + 'PATH_INFO' => '/bar', //<-- Virtual + )); + $s = new \Slim\Slim(); + $s->get('/bar', function () use ($s) { + $s->lastModified(1286139652); + }); + $s->call(); + list($status, $header, $body) = $s->response()->finalize(); + $this->assertTrue(isset($header['Last-Modified'])); + $this->assertEquals('Sun, 03 Oct 2010 21:00:52 GMT', $header['Last-Modified']); + } + /** * Test ETag matches */ @@ -716,7 +735,7 @@ public function testExpiresAsString() 'SCRIPT_NAME' => '/foo', //<-- Physical 'PATH_INFO' => '/bar', //<-- Virtual )); - $expectedDate = gmdate('D, d M Y', strtotime('5 days')); //Just the day, month, and year + $expectedDate = gmdate('D, d M Y H:i:s T', strtotime('5 days')); $s = new \Slim\Slim(); $s->get('/bar', function () use ($s) { $s->expires('5 days'); @@ -724,7 +743,7 @@ public function testExpiresAsString() $s->call(); list($status, $header, $body) = $s->response()->finalize(); $this->assertTrue(isset($header['Expires'])); - $this->assertEquals(0, strpos($header['Expires'], $expectedDate)); + $this->assertEquals($header['Expires'], $expectedDate); } /** @@ -737,7 +756,7 @@ public function testExpiresAsInteger() 'PATH_INFO' => '/bar', //<-- Virtual )); $fiveDaysFromNow = time() + (60 * 60 * 24 * 5); - $expectedDate = gmdate('D, d M Y', $fiveDaysFromNow); //Just the day, month, and year + $expectedDate = gmdate('D, d M Y H:i:s T', $fiveDaysFromNow); $s = new \Slim\Slim(); $s->get('/bar', function () use ($s, $fiveDaysFromNow) { $s->expires($fiveDaysFromNow); @@ -745,7 +764,7 @@ public function testExpiresAsInteger() $s->call(); list($status, $header, $body) = $s->response()->finalize(); $this->assertTrue(isset($header['Expires'])); - $this->assertEquals(0, strpos($header['Expires'], $expectedDate)); + $this->assertEquals($header['Expires'], $expectedDate); } /************************************************ From dd6aa9ab3fad0c09c665095642bda6f2a453e3ce Mon Sep 17 00:00:00 2001 From: Filipe Rodrigues Date: Thu, 18 Jul 2013 12:31:15 +0100 Subject: [PATCH 13/14] Fix issue #568: Parse string 'expires' before encoding secure cookie. The `\Slim\Http\Util::encodeSecureCookie` function requires the expires parameter to be a int. A string typed 'expires' or 'cookie.lifetime' would pass a string in this case, which causes the response cookie to be broken when parsed by `decodeSecureCookie`. --- Slim/Http/Util.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Slim/Http/Util.php b/Slim/Http/Util.php index b3a1f12d5..e85a9a43e 100644 --- a/Slim/Http/Util.php +++ b/Slim/Http/Util.php @@ -186,9 +186,15 @@ public static function serializeCookies(\Slim\Http\Headers &$headers, \Slim\Http { if ($config['cookies.encrypt']) { foreach ($cookies as $name => $settings) { + if (is_string($settings['expires'])) { + $expires = strtotime($settings['expires']); + } else { + $expires = (int) $settings['expires']; + } + $settings['value'] = static::encodeSecureCookie( $settings['value'], - $settings['expires'], + $expires, $config['cookies.secret_key'], $config['cookies.cipher'], $config['cookies.cipher_mode'] From 8af44d97489ec953f1e15a4dc435fd6ae88b1213 Mon Sep 17 00:00:00 2001 From: Filipe Rodrigues Date: Thu, 18 Jul 2013 13:14:52 +0100 Subject: [PATCH 14/14] Adds a unit test to serialize and decrypt cookies with string expires. This test complements the pull request #571 (related to issue #568). --- tests/Http/UtilTest.php | 44 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/tests/Http/UtilTest.php b/tests/Http/UtilTest.php index 9d665da0e..03035149a 100644 --- a/tests/Http/UtilTest.php +++ b/tests/Http/UtilTest.php @@ -324,6 +324,50 @@ public function testSetCookieHeaderWithNameAndValueAndDomainAndPathAndExpiresAnd $this->assertEquals('foo=bar; domain=foo.com; path=/foo; expires=' . $expiresFormat . '; secure; HttpOnly', $header['Set-Cookie']); } + /** + * Test serializeCookies and decrypt with string expires + * + * In this test a cookie with a string typed value for 'expires' is set, + * which should be parsed by `strtotime` to a timestamp when it's added to + * the headers; this timestamp should then be correctly parsed, and the + * value correctly decrypted, by `decodeSecureCookie`. + */ + public function testSerializeCookiesAndDecryptWithStringExpires() + { + $value = 'bar'; + + $headers = new \Slim\Http\Headers(); + + $settings = array( + 'cookies.encrypt' => true, + 'cookies.secret_key' => 'secret', + 'cookies.cipher' => MCRYPT_RIJNDAEL_256, + 'cookies.cipher_mode' => MCRYPT_MODE_CBC + ); + + $cookies = new \Slim\Http\Cookies(); + $cookies->set('foo', array( + 'value' => $value, + 'expires' => '1 hour' + )); + + \Slim\Http\Util::serializeCookies($headers, $cookies, $settings); + + $encrypted = $headers->get('Set-Cookie'); + $encrypted = strstr($encrypted, ';', true); + $encrypted = urldecode(substr(strstr($encrypted, '='), 1)); + + $decrypted = \Slim\Http\Util::decodeSecureCookie( + $encrypted, + $settings['cookies.secret_key'], + $settings['cookies.cipher'], + $settings['cookies.cipher_mode'] + ); + + $this->assertEquals($value, $decrypted); + $this->assertTrue($value !== $encrypted); + } + public function testDeleteCookieHeaderWithSurvivingCookie() { $header = array('Set-Cookie' => "foo=bar\none=two");