Merge pull request #3393 from preems/develop
Added img_id to capcha helper
Close #51
diff --git a/contributing.md b/contributing.md
index 99ed859..126bdc7 100644
--- a/contributing.md
+++ b/contributing.md
@@ -79,7 +79,9 @@
The Reactor Engineers will now be alerted about the change and at least one of the team will respond. If your change fails to meet the guidelines it will be bounced, or feedback will be provided to help you improve it.
-Once the Reactor Engineer handling your pull request is happy with it they will merge it into develop and your patch will be part of the next release. Keeping your fork up-to-date
+Once the Reactor Engineer handling your pull request is happy with it they will merge it into develop and your patch will be part of the next release.
+
+### Keeping your fork up-to-date
Unlike systems like Subversion, Git can have multiple remotes. A remote is the name for a URL of a Git repository. By default your fork will have a remote named "origin" which points to your fork, but you can add another remote named "codeigniter" which points to `git://github.com/bcit-ci/CodeIgniter.git`. This is a read-only remote but you can pull from this develop branch to update your own.
@@ -89,4 +91,4 @@
2. `git pull codeigniter develop`
3. `git push origin develop`
-Now your fork is up to date. This should be done regularly, or before you send a pull request at least.
\ No newline at end of file
+Now your fork is up to date. This should be done regularly, or before you send a pull request at least.
diff --git a/system/core/Router.php b/system/core/Router.php
index 7f18adb..d86735f 100644
--- a/system/core/Router.php
+++ b/system/core/Router.php
@@ -171,18 +171,21 @@
$_d = isset($_GET[$_d]) ? trim($_GET[$_d], " \t\n\r\0\x0B/") : '';
if ($_d !== '')
{
- $this->set_directory($this->uri->filter_uri($_d));
+ $this->uri->filter_uri($_d);
+ $this->set_directory($_d);
}
- $_c = $this->config->item('controller_trigger');
+ $_c = trim($this->config->item('controller_trigger'));
if ( ! empty($_GET[$_c]))
{
- $this->set_class(trim($this->uri->filter_uri(trim($_GET[$_c]))));
+ $this->uri->filter_uri($_GET[$_c]);
+ $this->set_class($_GET[$_c]);
- $_f = $this->config->item('function_trigger');
+ $_f = trim($this->config->item('function_trigger'));
if ( ! empty($_GET[$_f]))
{
- $this->set_method(trim($this->uri->filter_uri($_GET[$_f])));
+ $this->uri->filter_uri($_GET[$_f]);
+ $this->set_method($_GET[$_f]);
}
$this->uri->rsegments = array(
diff --git a/system/core/URI.php b/system/core/URI.php
index 1817374..7909101 100644
--- a/system/core/URI.php
+++ b/system/core/URI.php
@@ -173,8 +173,9 @@
// Populate the segments array
foreach (explode('/', trim($this->uri_string, '/')) as $val)
{
+ $val = trim($val);
// Filter segments for security
- $val = trim($this->filter_uri($val));
+ $this->filter_uri($val);
if ($val !== '')
{
@@ -318,21 +319,14 @@
* Filters segments for malicious characters.
*
* @param string $str
- * @return string
+ * @return void
*/
- public function filter_uri($str)
+ public function filter_uri(&$str)
{
if ( ! empty($str) && ! empty($this->_permitted_uri_chars) && ! preg_match('/^['.$this->_permitted_uri_chars.']+$/i'.(UTF8_ENABLED ? 'u' : ''), $str))
{
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
- );
}
// --------------------------------------------------------------------
diff --git a/tests/codeigniter/core/URI_test.php b/tests/codeigniter/core/URI_test.php
index b05a385..50663d5 100644
--- a/tests/codeigniter/core/URI_test.php
+++ b/tests/codeigniter/core/URI_test.php
@@ -119,26 +119,12 @@
*/
// --------------------------------------------------------------------
- public function test_filter_uri()
+ public function test_filter_uri_passing()
{
$this->uri->_set_permitted_uri_chars('a-z 0-9~%.:_\-');
- $str_in = 'abc01239~%.:_-';
- $str = $this->uri->filter_uri($str_in);
-
- $this->assertEquals($str, $str_in);
- }
-
- // --------------------------------------------------------------------
-
- public function test_filter_uri_escaping()
- {
- // ensure escaping even if dodgey characters are permitted
- $this->uri->_set_permitted_uri_chars('a-z 0-9~%.:_\-()$');
-
- $str = $this->uri->filter_uri('$destroy_app(foo)');
-
- $this->assertEquals($str, '$destroy_app(foo)');
+ $str = 'abc01239~%.:_-';
+ $this->uri->filter_uri($str);
}
// --------------------------------------------------------------------
@@ -149,7 +135,8 @@
$this->uri->config->set_item('enable_query_strings', FALSE);
$this->uri->_set_permitted_uri_chars('a-z 0-9~%.:_\-');
- $this->uri->filter_uri('$this()');
+ $segment = '$this()'; // filter_uri() accepts by reference
+ $this->uri->filter_uri($segment);
}
// --------------------------------------------------------------------
diff --git a/user_guide_src/source/changelog.rst b/user_guide_src/source/changelog.rst
index 57a7dc7..431016d 100644
--- a/user_guide_src/source/changelog.rst
+++ b/user_guide_src/source/changelog.rst
@@ -57,6 +57,7 @@
- Added support for changing the file extension of log files using ``$config['log_file_extension']``.
- Added support for turning newline standardization on/off via ``$config['standardize_newlines']`` and set it to FALSE by default.
- Added configuration setting ``$config['composer_autoload']`` to enable loading of a `Composer <https://getcomposer.org/>`_ auto-loader.
+ - Removed the automatic conversion of 'programmatic characters' to HTML entities from the :doc:`URI Library <libraries/uri>`.
- Helpers
@@ -446,6 +447,7 @@
- Added conditional PCRE UTF-8 support to the "invalid URI characters" check and removed the ``preg_quote()`` call from it to allow more flexibility.
- Renamed method ``_filter_uri()`` to ``filter_uri()``.
+ - Changed method ``filter_uri()`` to accept by reference and removed its return value.
- Changed private methods to protected so that MY_URI can override them.
- Renamed internal method ``_parse_cli_args()`` to ``_parse_argv()``.
- Renamed internal method ``_detect_uri()`` to ``_parse_request_uri()``.
diff --git a/user_guide_src/source/installation/upgrade_300.rst b/user_guide_src/source/installation/upgrade_300.rst
index ef85106..2e9ee4e 100644
--- a/user_guide_src/source/installation/upgrade_300.rst
+++ b/user_guide_src/source/installation/upgrade_300.rst
@@ -223,8 +223,24 @@
``$_COOKIE`` and ``$_SERVER`` superglobals are no longer
automatically overwritten when global XSS filtering is turned on.
+*************************************************
+Step 12: Check for potential XSS issues with URIs
+*************************************************
+
+The :doc:`URI Library <../libraries/uri>` used to automatically convert
+a certain set of "programmatic characters" to HTML entities when they
+are encountered in a URI segment.
+
+This was aimed at providing some automatic XSS prodection, in addition
+to the ``$config['permitted_uri_chars']`` setting, but has proven to be
+problematic and is now removed in CodeIgniter 3.0.
+
+If your application has relied on this feature, you should update it to
+filter URI segments through ``$this->security->xss_clean()`` whenever you
+output them.
+
********************************************************
-Step 12: Update usage of Input Class's get_post() method
+Step 13: Update usage of Input Class's get_post() method
********************************************************
Previously, the :doc:`Input Class <../libraries/input>` method ``get_post()``
@@ -235,14 +251,14 @@
``get_post()`` was doing before.
***********************************************************************
-Step 13: Update usage of Directory Helper's directory_map() function
+Step 14: Update usage of Directory Helper's directory_map() function
***********************************************************************
In the resulting array, directories now end with a trailing directory
separator (i.e. a slash, usually).
*************************************************************
-Step 14: Update usage of Database Forge's drop_table() method
+Step 15: Update usage of Database Forge's drop_table() method
*************************************************************
Up until now, ``drop_table()`` added an IF EXISTS clause by default or it didn't work
@@ -264,7 +280,7 @@
all drivers with the exception of ODBC.
***********************************************************
-Step 15: Change usage of Email library with multiple emails
+Step 16: Change usage of Email library with multiple emails
***********************************************************
The :doc:`Email Library <../libraries/email>` will automatically clear the
@@ -279,7 +295,7 @@
}
***************************************************
-Step 16: Update your Form_validation language lines
+Step 17: Update your Form_validation language lines
***************************************************
Two improvements have been made to the :doc:`Form Validation Library
@@ -310,7 +326,7 @@
later.
****************************************************************
-Step 17: Remove usage of (previously) deprecated functionalities
+Step 18: Remove usage of (previously) deprecated functionalities
****************************************************************
In addition to the ``$autoload['core']`` configuration setting, there's a