Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Auto create session table if it doesn't exist. #420

Closed
wants to merge 2 commits into from

11 participants

darkhouse David Dotson Jasmo fewds Tarique Tuku ThallisPHP Andrey Andreev Marco Monteiro Joseph Wynn Thor Tony Dew
darkhouse

Added a config option to auto create the session table if it doesn't exist and you've set sess_use_database to TRUE. This way, you don't need to create the table yourself or add it to migrations or anything. It's set to FALSE by default so that it's a conscious decision by the develop to auto create it. It uses dbforge so that it's database agnostic and should work with whatever system the develop is using.

This solves the same issue as my previous pull request, where when running migrations, if the session library was autoloaded, and it was set to use the database, and the creation script was in the first migration, you'd end up with a database error that the session table didn't exist. I think this solution is better, as it doesn't mess around with reverting back to cookies like the previous one did.

David Dotson

This is a good idea, but what if the user doesn't have CREATE TABLE privileges? Maybe just give then the same error they get today? Of course, normally this probably wouldn't be an issue, but it could come up.

darkhouse

Yeah, so in that case they should keep the sess_table_auto_create option set to FALSE. Again, this is to help with migrations, so, if they don't have CREATE TABLE privileges, then they won't be able to use migrations either, as it runs the same way to create the schema_version table if it doesn't exist.

Jasmo

I think this is a good feature, which helps developers a little bit when deploying or get used to this system.

fewds

This is one of the things that I have been waiting for! If the user doesn't have create table privileges then just output an error message to the browser. I think this will speed up the time it takes to get started on a new CI app by at least 1-2 mins.

darkhouse
Tarique Tuku

Good Idea. I am sure, it will be helpful for most of the people.

ThallisPHP

Good idea

Andrey Andreev
Owner

