Roundcube Community Forum

Release Support => Pending Issues => Topic started by: valqk on March 14, 2007, 07:11:49 AM

Title: WTF!!! Bug in the rcube_db.inc -> insert_id for postgresql AND mssql!
Post by: valqk on March 14, 2007, 07:11:49 AM

Hello everyone,
after losing about 3 hours debugging this issue I've found a SERIOS bug in the
insert_id function of rcube_db.inc.

I don't know who wrote it but please whoever is responsible for this to fix it.

what is broken?

see this code:

Code: [Select]

function insert_id($sequence = '')
  {
  if (!$this->db_handle || $this->db_mode=='r')
   return FALSE;
  switch($this->db_provider)
   {
   case 'pgsql':
    $result = &$this->db_handle->getOne("SELECT CURRVAL('$sequence')");

   case 'mssql':
    $result = &$this->db_handle->getOne("SELECT @@IDENTITY");

    if (DB::isError($result))
     raise_error(array('code' => 500, 'type' => 'db', 'line' => __LINE__, 'file' => __FILE__,
              'message' => $result->getMessage()), TRUE, FALSE);

    return $result;

   case 'mysql': // This is unfortuneate
    return mysql_insert_id($this->db_handle->connection);

   case 'mysqli':
    return mysqli_insert_id($this->db_handle->connection);

   case 'sqlite':
    return sqlite_last_insert_rowid($this->db_handle->connection);

   default:
    die("portability issue with this database, please have the developer fix");
   }
  }

What the _fuck_ do you think will happen when the this->db_provider is pgsql and we don't have break clause or we don't return after fetching the id?!??
Well... the MSSQL fetch part will exec invalid for pg db server query SELECT @@.... and the function will fail.

sooo here is the fix >:( >:( :D :D :D :D ;) :-\:

Code: [Select]

function insert_id($sequence = '')
  {
  if (!$this->db_handle || $this->db_mode=='r')
   return FALSE;
  switch($this->db_provider)
   {
   case 'pgsql':
    $result = &$this->db_handle->getOne("SELECT CURRVAL('$sequence')");
     break;
   case 'mssql':
    $result = &$this->db_handle->getOne("SELECT @@IDENTITY");
     break;
   case 'mysql': // This is unfortunate
    $result = mysql_insert_id($this->db_handle->connection);
     break;

   case 'mysqli':
    $result = mysqli_insert_id($this->db_handle->connection);
     break;

   case 'sqlite':
    $result = sqlite_last_insert_rowid($this->db_handle->connection);
     break;

   default:
    die("portability issue with this database, please have the developer fix");
   }

    if (DB::isError($result))
     raise_error(array('code' => 500, 'type' => 'db', 'line' => __LINE__, 'file' => __FILE__,
              'message' => $result->getMessage()), TRUE, FALSE);

    return $result;

  }
Please apply this fixed code to the current tree and notify me here so I can test with latest and greatest version.

This bug affects the create user method and it's veryy bad issue.

This bug got me very nervous. I'm going to have a cigarette.
Laters,

waiting for code include confirmation.

Title: Re: WTF!!! Bug in the rcube_db.inc -> insert_id for postgresql AND mssql!
Post by: seansan on March 14, 2007, 02:12:55 PM

You can add the suggested change as a bug to:

http://trac.roundcube.net/trac.cgi/report

Maybe be handy in a diff file

Title: Re: WTF!!! Bug in the rcube_db.inc -> insert_id for postgresql AND mssql!
Post by: MichaelZicks on July 24, 2014, 01:18:15 PM

Quote from: valqk on March 14, 2007, 07:11:49 AM

Hello everyone,
after losing about 3 hours debugging this issue I've found a SERIOS bug in the
insert_id function of rcube_db.inc.

I don't know who wrote it but please whoever is responsible for this to fix it.

what is broken?

see this code:

Code: [Select]

function insert_id($sequence = '')
  {
  if (!$this->db_handle || $this->db_mode=='r')
   return FALSE;
  switch($this->db_provider)
   {
   case 'pgsql':
    $result = &$this->db_handle->getOne("SELECT CURRVAL('$sequence')");

   case 'mssql':
    $result = &$this->db_handle->getOne("SELECT @@IDENTITY");

    if (DB::isError($result))
     raise_error(array('code' => 500, 'type' => 'db', 'line' => __LINE__, 'file' => __FILE__,
              'message' => $result->getMessage()), TRUE, FALSE);

    return $result;

   case 'mysql': // This is unfortuneate
    return mysql_insert_id($this->db_handle->connection);

   case 'mysqli':
    return mysqli_insert_id($this->db_handle->connection);

   case 'sqlite':
    return sqlite_last_insert_rowid($this->db_handle->connection);

   default:
    die("portability issue with this database, please have the developer fix");
   }
  }

What the _fuck_ do you think will happen when the this->db_provider is pgsql and we don't have break clause or we don't return after fetching the id?!??
Well... the MSSQL fetch part will exec invalid for pg db server query SELECT @@.... and the function will fail.

sooo here is the fix >:( >:( :D :D :D :D ;) :-\:

Code: [Select]

function insert_id($sequence = '')
  {
  if (!$this->db_handle || $this->db_mode=='r')
   return FALSE;
  switch($this->db_provider)
   {
   case 'pgsql':
    $result = &$this->db_handle->getOne("SELECT CURRVAL('$sequence')");
     break;
   case 'mssql':
    $result = &$this->db_handle->getOne("SELECT @@IDENTITY");
     break;
   case 'mysql': // This is unfortunate
    $result = mysql_insert_id($this->db_handle->connection);
     break;

   case 'mysqli':
    $result = mysqli_insert_id($this->db_handle->connection);
     break;

   case 'sqlite':
    $result = sqlite_last_insert_rowid($this->db_handle->connection);
     break;

   default:
    die("portability issue with this database, please have the developer fix");
   }

    if (DB::isError($result))
     raise_error(array('code' => 500, 'type' => 'db', 'line' => __LINE__, 'file' => __FILE__,
              'message' => $result->getMessage()), TRUE, FALSE);

    return $result;

  }
Please apply this fixed code to the current tree and notify me here so I can test with latest and greatest version.

This bug affects the create user method and it's veryy bad issue.

This bug got me very nervous. I'm going to have a cigarette manufactured by e cigaretettes manufacturers (http://www.ecigfiend.com/category/ecig-starter-kits/).
Laters,

waiting for code include confirmation.

Thanks for sharing out the code.. I will apply it and will try to solve out the bug..

Title: Re: WTF!!! Bug in the rcube_db.inc -> insert_id for postgresql AND mssql!
Post by: alec on July 24, 2014, 03:56:09 PM

You should update immediately and don't use so old version of Roundcube.