From 1a0eede079d2bba49d6ec4317691e7d14bf0c482 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Tue, 17 Dec 2013 15:38:31 -0500 Subject: [PATCH] Use $_SERVER instead of getenv. This partially reverts cec890c. In addition to other cleanup the commit changed from using $_SERVER to getenv. As @kafene said on issue #21, getenv provides a nicer API making the code clean. I.E. $val = getenv('val') ?: 'default'; Instead of: $val = isset($_SERVER['val']) ? $_SERVER['val'] : 'default'; Although this is true unfortuantly it is less reliable. Using getenv in this way assumes the application is running under some sort of CGI interface or an interface that is simulating the CGI interface. While this assumption is very often true it is not always true. For example when using [PHP built-in webserver][1] (useful for development) getenv will return false for most of these values even through $_SERVER is populated. I believe also some interfaces on Windows don't populate getenv. This means that flight is not usable in those environments. By partially reverting cec890c we re-enable flight in those environments. For Engine.php and Response.php I just directly reverted cec890c. Since request makes so much use of getenv it seemed wise to abstract the conditional checking to keep the code clean via a private function. The tests were also updated to populate $_SERVER instead of the environment via putenv. [1]: http://php.net/manual/en/features.commandline.webserver.php --- flight/Engine.php | 6 ++++-- flight/net/Request.php | 31 +++++++++++++++++++------------ flight/net/Response.php | 2 +- tests/RedirectTest.php | 2 +- tests/RequestTest.php | 19 +++++++++---------- 5 files changed, 34 insertions(+), 26 deletions(-) diff --git a/flight/Engine.php b/flight/Engine.php index 554a93f..6130c41 100644 --- a/flight/Engine.php +++ b/flight/Engine.php @@ -466,7 +466,8 @@ class Engine { $this->response()->header('ETag', $id); - if ($id === getenv('HTTP_IF_NONE_MATCH')) { + if (isset($_SERVER['HTTP_IF_NONE_MATCH']) && + $_SERVER['HTTP_IF_NONE_MATCH'] === $id) { $this->halt(304); } } @@ -479,7 +480,8 @@ class Engine { public function _lastModified($time) { $this->response()->header('Last-Modified', date(DATE_RFC1123, $time)); - if ($time === strtotime(getenv('HTTP_IF_MODIFIED_SINCE'))) { + if (isset($_SERVER['HTTP_IF_MODIFIED_SINCE']) && + strtotime($_SERVER['HTTP_IF_MODIFIED_SINCE']) === $time) { $this->halt(304); } } diff --git a/flight/net/Request.php b/flight/net/Request.php index 0100634..051bc07 100644 --- a/flight/net/Request.php +++ b/flight/net/Request.php @@ -132,23 +132,23 @@ class Request { // Default properties if (empty($config)) { $config = array( - 'url' => getenv('REQUEST_URI') ?: '/', - 'base' => str_replace(array('\\',' '), array('/','%20'), dirname(getenv('SCRIPT_NAME'))), - 'method' => getenv('REQUEST_METHOD') ?: 'GET', - 'referrer' => getenv('HTTP_REFERER') ?: '', - 'ip' => getenv('REMOTE_ADDR') ?: '', - 'ajax' => getenv('HTTP_X_REQUESTED_WITH') == 'XMLHttpRequest', - 'scheme' => getenv('SERVER_PROTOCOL') ?: 'HTTP/1.1', - 'user_agent' => getenv('HTTP_USER_AGENT') ?: '', + 'url' => $this->server('REQUEST_URI', '/'), + 'base' => str_replace(array('\\',' '), array('/','%20'), dirname($this->server('SCRIPT_NAME'))), + 'method' => $this->server('REQUEST_METHOD', 'GET'), + 'referrer' => $this->server('HTTP_REFERER'), + 'ip' => $this->server('REMOTE_ADDR'), + 'ajax' => $this->server('HTTP_X_REQUESTED_WITH') == 'XMLHttpRequest', + 'scheme' => $this->server('SERVER_PROTOCOL', 'HTTP/1.1'), + 'user_agent' => $this->server('HTTP_USER_AGENT'), 'body' => file_get_contents('php://input'), - 'type' => getenv('CONTENT_TYPE') ?: '', - 'length' => getenv('CONTENT_LENGTH') ?: 0, + 'type' => $this->server('CONTENT_TYPE'), + 'length' => $this->server('CONTENT_LENGTH', 0), 'query' => new Collection($_GET), 'data' => new Collection($_POST), 'cookies' => new Collection($_COOKIE), 'files' => new Collection($_FILES), - 'secure' => getenv('HTTPS') && getenv('HTTPS') != 'off', - 'accept' => getenv('HTTP_ACCEPT') ?: '', + 'secure' => $this->server('HTTPS', 'off') != 'off', + 'accept' => $this->server('HTTP_ACCEPT'), 'proxy_ip' => $this->getProxyIpAddress() ); } @@ -225,4 +225,11 @@ class Request { return ''; } + + /** + * Get variable from $_SERVER using $default if not provided + */ + private function server($var, $default='') { + return isset($_SERVER[$var]) ? $_SERVER[$var] : $default; + } } diff --git a/flight/net/Response.php b/flight/net/Response.php index c2d6ca9..69a12fc 100644 --- a/flight/net/Response.php +++ b/flight/net/Response.php @@ -187,7 +187,7 @@ class Response { header( sprintf( '%s %d %s', - getenv('SERVER_PROTOCOL') ?: 'HTTP/1.1', + (isset($_SERVER['SERVER_PROTOCOL']) ? $_SERVER['SERVER_PROTOCOL'] : 'HTTP/1.1'), $this->status, self::$codes[$this->status]), true, diff --git a/tests/RedirectTest.php b/tests/RedirectTest.php index a877653..d35b221 100644 --- a/tests/RedirectTest.php +++ b/tests/RedirectTest.php @@ -25,7 +25,7 @@ class RedirectTest extends PHPUnit_Framework_TestCase } function setUp() { - putenv('SCRIPT_NAME=/subdir/index.php'); + $_SERVER['SCRIPT_NAME'] = '/subdir/index.php'; $this->app = new \flight\Engine(); $this->app->set('flight.base_url', '/testdir'); diff --git a/tests/RequestTest.php b/tests/RequestTest.php index f28a96e..1b6acae 100644 --- a/tests/RequestTest.php +++ b/tests/RequestTest.php @@ -17,13 +17,12 @@ class RequestTest extends PHPUnit_Framework_TestCase private $request; function setUp() { - putenv('REQUEST_URI=/'); - putenv('SCRIPT_NAME=/index.php'); - putenv('REQUEST_METHOD=GET'); - putenv('HTTP_X_REQUESTED_WITH=XMLHttpRequest'); - putenv('REQUEST_URI=/'); - putenv('REMOTE_ADDR=8.8.8.8'); - putenv('HTTPS=on'); + $_SERVER['REQUEST_URI'] = '/'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + $_SERVER['REQUEST_METHOD'] = 'GET'; + $_SERVER['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest'; + $_SERVER['REMOTE_ADDR'] = '8.8.8.8'; + $_SERVER['HTTPS'] = 'on'; $_SERVER['HTTP_X_FORWARDED_FOR'] = '32.32.32.32'; $this->request = new \flight\net\Request(); @@ -48,7 +47,7 @@ class RequestTest extends PHPUnit_Framework_TestCase } function testSubdirectory() { - putenv('SCRIPT_NAME=/subdir/index.php'); + $_SERVER['SCRIPT_NAME'] = '/subdir/index.php'; $request = new \flight\net\Request(); @@ -56,7 +55,7 @@ class RequestTest extends PHPUnit_Framework_TestCase } function testQueryParameters() { - putenv('REQUEST_URI=/page?id=1&name=bob'); + $_SERVER['REQUEST_URI'] = '/page?id=1&name=bob'; $request = new \flight\net\Request(); @@ -66,7 +65,7 @@ class RequestTest extends PHPUnit_Framework_TestCase } function testCollections() { - putenv('REQUEST_URI=/page?id=1'); + $_SERVER['REQUEST_URI'] = '/page?id=1'; $_GET['q'] = 1; $_POST['q'] = 1;