At first, it looks like a good idea, but ... there are too many "if"s (I don't mean if() as the language structure).

It's good that it's optional, to solve the privileges problem, but still - if enabled, it would require another database query per each page execution. Considering that this is done for sessions (which should be as fast as possible) and that you'd benefit of it like 1 in a million times - I don't think it's a good trade-off.

Plus, some databases might not support it - e.g. I don't think that Oracle has a field of type 'text'.

Marco Monteiro

I have to agree with @narfbg on this one.

darkhouse

@narfbg and @mpmont Thanks for your input. My thinking is if you don't want to use it, just don't use it. It's not causing any harm, and if you do use it, you can turn it off at any point. If your database platform doesn't support it, you'll be doing something different anyways so that's a non-issue. Some would argue that using the database for sessions is a bad idea in general, so those people choose not to use the database, and likely have a custom session library running, maybe using native sessions, or some sort of cache system. The point is, it's completely optional and nobody is forcing them to use databases, just like I'm not forcing anybody to make CI generate the sessions table.

The goal here was to solve the issue with migrations, so when you deploy your app in a new environment, you don't have to turn sessions off just to run the migrations. Or maybe we should just be smarter about running migrations, such as from a hook before anything is autoloaded.

The other issue it solves is keeping the session table consistent with the code in the session library. There was an update in either v2.0.2 or 2.0.3 that broke sessions, where it would generate a new session every page load, simply because they changed the length of the user_agent field. If the session table was automatically generated by the session library, then it wouldn't matter (when deploying in a new environment).

The thing is, the migrations system (assuming Phil's code is what is being included in v2.1) generates the schema_version table if it doesn't exist, so I figured anything that requires a specific table with specific fields, should attempt to be generated automatically by CodeIgniter. Sessions are the only other thing that require a table (if you're using the database), so it makes sense to me that it should generate the table for me. I don't think that the session table should be generated in a migration, they should just be for your app tables.

However, if the general consensus is that the feature is not necessary and may cause more problems than it solves, then it won't be included (which is fine if that's the case), and we'll all just need to be more careful when we're deploying our apps in new environments. Obviously we should be as careful as possible anyways, I was just trying to solve some common issues that a lot of us run into regularly, and keep it optional so that if people don't want to use it, they don't have to.

Joseph Wynn

While I personally don't think that this is a good idea (I'll echo what narfbg said about unnecessary queries every page load), you should pull in the latest upstream and make sure your code adheres to the style guide. There's also a whole bunch of unrelated stuff in the changelog.

Thor

I'd be nice if the default welcome controller also had some additional methods built in so that on first load you could have "one-click" install/set-up of features such as this one. But I agree, an extra database call on each page load, just to check if something that only needs to run once has been run, that's not the best way to go about it.

Andrey Andreev narfbg closed this
Tony Dew

I wish that I'd have seen this pull request when it was open. For what it is worth, I think this is a great idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 9, 2011
  1. Auto create session table if it doesn't exist

    Adam Jackett authored
  2. Forgot to add the default config option to the foreach. Why do I alwa…

    Adam Jackett authored
    …ys do that?
This page is out of date. Refresh to see the latest.
19 application/config/config.php
View
@@ -250,15 +250,16 @@
| 'sess_time_to_update' = how many seconds between CI refreshing Session Information
|
*/
-$config['sess_cookie_name'] = 'ci_session';
-$config['sess_expiration'] = 7200;
-$config['sess_expire_on_close'] = FALSE;
-$config['sess_encrypt_cookie'] = FALSE;
-$config['sess_use_database'] = FALSE;
-$config['sess_table_name'] = 'ci_sessions';
-$config['sess_match_ip'] = FALSE;
-$config['sess_match_useragent'] = TRUE;
-$config['sess_time_to_update'] = 300;
+$config['sess_cookie_name'] = 'ci_session';
+$config['sess_expiration'] = 7200;
+$config['sess_expire_on_close'] = FALSE;
+$config['sess_encrypt_cookie'] = FALSE;
+$config['sess_use_database'] = FALSE;
+$config['sess_table_name'] = 'ci_sessions';
+$config['sess_table_auto_create'] = FALSE;
+$config['sess_match_ip'] = FALSE;
+$config['sess_match_useragent'] = TRUE;
+$config['sess_time_to_update'] = 300;
/*
|--------------------------------------------------------------------------
25 system/libraries/Session.php
View
@@ -29,6 +29,7 @@ class CI_Session {
var $sess_encrypt_cookie = FALSE;
var $sess_use_database = FALSE;
var $sess_table_name = '';
+ var $sess_table_auto_create = FALSE;
var $sess_expiration = 7200;
var $sess_expire_on_close = FALSE;
var $sess_match_ip = FALSE;
@@ -62,7 +63,7 @@ public function __construct($params = array())
// Set all the session preferences, which can either be set
// manually via the $params array above or via the config file
- foreach (array('sess_encrypt_cookie', 'sess_use_database', 'sess_table_name', 'sess_expiration', 'sess_expire_on_close', 'sess_match_ip', 'sess_match_useragent', 'sess_cookie_name', 'cookie_path', 'cookie_domain', 'cookie_secure', 'sess_time_to_update', 'time_reference', 'cookie_prefix', 'encryption_key') as $key)
+ foreach (array('sess_encrypt_cookie', 'sess_use_database', 'sess_table_name', 'sess_table_auto_create', 'sess_expiration', 'sess_expire_on_close', 'sess_match_ip', 'sess_match_useragent', 'sess_cookie_name', 'cookie_path', 'cookie_domain', 'cookie_secure', 'sess_time_to_update', 'time_reference', 'cookie_prefix', 'encryption_key') as $key)
{
$this->$key = (isset($params[$key])) ? $params[$key] : $this->CI->config->item($key);
}
@@ -85,6 +86,28 @@ public function __construct($params = array())
if ($this->sess_use_database === TRUE AND $this->sess_table_name != '')
{
$this->CI->load->database();
+
+ // Does the table exist? If not, should we create it automatically?
+ if ($this->sess_table_auto_create === TRUE && ! $this->CI->db->table_exists($this->sess_table_name))
+ {
+ $this->CI->load->dbforge();
+
+ // Create the fields, keys and table
+ $this->CI->dbforge->add_field(
+ array(
+ 'session_id' => array('type' => 'varchar', 'constraint' => 40, 'null' => FALSE),
+ 'ip_address' => array('type' => 'varchar', 'constraint' => 16, 'null' => FALSE),
+ 'user_agent' => array('type' => 'varchar', 'constraint' => 120, 'null' => FALSE),
+ 'last_activity' => array('type' => 'int', 'constraint' => 10, 'null' => FALSE, 'unsigned' => TRUE),
+ 'user_data' => array('type' => 'text', 'null' => FALSE)
+ )
+ );
+
+ $this->CI->dbforge->add_key('session_id', TRUE);
+ $this->CI->dbforge->add_key('last_activity');
+
+ $this->CI->dbforge->create_table($this->sess_table_name, TRUE);
+ }
}
// Set the "now" time. Can either be GMT or server time, based on the
48 user_guide/changelog.html
View
@@ -65,44 +65,31 @@
<ul>
<li>General Changes
<ul>
- <li>Added Android to the list of user agents.</li>
- <li>Added Windows 7 to the list of user platforms.</li>
- <li>Callback validation rules can now accept parameters like any other validation rule.</li>
- <li>Ability to log certain error types, not all under a threshold.</li>
- <li>Added html_escape() to <a href="general/common_functions.html">Common functions</a> to escape HTML output for preventing XSS.</li>
+ <li class="reactor">Callback validation rules can now accept parameters like any other validation rule.</li>
</ul>
</li>
<li>Helpers
<ul>
- <li>Added <samp>increment_string()</samp> to <a href="helpers/string_helper.html">String Helper</a> to turn "foo" into "foo-1" or "foo-1" into "foo-2".</li>
- <li>Altered form helper - made action on form_open_multipart helper function call optional. Fixes (#65)</li>
- <li><samp>url_title()</samp> will now trim extra dashes from beginning and end.</li>
- <li>Improved speed of <a href="helpers/string_helper.html">String Helper</a>'s <b>random_string()</b> method</li>
+ <li class="reactor">Added <samp>increment_string()</samp> to <a href="helpers/string_helper.html">String Helper</a> to turn "foo" into "foo-1" or "foo-1" into "foo-2".</li>
+ <li>Altered form helper - made action on form_open_multipart helper function call optional. Fixes (#65)</li>
</ul>
</li>
<li>Database
<ul>
- <li>Added a <a href="http://www.cubrid.org/" target="_blank">CUBRID</a> driver to the <a href="database/index.html">Database Driver</a>. Thanks to the CUBRID team for supplying this patch.</li>
- <li>Typecast limit and offset in the <a href="database/queries.html">Database Driver</a> to integers to avoid possible injection.</li>
- <li>
+ <li class="reactor">Added a <a href="http://www.cubrid.org/" target="_blank">CUBRID</a> driver to the <a href="libraries/database.html">Database Driver</a>. Thanks to the CUBRID team for supplying this patch.</li>
+ <li class="reactor">Typecast limit and offset in the <a href="database/queries.html">Database Driver</a> to integers to avoid possible injection.</li>
+ <li class="reactor">
Added additional option 'none' for the optional third argument for <kbd>$this->db->like()</kbd> in the <a href="database/active_record.html">Database Driver</a>.
</li>
</ul>
</li>
<li>Libraries
<ul>
- <li>Changed <kbd>$this->cart->insert()</kbd> in the <a href="libraries/cart.html">Cart Library</a> to return the Row ID if a single item was inserted successfully.</li>
- <li>Added support to set an optional parameter in your callback rules of validation using the <a href="libraries/form_validation.html">Form Validation Library</a>.</li>
- <li>Added a <a href="libraries/migration.html">Migration Library</a> to assist with applying incremental updates to your database schema.</li>
- <li>Driver children can be located in any package path.</li>
- <li>Added max_filename_increment config setting for Upload library.</li>
- <li><samp>CI_Loader::_ci_autoloader()</samp> is now a protected method.</li>
- <li>Added <kbd>is_unique</kbd> to the <a href="libraries/form_validation.html">Form Validation library</a>.</li>
- </ul>
- </li>
- <li>Core
- <ul>
- <li>Changed private functions in CI_URI to protected so MY_URI can override them.</li>
+ <li class="reactor">Changed <kbd>$this->cart->insert()</kbd> in the <a href="libraries/cart.html">Cart Library</a> to return the Row ID if a single item was inserted successfully.</li>
+ <li class="reactor">Added support to set an optional parameter in your callback rules of validation using the <a href="libraries/form_validation.html">Form Validation Library</a>.</li>
+ <li class="reactor">Added a <a href="libraries/migration.html">Migration Library</a> to assist with applying incremental updates to your database schema.</li>
+ <li class="reactor">Driver children can be located in any package path.</li>
+ <li class="reactor"><a href="libraries/session.html">Session Library</a> now has a config option to auto create the database table if it doesn't exist.</li>
</ul>
</li>
</ul>
@@ -114,14 +101,8 @@
<li class="reactor">Fixed a bug (Reactor #19) where 1) the 404_override route was being ignored in some cases, and 2) auto-loaded libraries were not available to the 404_override controller when a controller existed but the requested method did not.</li>
<li class="rector">Fixed a bug (Reactor #89) where MySQL export would fail if the table had hyphens or other non alphanumeric/underscore characters.</li>
<li class="reactor">Fixed a bug (#200) where MySQL queries would be malformed after calling <samp>count_all()</samp> then <samp>db->get()</samp></li>
- <li class="reactor">Fixed bug #105 that stopped query errors from being logged unless database debugging was enabled</li>
<li>Fixed a bug (#181) where a mis-spelling was in the form validation language file.</li>
<li>Fixed a bug (#160) - Removed unneeded array copy in the file cache driver.</li>
- <li>Fixed a bug (#150) - <samp>field_data()</samp> now correctly returns column length.</li>
- <li>Fixed a bug (#8) - <samp>load_class()</samp> now looks for core classes in <samp>APPPATH</samp> first, allowing them to be replaced.</li>
- <li>Fixed a bug (#24) - ODBC database driver called incorrect parent in __construct().</li>
- <li>Fixed a bug (#85) - OCI8 (Oracle) database escape_str() function did not escape correct.</li>
- <li>Fixed a bug (#344) - Using schema found in <a href="libraries/sessions.html">Saving Session Data to a Database</a>, system would throw error "user_data does not have a default value" when deleting then creating a session.</li>
</ul>
<h2>Version 2.0.3</h2>
@@ -141,13 +122,7 @@
<li>Visual updates to the welcome_message view file and default error templates. Thanks to <a href="https://bitbucket.org/danijelb">danijelb</a> for the pull request.</li>
<li class="reactor">Added <samp>insert_batch()</samp> function to the PostgreSQL database driver. Thanks to epallerols for the patch.</li>
<li class="reactor">Added "application/x-csv" to mimes.php.</li>
- <li class="reactor">Added CSRF protection URI whitelisting.</li>
<li>Fixed a bug where <a href="libraries/email.html">Email library</a> attachments with a "." in the name would using invalid MIME-types.</li>
- <li>Added support for pem,p10,p12,p7a,p7c,p7m,p7r,p7s,crt,crl,der,kdb,rsa,cer,sst,csr Certs to mimes.php.</li>
- <li>Added support pgp,gpg to mimes.php.</li>
- <li>Added support 3gp, 3g2, mp4, wmv, f4v, vlc Video files to mimes.php.</li>
- <li>Added support m4a, aac, m4u, xspf, au, ac3, flac, ogg Audio files to mimes.php.</li>
-
</ul>
</li>
<li>Helpers
@@ -160,6 +135,7 @@
<li>Libraries
<ul>
<li>Altered Session to use a longer match against the user_agent string. See upgrade notes if using database sessions.</li>
+ <li class="reactor">Added <kbd>is_unique</kbd> to the <a href="libraries/form_validation.html">Form Validation library</a>.</li>
<li class="reactor">Added <kbd>$this->db->set_dbprefix()</kbd> to the <a href="database/queries.html">Database Driver</a>.</li>
<li class="reactor">Changed <kbd>$this->cart->insert()</kbd> in the <a href="libraries/cart.html">Cart Library</a> to return the Row ID if a single item was inserted successfully.</li>
<li class="reactor">Added <kbd>$this->load->get_var()</kbd> to the <a href="libraries/loader.html">Loader library</a> to retrieve global vars set with <kbd>$this->load->view()</kbd> and <kbd>$this->load->vars()</kbd>.</li>
6 user_guide/libraries/sessions.html
View
@@ -301,6 +301,12 @@
<td class="td">The name of the session database table.</td>
</tr>
<tr>
+ <td class="td"><strong>sess_table_auto_create</strong></td>
+ <td class="td">FALSE</td>
+ <td class="td">TRUE/FALSE (boolean)</td>
+ <td class="td">Whether to create the session table automatically if it doesn't exist and sess_use_database is TRUE.</td>
+</tr>
+<tr>
<td class="td"><strong>sess_time_to_update</strong></td>
<td class="td">300</td>
<td class="td">Time in seconds</td>
Something went wrong with that request. Please try again.