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()).

3 files changed, 85 insertions, 34 deletions

diff --git a/application/config/config.php b/application/config/config.php
index ab1508e7b..6867cee88 100644
--- a/application/config/config.php
+++ b/application/config/config.php
@@ -62,11 +62,12 @@ $config['index_page'] = 'index.php';
 | 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';
diff --git a/system/core/URI.php b/system/core/URI.php
index 3d942eda7..6692d07a6 100644
--- a/system/core/URI.php
+++ b/system/core/URI.php
@@ -98,12 +98,12 @@ class CI_URI {
 			// 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;
@@ -111,18 +111,17 @@ class CI_URI {
 
 			// 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);
 				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 @@ class CI_URI {
 
 		$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 @@ class CI_URI {
 	// --------------------------------------------------------------------
 
 	/**
-	 * 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 @@ class CI_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 @@ class CI_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 @@ class CI_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']);
+		}
+	}
+
+	// --------------------------------------------------------------------
+
+	/**
 	 * Is CLI Request?
 	 *
 	 * Duplicate of method from the Input class to test to see if
@@ -255,10 +302,10 @@ class CI_URI {
 	 *
 	 * @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) : '';
 	}
 
 	// --------------------------------------------------------------------
diff --git a/user_guide_src/source/changelog.rst b/user_guide_src/source/changelog.rst
index 8e823d08d..4fda2903a 100644
--- a/user_guide_src/source/changelog.rst
+++ b/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()``.