Bug in Session.php CI 1.7.3

[eluser]Daniel_C[/eluser]
In the session user guide we have that to store a session in the database we have to run these sql statements:

Code:

CREATE TABLE IF NOT EXISTS  `ci_sessions` (
session_id varchar(40) DEFAULT '0' NOT NULL,
ip_address varchar(16) DEFAULT '0' NOT NULL,
user_agent varchar(50) NOT NULL,
last_activity int(10) unsigned DEFAULT 0 NOT NULL,
user_data text NOT NULL,
PRIMARY KEY (session_id)
);

In Session.php

Code:

(line 309) $this->userdata = array(
'session_id'     => md5(uniqid($sessid, TRUE)),
'ip_address'     => $this->CI->input->ip_address(),
'user_agent'     => substr($this->CI->input->user_agent(), 0, 50),
'last_activity'    => $this->now
);

The problem is that user_data is NOT NULL and in the last code there is not a user_data value, so the row is never inserted in the database because it shows an error:
ERROR 1364 (HY000): Field 'user_data' doesn't have a default value

[eluser]Daniel_C[/eluser]
Another bug in the same file is in the line 209:

Code:

$query = $this->CI->db->get($this->sess_table_name);

// No result?  Kill it!
(line 209) if ($query->num_rows() == 0)

When the ci_sessions table does not exist in the database, the return of the get method is false, access false->num_rows() throws a fatal error.
The solution is add a comparison like this

Code:

if ($query === FALSE || $query->num_rows() == 0)

Obviously, it has no sense to use database feature when the ci_sessions table is not created.

[eluser]WanWizard[/eluser]
The first one is indeed a bug, because (at least in MySQL) a text/blob field can't be defined with a default value, so either the "NOT NULL" must be removed, or the code should provide a default value.

The second one is imho not a bug. Why use the session library, and define the user of the database yourself (in the config file, this is not the default), and then NOT create the table that is required if you do that?

[eluser]Daniel_C[/eluser]
I am not so involved in the framework develop, but in my honest opinion too, it makes no sense that a framework stops running with a PHP fatal error for a misconfiguration, the framework should use at least the show_error method to show the corresponding error.

[eluser]WanWizard[/eluser]
I beg to differ. If the framework should contain checks for every mistake a developer can make, it would be slow as hell.

In this case, if you have configured database sessions, you have done that because you intent to use it. So make sure the database is present.

[eluser]InsiteFX[/eluser]
This is what the Session Table should be to work!

Code:

--
-- Table structure for CodeIgniter cisessions.
--
DROP TABLE IF EXISTS `ci_sessions`;

CREATE TABLE IF NOT EXISTS `ci_sessions` (
  `session_id`        varchar(40)            DEFAULT '0'    NOT NULL,
  `ip_address`        varchar(16)            DEFAULT '0'    NOT NULL,
  `user_agent`        varchar(50)                    NOT NULL,
  `last_activity`    int(10)        unsigned    DEFAULT 0    NOT NULL,
  `user_data`        text                DEFAULT ''    NOT NULL,
  PRIMARY KEY (`session_id`)
) ENGINE=MyISAM DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci ;

-- For more user_data storage use one of the below values!
---------------------------------------------------------------------------------
--  `user_data`        text                DEFAULT ''    NOT NULL,
--  `user_data`        mediumtext            DEFAULT ''    NOT NULL,
--  `user_data`        longtext            DEFAULT ''    NOT NULL,

InsiteFX

[eluser]WanWizard[/eluser]
One of the issues reported is that blob's or text's in MySQL can't have a DEFAULT value.
So you should not use NOT NULL, that will generate an error when a new session is created.

[eluser]Daniel_C[/eluser]
[quote author="WanWizard" date="1293171110"]I beg to differ. If the framework should contain checks for every mistake a developer can make, it would be slow as hell.

In this case, if you have configured database sessions, you have done that because you intent to use it. So make sure the database is present.[/quote]

At the moment I did some mistakes on purpose and they have not resulted in fatal errors.
For example I changed language config to one non-existent:

Code:

$config['language'] = "r";

And the frameworks shows a beauty message:
An Error Was Encountered
Unable to load the requested language file: language/r/error_lang.php

The frameworks should contain checks for security mistakes, if CI does not check this database error it is vulnerable to full path disclosure, bug that we can fix in index.php with error_reporting(0)