From 440cca59013e65b8d6a20425d863db7d50b09336 Mon Sep 17 00:00:00 2001 From: Mike Cao Date: Thu, 10 Jan 2013 09:39:21 -0800 Subject: [PATCH 1/9] Minor code changes --- flight/net/Request.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/flight/net/Request.php b/flight/net/Request.php index 3baefe1..26c991a 100644 --- a/flight/net/Request.php +++ b/flight/net/Request.php @@ -46,7 +46,7 @@ class Request { 'base' => str_replace('\\', '/', dirname(getenv('SCRIPT_NAME'))), 'method' => getenv('REQUEST_METHOD') ?: 'GET', 'referrer' => getenv('HTTP_REFERER') ?: '', - 'ip' => getenv('REMOTE_ADDR'), + 'ip' => getenv('REMOTE_ADDR') ?: '', 'ajax' => getenv('HTTP_X_REQUESTED_WITH') == 'XMLHttpRequest', 'scheme' => getenv('SERVER_PROTOCOL') ?: 'HTTP/1.1', 'user_agent' => getenv('HTTP_USER_AGENT') ?: '', @@ -59,7 +59,7 @@ class Request { 'files' => new Collection($_FILES), 'secure' => getenv('HTTPS') && getenv('HTTPS') != 'off', 'accept' => getenv('HTTP_ACCEPT'), - 'proxy' => $this->getProxyIpAddress() + 'proxy_ip' => $this->getProxyIpAddress() ); } @@ -121,16 +121,19 @@ class Request { 'HTTP_FORWARDED_FOR', 'HTTP_FORWARDED' ); + $flags = \FILTER_FLAG_NO_PRIV_RANGE | \FILTER_FLAG_NO_RES_RANGE; foreach ($forwarded as $key) { if (array_key_exists($key, $_SERVER)) { sscanf($_SERVER[$key], '%[^,]', $ip); - if(filter_var($ip, FILTER_VALIDATE_IP, $flags) !== false) { + if (filter_var($ip, \FILTER_VALIDATE_IP, $flags) !== false) { return $ip; } } } + + return ''; } } ?> From ecc0b87024d2a4d06b65023fa2d25b372212d895 Mon Sep 17 00:00:00 2001 From: Mike Cao Date: Thu, 10 Jan 2013 10:00:44 -0800 Subject: [PATCH 2/9] Updated getTemplate to allow subdirectories. --- flight/template/View.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/flight/template/View.php b/flight/template/View.php index d3d83f9..1f44a64 100644 --- a/flight/template/View.php +++ b/flight/template/View.php @@ -146,7 +146,10 @@ class View { * @return string Template file location */ public function getTemplate($file) { - return $this->path.'/'.basename($file, '.php').'.php'; + if ((substr($file, -4) != '.php')) { + $file .= '.php'; + } + return $this->path.'/'.$file; } /** From 98d57a82b4c0c5802c55dbc8390223dd3ed8cdfc Mon Sep 17 00:00:00 2001 From: Mike Cao Date: Fri, 11 Jan 2013 11:38:17 -0800 Subject: [PATCH 3/9] $_GET should not be overwritten. --- flight/net/Request.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/flight/net/Request.php b/flight/net/Request.php index 26c991a..c3da429 100644 --- a/flight/net/Request.php +++ b/flight/net/Request.php @@ -58,7 +58,7 @@ class Request { 'cookies' => new Collection($_COOKIE), 'files' => new Collection($_FILES), 'secure' => getenv('HTTPS') && getenv('HTTPS') != 'off', - 'accept' => getenv('HTTP_ACCEPT'), + 'accept' => getenv('HTTP_ACCEPT') ?: '', 'proxy_ip' => $this->getProxyIpAddress() ); } @@ -84,7 +84,7 @@ class Request { $this->url = '/'; } else { - $_GET = self::parseQuery($this->url); + $_GET += self::parseQuery($this->url); $this->query->setData($_GET); } From 58303b2ae3b26c7d8f4feaee86d8a74f7df9ff71 Mon Sep 17 00:00:00 2001 From: Mike Cao Date: Sat, 12 Jan 2013 12:50:11 -0800 Subject: [PATCH 4/9] Added test for Request class. --- tests/RequestTest.php | 80 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 tests/RequestTest.php diff --git a/tests/RequestTest.php b/tests/RequestTest.php new file mode 100644 index 0000000..a9537a5 --- /dev/null +++ b/tests/RequestTest.php @@ -0,0 +1,80 @@ + + * @license http://www.opensource.org/licenses/mit-license.php + */ + +require_once 'PHPUnit/Autoload.php'; +require_once __DIR__.'/../flight/net/Request.php'; + +class RequestTest extends PHPUnit_Framework_TestCase +{ + private $request; + + function setUp() { + putenv('REQUEST_URI=/'); + putenv('REQUEST_METHOD=GET'); + putenv('HTTP_X_REQUESTED_WITH=XMLHttpRequest'); + putenv('REQUEST_URI=/'); + putenv('REMOTE_ADDR=8.8.8.8'); + putenv('HTTPS=on'); + $_SERVER['HTTP_X_FORWARDED_FOR'] = '32.32.32.32'; + + $this->request = new \flight\net\Request(); + } + + function testDefaults() { + $this->assertEquals('/', $this->request->url); + $this->assertEquals('', $this->request->base); + $this->assertEquals('GET', $this->request->method); + $this->assertEquals('', $this->request->referrer); + $this->assertEquals(true, $this->request->ajax); + $this->assertEquals('HTTP/1.1', $this->request->scheme); + $this->assertEquals('', $this->request->type); + $this->assertEquals(0, $this->request->length); + $this->assertEquals(true, $this->request->secure); + $this->assertEquals('', $this->request->accept); + } + + function testIpAddress() { + $this->assertEquals('8.8.8.8', $this->request->ip); + $this->assertEquals('32.32.32.32', $this->request->proxy_ip); + } + + function testSubdirectory() { + putenv('SCRIPT_NAME=/subdir/index.php'); + + $request = new \flight\net\Request(); + + $this->assertEquals('/subdir', $request->base); + } + + function testQueryParameters() { + putenv('REQUEST_URI=/page?id=1&name=bob'); + + $request = new \flight\net\Request(); + + $this->assertEquals('/page?id=1&name=bob', $request->url); + $this->assertEquals(1, $request->query->id); + $this->assertEquals('bob', $request->query->name); + } + + function testCollections() { + putenv('REQUEST_URI=/page?id=1'); + + $_GET['q'] = 1; + $_POST['q'] = 1; + $_COOKIE['q'] = 1; + $_FILES['q'] = 1; + + $request = new \flight\net\Request(); + + $this->assertEquals(1, $request->query->q); + $this->assertEquals(1, $request->query->id); + $this->assertEquals(1, $request->data->q); + $this->assertEquals(1, $request->cookies->q); + $this->assertEquals(1, $request->files->q); + } +} From 61ce37ca641c475ef205d56e46eaf7943453dacb Mon Sep 17 00:00:00 2001 From: Mike Cao Date: Thu, 17 Jan 2013 00:54:24 -0800 Subject: [PATCH 5/9] Updated router pattern matching. --- flight/net/Router.php | 45 ++++++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/flight/net/Router.php b/flight/net/Router.php index d6e7519..26f8e84 100644 --- a/flight/net/Router.php +++ b/flight/net/Router.php @@ -71,7 +71,7 @@ class Router { } /** - * Tries to match a requst to a route. Also parses named parameters in the url. + * Tries to match a request to a route. Also parses named parameters in the url. * * @param string $pattern URL pattern * @param string $url Requested URL @@ -81,27 +81,36 @@ class Router { $ids = array(); // Build the regex for matching - $regex = '/^'.implode('\/', array_map( - function($str) use (&$ids){ - if ($str == '*') { - $str = '(.*)'; + $regex = preg_replace_callback( + '#@([\w]+)(:([^/\(\)]*))?#', + function($matches) use (&$ids) { + $ids[$matches[1]] = null; + if (isset($matches[3])) { + return '(?P<'.$matches[1].'>'.$matches[3].')'; } - else if ($str != null && $str{0} == '@') { - if (preg_match('/@(\w+)(\:([^\/|\(]*))?([\(|\)]+)?/', $str, $matches)) { - $ids[$matches[1]] = null; - return '(?P<'.$matches[1].'>' - .(!empty($matches[3]) ? $matches[3] : '[^(\/|\?)]+') - .')' - .(!empty($matches[4]) ? str_replace(')',')?',$matches[4]) : ''); - } - } - return $str; + return '(?P<'.$matches[1].'>[^/\?]+)'; }, - explode('/', $pattern) - )).'\/?(?:\?.*)?$/i'; + $pattern + ); + + // Allow trailing slash + if (substr($pattern, -1) === '/') { + $regex .= '?'; + } + else { + $regex .= '/?'; + } + + // Replace wildcard + if (substr($pattern, -1) === '*') { + $regex = str_replace('*', '.+?', $pattern); + } + + // Convert optional parameters + $regex = str_replace(')', ')?', $regex); // Attempt to match route and named parameters - if (preg_match($regex, $url, $matches)) { + if (preg_match('#^'.$regex.'(?:\?.*)?$#i', $url, $matches)) { foreach ($ids as $k => $v) { $this->params[$k] = (array_key_exists($k, $matches)) ? urldecode($matches[$k]) : null; } From ae32c228d1d17c36834306d0842fd4b34195f477 Mon Sep 17 00:00:00 2001 From: Mike Cao Date: Thu, 17 Jan 2013 00:58:31 -0800 Subject: [PATCH 6/9] Changed PHPUnit references. Added new tests for router. --- tests/AutoloadTest.php | 2 +- tests/DispatcherTest.php | 2 +- tests/FilterTest.php | 2 +- tests/LoaderTest.php | 2 +- tests/MapTest.php | 2 +- tests/RegisterTest.php | 2 +- tests/RenderTest.php | 4 +-- tests/RouterTest.php | 46 ++++++++++++++++++++-------- tests/VariableTest.php | 2 +- tests/ViewTest.php | 6 ++-- tests/views/{ => layouts}/layout.php | 0 11 files changed, 46 insertions(+), 24 deletions(-) rename tests/views/{ => layouts}/layout.php (100%) diff --git a/tests/AutoloadTest.php b/tests/AutoloadTest.php index b65314d..f73f9f6 100644 --- a/tests/AutoloadTest.php +++ b/tests/AutoloadTest.php @@ -6,7 +6,7 @@ * @license http://www.opensource.org/licenses/mit-license.php */ -require_once 'PHPUnit.php'; +require_once 'PHPUnit/Autoload.php'; require_once __DIR__.'/../flight/Flight.php'; class AutoloadTest extends PHPUnit_Framework_TestCase diff --git a/tests/DispatcherTest.php b/tests/DispatcherTest.php index e183553..a554657 100644 --- a/tests/DispatcherTest.php +++ b/tests/DispatcherTest.php @@ -6,7 +6,7 @@ * @license http://www.opensource.org/licenses/mit-license.php */ -require_once 'PHPUnit.php'; +require_once 'PHPUnit/Autoload.php'; require_once __DIR__.'/../flight/core/Dispatcher.php'; require_once __DIR__.'/classes/Hello.php'; diff --git a/tests/FilterTest.php b/tests/FilterTest.php index 6dbcd7c..df84d28 100644 --- a/tests/FilterTest.php +++ b/tests/FilterTest.php @@ -6,7 +6,7 @@ * @license http://www.opensource.org/licenses/mit-license.php */ -require_once 'PHPUnit.php'; +require_once 'PHPUnit/Autoload.php'; require_once __DIR__.'/../flight/Flight.php'; class FilterTest extends PHPUnit_Framework_TestCase diff --git a/tests/LoaderTest.php b/tests/LoaderTest.php index c3a1667..3e313b6 100644 --- a/tests/LoaderTest.php +++ b/tests/LoaderTest.php @@ -6,7 +6,7 @@ * @license http://www.opensource.org/licenses/mit-license.php */ -require_once 'PHPUnit.php'; +require_once 'PHPUnit/Autoload.php'; require_once __DIR__.'/../flight/core/Loader.php'; class LoaderTest extends PHPUnit_Framework_TestCase diff --git a/tests/MapTest.php b/tests/MapTest.php index 070d358..ce0a574 100644 --- a/tests/MapTest.php +++ b/tests/MapTest.php @@ -6,7 +6,7 @@ * @license http://www.opensource.org/licenses/mit-license.php */ -require_once 'PHPUnit.php'; +require_once 'PHPUnit/Autoload.php'; require_once __DIR__.'/../flight/Flight.php'; require_once __DIR__.'/classes/Hello.php'; diff --git a/tests/RegisterTest.php b/tests/RegisterTest.php index 20acb2f..b1e93b3 100644 --- a/tests/RegisterTest.php +++ b/tests/RegisterTest.php @@ -6,7 +6,7 @@ * @license http://www.opensource.org/licenses/mit-license.php */ -require_once 'PHPUnit.php'; +require_once 'PHPUnit/Autoload.php'; require_once __DIR__.'/../flight/Flight.php'; class RegisterTest extends PHPUnit_Framework_TestCase diff --git a/tests/RenderTest.php b/tests/RenderTest.php index 095bb74..34c0819 100644 --- a/tests/RenderTest.php +++ b/tests/RenderTest.php @@ -6,7 +6,7 @@ * @license http://www.opensource.org/licenses/mit-license.php */ -require_once 'PHPUnit.php'; +require_once 'PHPUnit/Autoload.php'; require_once __DIR__.'/../flight/Flight.php'; class RenderTest extends PHPUnit_Framework_TestCase @@ -26,7 +26,7 @@ class RenderTest extends PHPUnit_Framework_TestCase // Renders a view into a layout function testRenderLayout(){ Flight::render('hello', array('name' => 'Bob'), 'content'); - Flight::render('layout'); + Flight::render('layouts/layout'); $this->expectOutputString('Hello, Bob!'); } diff --git a/tests/RouterTest.php b/tests/RouterTest.php index e696fc2..8c95d6a 100644 --- a/tests/RouterTest.php +++ b/tests/RouterTest.php @@ -6,7 +6,7 @@ * @license http://www.opensource.org/licenses/mit-license.php */ -require_once 'PHPUnit.php'; +require_once 'PHPUnit/Autoload.php'; require_once __DIR__.'/../flight/net/Router.php'; require_once __DIR__.'/../flight/net/Request.php'; @@ -23,11 +23,15 @@ class RouterTest extends PHPUnit_Framework_TestCase private $request; function setUp(){ - Flight::init(); $this->router = new \flight\net\Router(); $this->request = new \flight\net\Request(); } + // Simple output + function ok(){ + echo 'OK'; + } + // Checks if a route was matched function check($str = 'OK'){ $callback = $this->router->route($this->request); @@ -42,7 +46,7 @@ class RouterTest extends PHPUnit_Framework_TestCase // Default route function testDefaultRoute(){ - $this->router->map('/', 'ok'); + $this->router->map('/', array($this, 'ok')); $this->request->url = '/'; $this->check(); @@ -50,7 +54,7 @@ class RouterTest extends PHPUnit_Framework_TestCase // Simple path function testPathRoute() { - $this->router->map('/path', 'ok'); + $this->router->map('/path', array($this, 'ok')); $this->request->url = '/path'; $this->check(); @@ -58,7 +62,7 @@ class RouterTest extends PHPUnit_Framework_TestCase // POST route function testPostRoute(){ - $this->router->map('POST /', 'ok'); + $this->router->map('POST /', array($this, 'ok')); $this->request->url = '/'; $this->request->method = 'POST'; @@ -67,7 +71,7 @@ class RouterTest extends PHPUnit_Framework_TestCase // Either GET or POST route function testGetPostRoute(){ - $this->router->map('GET|POST /', 'ok'); + $this->router->map('GET|POST /', array($this, 'ok')); $this->request->url = '/'; $this->request->method = 'GET'; @@ -76,7 +80,7 @@ class RouterTest extends PHPUnit_Framework_TestCase // Test regular expression matching function testRegEx(){ - $this->router->map('/num/[0-9]+', 'ok'); + $this->router->map('/num/[0-9]+', array($this, 'ok')); $this->request->url = '/num/1234'; $this->check(); @@ -114,15 +118,33 @@ class RouterTest extends PHPUnit_Framework_TestCase $this->check('2000,,'); } + // Regex in optional parameters + function testRegexOptionalParameters(){ + $this->router->map('/@controller/@method(/@id:[0-9]+)', function($controller, $method, $id){ + echo "$controller,$method,$id"; + }); + + $this->request->url = '/user/delete/123'; + + $this->check('user,delete,123'); + } + + // Regex in optional parameters + function testRegexEmptyOptionalParameters(){ + $this->router->map('/@controller/@method(/@id:[0-9]+)', function($controller, $method, $id){ + echo "$controller,$method,$id"; + }); + + $this->request->url = '/user/delete/'; + + $this->check('user,delete,'); + } + // Wildcard matching function testWildcard(){ - $this->router->map('/account/*', 'ok'); + $this->router->map('/account/*', array($this, 'ok')); $this->request->url = '/account/123/abc/xyz'; $this->check(); } } - -function ok(){ - echo 'OK'; -} diff --git a/tests/VariableTest.php b/tests/VariableTest.php index d5b9d83..d20f0f2 100644 --- a/tests/VariableTest.php +++ b/tests/VariableTest.php @@ -6,7 +6,7 @@ * @license http://www.opensource.org/licenses/mit-license.php */ -require_once 'PHPUnit.php'; +require_once 'PHPUnit/Autoload.php'; require_once __DIR__.'/../flight/Flight.php'; class VariableTest extends PHPUnit_Framework_TestCase diff --git a/tests/ViewTest.php b/tests/ViewTest.php index 23f57c7..1bf3f60 100644 --- a/tests/ViewTest.php +++ b/tests/ViewTest.php @@ -6,7 +6,7 @@ * @license http://www.opensource.org/licenses/mit-license.php */ -require_once 'PHPUnit.php'; +require_once 'PHPUnit/Autoload.php'; require_once __DIR__.'/../flight/template/View.php'; class ViewTest extends PHPUnit_Framework_TestCase @@ -30,9 +30,9 @@ class ViewTest extends PHPUnit_Framework_TestCase $this->assertTrue($this->view->has('test')); $this->assertTrue(!$this->view->has('unknown')); - $this->view->clear('tests'); + $this->view->clear('test'); - $this->assertEquals(null, $this->view->get('tess')); + $this->assertEquals(null, $this->view->get('test')); } // Check if template files exist diff --git a/tests/views/layout.php b/tests/views/layouts/layout.php similarity index 100% rename from tests/views/layout.php rename to tests/views/layouts/layout.php From 1ba8770da7d82283fbea37a42ba500ba98450d64 Mon Sep 17 00:00:00 2001 From: Mike Cao Date: Thu, 17 Jan 2013 01:40:10 -0800 Subject: [PATCH 7/9] Added matched regex property. --- flight/net/Router.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/flight/net/Router.php b/flight/net/Router.php index 26f8e84..2d4d454 100644 --- a/flight/net/Router.php +++ b/flight/net/Router.php @@ -35,6 +35,13 @@ class Router { */ public $params = array(); + /** + * Matching regular expression. + * + * @var string + */ + public $regex = null; + /** * Gets mapped routes. * @@ -116,6 +123,7 @@ class Router { } $this->matched = $pattern; + $this->regex = $regex; return true; } From 65807a1f29540c3bbbd9a6f9ef9d5ace7462b277 Mon Sep 17 00:00:00 2001 From: Mike Cao Date: Thu, 17 Jan 2013 01:46:06 -0800 Subject: [PATCH 8/9] Parse optional parameters before regex. --- flight/net/Router.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/flight/net/Router.php b/flight/net/Router.php index 2d4d454..a344484 100644 --- a/flight/net/Router.php +++ b/flight/net/Router.php @@ -87,6 +87,9 @@ class Router { public function match($pattern, $url) { $ids = array(); + // Convert optional parameters + $pattern = str_replace(')', ')?', $pattern); + // Build the regex for matching $regex = preg_replace_callback( '#@([\w]+)(:([^/\(\)]*))?#', @@ -100,21 +103,18 @@ class Router { $pattern ); - // Allow trailing slash + // Fix trailing slash if (substr($pattern, -1) === '/') { $regex .= '?'; } - else { - $regex .= '/?'; - } - // Replace wildcard - if (substr($pattern, -1) === '*') { + else if (substr($pattern, -1) === '*') { $regex = str_replace('*', '.+?', $pattern); } - - // Convert optional parameters - $regex = str_replace(')', ')?', $regex); + // Allow trailing slash + else { + $regex .= '/?'; + } // Attempt to match route and named parameters if (preg_match('#^'.$regex.'(?:\?.*)?$#i', $url, $matches)) { From 4cf069a5523409ff0efb02deedf08df5547b7d51 Mon Sep 17 00:00:00 2001 From: Mike Cao Date: Thu, 17 Jan 2013 16:29:36 -0800 Subject: [PATCH 9/9] Minor code changes. --- flight/net/Router.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/flight/net/Router.php b/flight/net/Router.php index a344484..df94cc1 100644 --- a/flight/net/Router.php +++ b/flight/net/Router.php @@ -86,8 +86,7 @@ class Router { */ public function match($pattern, $url) { $ids = array(); - - // Convert optional parameters + $char = substr($pattern, -1); $pattern = str_replace(')', ')?', $pattern); // Build the regex for matching @@ -104,11 +103,11 @@ class Router { ); // Fix trailing slash - if (substr($pattern, -1) === '/') { + if ($char === '/') { $regex .= '?'; } // Replace wildcard - else if (substr($pattern, -1) === '*') { + else if ($char === '*') { $regex = str_replace('*', '.+?', $pattern); } // Allow trailing slash @@ -139,6 +138,7 @@ class Router { */ public function route(Request $request) { $this->matched = null; + $this->regex = null; $this->params = array(); $routes = isset($this->routes[$request->method]) ? $this->routes[$request->method] : array();