Permalink
Browse files

Multiple improvements to the URI class

(thanks to @sourcejedi, PR #1326 for most of the ideas)

 - Renamed _detect_uri() and _parse_cli_args() to _parse_request_uri() and _parse_argv() respectively.
 - Added _parse_query_string() which allows us to detect the URI path from QUERY_STRING much like it is done in _parse_request_uri().

(the above changes also allow for a simpler logic in the case where the *uri_protocol* setting is not set to 'AUTO')

 - Updated application/config/config.php with a better list of the *uri_protocol* options.
 - Added _reset_query_string() to aid in re-processing  from the QUERY_STRING (utilized in _parse_request_uri() and _parse_query_string()).
  • Loading branch information...
1 parent d4516e3 commit f2b19fee7876708c7a7bb5cba6b7df682a9d2a53 @narfbg narfbg committed Oct 31, 2012
Showing with 85 additions and 34 deletions.
  1. +6 −5 application/config/config.php
  2. +75 −28 system/core/URI.php
  3. +4 −1 user_guide_src/source/changelog.rst
View
11 application/config/config.php
@@ -62,11 +62,12 @@
| URI string. The default setting of 'AUTO' works for most servers.
| If your links do not seem to work, try one of the other delicious flavors:
|
-| 'AUTO' Default - auto detects
-| 'PATH_INFO' Uses the PATH_INFO
-| 'QUERY_STRING' Uses the QUERY_STRING
-| 'REQUEST_URI' Uses the REQUEST_URI
-| 'ORIG_PATH_INFO' Uses the ORIG_PATH_INFO
+| 'AUTO' Default - auto detects
+| 'CLI' or 'argv' Uses $_SERVER['argv'] (for php-cli only)
+| 'REQUEST_URI' Uses $_SERVER['REQUEST_URI']
+| 'PATH_INFO' Uses $_SERVER['PATH_INFO']
+| 'QUERY_STRING' Uses $_SERVER['QUERY_STRING']
+| 'ORIG_PATH_INFO' Uses $_SERVER['ORIG_PATH_INFO']
|
*/
$config['uri_protocol'] = 'AUTO';
View
103 system/core/URI.php
@@ -98,31 +98,30 @@ public function _fetch_uri_string()
// Is the request coming from the command line?
if ($this->_is_cli_request())
{
- $this->_set_uri_string($this->_parse_cli_args());
+ $this->_set_uri_string($this->_parse_argv());
return;
}
// Let's try the REQUEST_URI first, this will work in most situations
- if ($uri = $this->_detect_uri())
+ if (($uri = $this->_parse_request_uri()) !== '')
{
$this->_set_uri_string($uri);
return;
}
// Is there a PATH_INFO variable?
// Note: some servers seem to have trouble with getenv() so we'll test it two ways
- $path = isset($_SERVER['PATH_INFO']) ? $_SERVER['PATH_INFO'] : @getenv('PATH_INFO');
- if (trim($path, '/') !== '' && $path !== '/'.SELF)
+ $uri = isset($_SERVER['PATH_INFO']) ? $_SERVER['PATH_INFO'] : @getenv('PATH_INFO');
+ if (trim($uri, '/') !== '' && $uri !== '/'.SELF)
{
$this->_set_uri_string($path);
@sourcejedi
sourcejedi Oct 31, 2012

Does this need changing to $this->_set_uri_string($uri)?

return;
}
// No PATH_INFO?... What about QUERY_STRING?
- $path = isset($_SERVER['QUERY_STRING']) ? $_SERVER['QUERY_STRING'] : @getenv('QUERY_STRING');
- if (trim($path, '/') !== '')
+ if (($uri = $this->_parse_query_string()) !== '')
{
- $this->_set_uri_string($path);
+ $this->_set_uri_string($uri);
return;
}
@@ -140,19 +139,19 @@ public function _fetch_uri_string()
$uri = strtoupper($this->config->item('uri_protocol'));
- if ($uri === 'REQUEST_URI')
+ if ($uri === 'CLI')
{
- $this->_set_uri_string($this->_detect_uri());
+ $this->_set_uri_string($this->_parse_argv());
return;
}
- elseif ($uri === 'CLI')
+ elseif (method_exists($this, ($method = '_parse_'.strtolower($uri))))
{
- $this->_set_uri_string($this->_parse_cli_args());
+ $this->_set_uri_string($this->$method());
return;
}
- $path = isset($_SERVER[$uri]) ? $_SERVER[$uri] : @getenv($uri);
- $this->_set_uri_string($path);
+ $uri = isset($_SERVER[$uri]) ? $_SERVER[$uri] : @getenv($uri);
+ $this->_set_uri_string($uri);
}
// --------------------------------------------------------------------
@@ -172,13 +171,15 @@ protected function _set_uri_string($str)
// --------------------------------------------------------------------
/**
- * Detects URI
+ * Parse REQUEST_URI
*
- * Will detect the URI automatically and fix the query string if necessary.
+ * Will parse REQUEST_URI and automatically detect the URI from it,
+ * while fixing the query string if necessary.
*
+ * @used-by CI_URI::_fetch_uri_string()
* @return string
*/
- protected function _detect_uri()
+ protected function _parse_request_uri()
{
if ( ! isset($_SERVER['REQUEST_URI'], $_SERVER['SCRIPT_NAME']))
{
@@ -197,10 +198,10 @@ protected function _detect_uri()
{
$uri = (string) substr($uri, strlen(dirname($_SERVER['SCRIPT_NAME'])));
}
+
// This section ensures that even on servers that require the URI to be in the query string (Nginx) a correct
// URI is found, and also fixes the QUERY_STRING server var and $_GET array.
-
- if ($uri === '' && strncmp($query, '/', 1) === 0)
+ if (trim($uri, '/') === '' && strncmp($query, '/', 1) === 0)
{
$query = explode('?', $query, 2);
$uri = $query[0];
@@ -211,14 +212,7 @@ protected function _detect_uri()
$_SERVER['QUERY_STRING'] = $query;
}
- if ($_SERVER['QUERY_STRING'] === '')
- {
- $_GET = array();
- }
- else
- {
- parse_str($_SERVER['QUERY_STRING'], $_GET);
- }
+ $this->_reset_query_string();
if ($uri === '/' OR $uri === '')
{
@@ -232,6 +226,59 @@ protected function _detect_uri()
// --------------------------------------------------------------------
/**
+ * Parse QUERY_STRING
+ *
+ * Will parse QUERY_STRING and automatically detect the URI from it.
+ *
+ * @used-by CI_URI::_fetch_uri_string()
+ * @return string
+ */
+ protected function _parse_query_string()
+ {
+ $uri = isset($_SERVER['QUERY_STRING']) ? $_SERVER['QUERY_STRING'] : @getenv('QUERY_STRING');
+
+ if (trim($uri, '/') === '')
+ {
+ return '';
+ }
+ elseif (strncmp($uri, '/', 1) === 0)
+ {
+ $uri = explode('?', $uri, 2);
+ $_SERVER['QUERY_STRING'] = isset($uri[1]) ? $uri[1] : '';
+ $uri = $uri[0];
+ }
+ $this->_reset_query_string();
+
+ return str_replace(array('//', '../'), '/', trim($uri, '/'));
+ }
+
+ // --------------------------------------------------------------------
+
+ /**
+ * Reset QUERY_STRING
+ *
+ * Re-processes QUERY_STRING to and fetches the real GET values from it.
+ * Useful for cases where we got the URI path from it's query string.
+ *
+ * @used-by CI_URI::_parse_request_uri()
+ * @used-by CI_URI::_parse_query_string()
+ * @return void
+ */
+ protected function _reset_query_string()
+ {
+ if ($_SERVER['QUERY_STRING'] === '')
+ {
+ $_GET = array();
+ }
+ else
+ {
+ parse_str($_SERVER['QUERY_STRING']);
@sourcejedi
sourcejedi Oct 31, 2012

This needs to be parse_str($_SERVER['QUERY_STRING'], $_GET)

@sourcejedi
sourcejedi Oct 31, 2012

Also, I believe I remarked somewhere in my commits, parse_str() seems quite capable of handling the empty string. I don't think that case needs special hndling.

+ }
+ }
+
+ // --------------------------------------------------------------------
+
+ /**
* Is CLI Request?
*
* Duplicate of method from the Input class to test to see if
@@ -255,10 +302,10 @@ protected function _is_cli_request()
*
* @return string
*/
- protected function _parse_cli_args()
+ protected function _parse_argv()
{
$args = array_slice($_SERVER['argv'], 1);
- return $args ? '/'.implode('/', $args) : '';
+ return $args ? implode('/', $args) : '';
}
// --------------------------------------------------------------------
View
5 user_guide_src/source/changelog.rst
@@ -232,7 +232,10 @@ Release Date: Not Released
- :doc:`URI Library <libraries/uri>` changes include:
- Changed private methods to protected so that MY_URI can override them.
- - Changed ``_detect_uri()`` to accept absolute URIs for compatibility with HTTP/1.1 as per `RFC2616 <http://www.ietf.org/rfc/rfc2616.txt>`.
+ - Renamed internal method ``_parse_cli_args()`` to ``_parse_argv()``.
+ - Renamed internal method ``_detect_uri()`` to ``_parse_request_uri()``.
+ - Changed ``_parse_request_uri()`` to accept absolute URIs for compatibility with HTTP/1.1 as per `RFC2616 <http://www.ietf.org/rfc/rfc2616.txt>`.
+ - Added protected method ``_parse_query_string()`` to URI paths in the the **QUERY_STRING** value, like ``_parse_request_uri()`` does.
- Removed ``CI_CORE`` boolean constant from *CodeIgniter.php* (no longer Reactor and Core versions).
- :doc:`Loader Library <libraries/loader>` changes include:
- Added method ``get_vars()`` to the Loader to retrieve all variables loaded with ``$this->load->vars()``.

0 comments on commit f2b19fe

Please sign in to comment.