From 4e6c5281a3258b3ff530a43580d1b8dc8e34dd59 Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Fri, 10 Jan 2014 19:29:49 +0200 Subject: Finally get rid of the CI_Router::_set_overrides() calls --- system/core/CodeIgniter.php | 6 ----- system/core/Router.php | 55 +++++++++++++++++++-------------------------- 2 files changed, 23 insertions(+), 38 deletions(-) (limited to 'system/core') diff --git a/system/core/CodeIgniter.php b/system/core/CodeIgniter.php index 45c3485bf..cc12f149f 100644 --- a/system/core/CodeIgniter.php +++ b/system/core/CodeIgniter.php @@ -163,12 +163,6 @@ defined('BASEPATH') OR exit('No direct script access allowed'); */ $RTR =& load_class('Router', 'core'); - // Set any routing overrides that may exist in the main index file - if (isset($routing)) - { - $RTR->_set_overrides($routing); - } - /* * ------------------------------------------------------ * Instantiate the output class diff --git a/system/core/Router.php b/system/core/Router.php index d467d60fd..cb44a3ce9 100644 --- a/system/core/Router.php +++ b/system/core/Router.php @@ -102,9 +102,32 @@ class CI_Router { */ public function __construct() { + global $routing; + $this->config =& load_class('Config', 'core'); $this->uri =& load_class('URI', 'core'); $this->_set_routing(); + + // Set any routing overrides that may exist in the main index file + if (isset($routing) && is_array($routing)) + { + if (isset($routing['directory'])) + { + $this->set_directory($routing['directory']); + } + + if ( ! empty($routing['controller'])) + { + $this->set_class($routing['controller']); + } + + if (isset($routing['function'])) + { + $routing['function'] = empty($routing['function']) ? 'index' : $routing['function']; + $this->set_method($routing['function']); + } + } + log_message('debug', 'Router Class Initialized'); } @@ -519,38 +542,6 @@ class CI_Router { return $this->directory; } - // -------------------------------------------------------------------- - - /** - * Set controller overrides - * - * @param array $routing Route overrides - * @return void - */ - public function _set_overrides($routing) - { - if ( ! is_array($routing)) - { - return; - } - - if (isset($routing['directory'])) - { - $this->set_directory($routing['directory']); - } - - if ( ! empty($routing['controller'])) - { - $this->set_class($routing['controller']); - } - - if (isset($routing['function'])) - { - $routing['function'] = empty($routing['function']) ? 'index' : $routing['function']; - $this->set_method($routing['function']); - } - } - } /* End of file Router.php */ -- cgit v1.2.3-24-g4f1b From 3d215207ceff44193e3c1888b868fc3f691718c0 Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Wed, 15 Jan 2014 11:08:47 +0200 Subject: Fix incorrect checks for the fwrite() return value ! fwrite() could trigger false-positives as it is possible for it to return 0 instead of boolean FALSE. (issue #2822) Also removed an unnecessary log level check that caused an extra space to be inserted for the INFO level. (proposed in PR #2821) --- system/core/Log.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'system/core') diff --git a/system/core/Log.php b/system/core/Log.php index b2327b8f0..ff3c63568 100644 --- a/system/core/Log.php +++ b/system/core/Log.php @@ -175,7 +175,7 @@ class CI_Log { return FALSE; } - $message .= $level.' '.($level === 'INFO' ? ' -' : '-').' '.date($this->_date_fmt).' --> '.$msg."\n"; + $message .= $level.' - '.date($this->_date_fmt).' --> '.$msg."\n"; flock($fp, LOCK_EX); fwrite($fp, $message); -- cgit v1.2.3-24-g4f1b From de14aa5a29b1b122bfd536f979dfda7f2fd9f53d Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Wed, 15 Jan 2014 15:51:08 +0200 Subject: CI_URI changes related to the 'permitted_uri_chars' setting - Initialize and cache the value in the class constructor instead of searching for it every time - Removed the preg_quote() call from _filter_uri() to allow more fine-tuning from configuration - Renamed _filter_uri() to filter_uri() - it was public anyway and using it cannot break anything Related: issue #2799 --- system/core/Router.php | 6 +++--- system/core/URI.php | 35 +++++++++++++++++++++++------------ 2 files changed, 26 insertions(+), 15 deletions(-) (limited to 'system/core') diff --git a/system/core/Router.php b/system/core/Router.php index cb44a3ce9..71530ff07 100644 --- a/system/core/Router.php +++ b/system/core/Router.php @@ -154,16 +154,16 @@ class CI_Router { { if (isset($_GET[$this->config->item('directory_trigger')]) && is_string($_GET[$this->config->item('directory_trigger')])) { - $this->set_directory(trim($this->uri->_filter_uri($_GET[$this->config->item('directory_trigger')]))); + $this->set_directory(trim($this->uri->filter_uri($_GET[$this->config->item('directory_trigger')]))); $segments[] = $this->directory; } - $this->set_class(trim($this->uri->_filter_uri($_GET[$this->config->item('controller_trigger')]))); + $this->set_class(trim($this->uri->filter_uri($_GET[$this->config->item('controller_trigger')]))); $segments[] = $this->class; if ( ! empty($_GET[$this->config->item('function_trigger')]) && is_string($_GET[$this->config->item('function_trigger')])) { - $this->set_method(trim($this->uri->_filter_uri($_GET[$this->config->item('function_trigger')]))); + $this->set_method(trim($this->uri->filter_uri($_GET[$this->config->item('function_trigger')]))); $segments[] = $this->method; } } diff --git a/system/core/URI.php b/system/core/URI.php index 5e4c80a00..3d6d202c0 100644 --- a/system/core/URI.php +++ b/system/core/URI.php @@ -69,6 +69,15 @@ class CI_URI { */ public $rsegments = array(); + /** + * Permitted URI chars + * + * PCRE character group allowed in URI segments + * + * @var string + */ + protected $_permitted_uri_chars; + /** * Class constructor * @@ -81,6 +90,12 @@ class CI_URI { public function __construct() { $this->config =& load_class('Config', 'core'); + + if ($this->config->item('enable_query_strings') !== TRUE OR is_cli()) + { + $this->_permitted_uri_chars = $this->config->item('permitted_uri_chars'); + } + log_message('debug', 'URI Class Initialized'); } @@ -303,23 +318,19 @@ class CI_URI { * @param string $str * @return string */ - public function _filter_uri($str) + public function filter_uri($str) { - if ($str !== '' && $this->config->item('permitted_uri_chars') != '' && $this->config->item('enable_query_strings') === FALSE) + if ( ! empty($str) && ! empty($this->_permitted_uri_chars) && ! preg_match('/^['.$this->_permitted_uri_chars.']+$/i', $str)) { - // preg_quote() in PHP 5.3 escapes -, so the str_replace() and addition of - to preg_quote() is to maintain backwards - // compatibility as many are unaware of how characters in the permitted_uri_chars will be parsed as a regex pattern - if ( ! preg_match('|^['.str_replace(array('\\-', '\-'), '-', preg_quote($this->config->item('permitted_uri_chars'), '-')).']+$|i', $str)) - { - show_error('The URI you submitted has disallowed characters.', 400); - } + show_error('The URI you submitted has disallowed characters.', 400); } // Convert programatic characters to entities and return return str_replace( - array('$', '(', ')', '%28', '%29'), // Bad - array('$', '(', ')', '(', ')'), // Good - $str); + array('$', '(', ')', '%28', '%29'), // Bad + array('$', '(', ')', '(', ')'), // Good + $str + ); } // -------------------------------------------------------------------- @@ -365,7 +376,7 @@ class CI_URI { foreach (explode('/', preg_replace('|/*(.+?)/*$|', '\\1', $this->uri_string)) as $val) { // Filter segments for security - $val = trim($this->_filter_uri($val)); + $val = trim($this->filter_uri($val)); if ($val !== '') { -- cgit v1.2.3-24-g4f1b From d8b1ad31cf7ee205ddf3cf396b1d1bfa45af49fa Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Wed, 15 Jan 2014 17:42:52 +0200 Subject: Fix #2822: Incorrect usage of fwrite() We only used to check (and not always) if the return value of fwrite() is boolean FALSE, while it is possible that the otherwise returned bytecount is less than the length of data that we're trying to write. This allowed incomplete writes over network streams and possibly a few other edge cases. --- system/core/Log.php | 12 ++++++++++-- system/core/Output.php | 44 +++++++++++++++++++++++++++++++------------- 2 files changed, 41 insertions(+), 15 deletions(-) (limited to 'system/core') diff --git a/system/core/Log.php b/system/core/Log.php index ff3c63568..63fef2088 100644 --- a/system/core/Log.php +++ b/system/core/Log.php @@ -178,7 +178,15 @@ class CI_Log { $message .= $level.' - '.date($this->_date_fmt).' --> '.$msg."\n"; flock($fp, LOCK_EX); - fwrite($fp, $message); + + for ($written = 0, $length = strlen($message); $written < $length; $written += $result) + { + if (($result = fwrite($fp, substr($message, $written))) === FALSE) + { + break; + } + } + flock($fp, LOCK_UN); fclose($fp); @@ -187,7 +195,7 @@ class CI_Log { @chmod($filepath, FILE_WRITE_MODE); } - return TRUE; + return is_int($result); } } diff --git a/system/core/Output.php b/system/core/Output.php index 10332f0d8..a7680b3d0 100644 --- a/system/core/Output.php +++ b/system/core/Output.php @@ -542,17 +542,26 @@ class CI_Output { return; } - $expire = time() + ($this->cache_expiration * 60); - - // Put together our serialized info. - $cache_info = serialize(array( - 'expire' => $expire, - 'headers' => $this->headers - )); - if (flock($fp, LOCK_EX)) { - fwrite($fp, $cache_info.'ENDCI--->'.$output); + $expire = time() + ($this->cache_expiration * 60); + + // Put together our serialized info. + $cache_info = serialize(array( + 'expire' => $expire, + 'headers' => $this->headers + )); + + $output = $cache_info.'ENDCI--->'.$output; + + for ($written = 0, $length = strlen($output); $written < $length; $written += $result) + { + if (($result = fwrite($fp, substr($output, $written))) === FALSE) + { + break; + } + } + flock($fp, LOCK_UN); } else @@ -560,13 +569,22 @@ class CI_Output { log_message('error', 'Unable to secure a file lock for file at: '.$cache_path); return; } + fclose($fp); - @chmod($cache_path, FILE_WRITE_MODE); - log_message('debug', 'Cache file written: '.$cache_path); + if (is_int($result)) + { + @chmod($cache_path, FILE_WRITE_MODE); + log_message('debug', 'Cache file written: '.$cache_path); - // Send HTTP cache-control headers to browser to match file cache settings. - $this->set_cache_header($_SERVER['REQUEST_TIME'], $expire); + // Send HTTP cache-control headers to browser to match file cache settings. + $this->set_cache_header($_SERVER['REQUEST_TIME'], $expire); + } + else + { + @unlink($cache_path); + log_message('error', 'Unable to write the complete cache content at: '.$cache_path); + } } // -------------------------------------------------------------------- -- cgit v1.2.3-24-g4f1b