fuel / core

Fuel PHP Framework - The core of the Fuel v1 framework
http://fuelphp.com
813 stars 345 forks source link

Sessions - Upgrade to 1.5 throws an exception unless I delete cookies #1293

Closed lukearmstrong closed 11 years ago

lukearmstrong commented 11 years ago

I've just updated a site to 1.5, and am greeted by this error.

Fuel\Core\Database_Exception [ 21000 ]: SQLSTATE[21000]: Cardinality violation: 1241 Operand should contain 1 column(s) with query: "SELECT * FROM sessions WHERE session_id = ('1aa7d3cdbc492f77f6858b152c778c51', '1aa7d3cdbc492f77f6858b152c778c51', '25a422f46af93e0ea70033028c4a260a', 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.17 (KHTML, like Gecko) Chrome/24.0.1312.52 Safari/537.17', 1358848073, 1358848109)"

I read that the sessions code had been changed so previous sessions have been invalidated, so I truncated the sessions table.

The error goes away when I clear my cookies. Something which I obviously can't do for visitors to the site.

Anyone else had this problem?

andrewwheal commented 11 years ago

Unfortunately sites will need to have all sessions deleted when they are updated. It might be worth testing sites thoroughly and then pushing it live in the night so that as few users are affected as possible.

lukearmstrong commented 11 years ago

Seems like a bug to me.

emlynwest commented 11 years ago

You could just remove existing sessions from the database.

phil-lavin commented 11 years ago

Migration, maybe?

emlynwest commented 11 years ago

After talking in IRC: The name of the cookie could be changed to work around this.

lukearmstrong commented 11 years ago

Removing existing sessions from the database doesn't clear the error.

The error is caused because the site is reading the cookie and trying to load the session, thus throwing a MySQL error. So it seems like a bug.

@stevewest suggested on IRC that I change the name of the fuel session cookie in the config to prevent the bug.

frankdejonge commented 11 years ago

This is mentioned in the changelog: https://github.com/fuel/core/wiki/Changelog-v1.5 [EDIT] You can solve this by changing the cookie name.

lukearmstrong commented 11 years ago

@FrenkyNet emptying the sessions table in the database doesn't resolve the problem

frankdejonge commented 11 years ago

@lukearmstrong, noted :)

WanWizard commented 11 years ago

This is odd, what exactly causes this exception? Nothing has been changed to either the structure of the session id, nor the way the session database is used.

When receiving the cookie it should detect it's not valid, scrap the session, and create a new one.

I've migrated all my applications some time ago, and have not observed this issue.

WanWizard commented 11 years ago

Released a 1.5.1. hotfix to deal with this.

lukearmstrong commented 11 years ago

@phil-lavin tested it without the fix and was able to reproduce it, and then updated with the fix and it worked.

mirahost commented 11 years ago

I downloaded the 1.5.1 files and the cookie issue is still present.

WanWizard commented 11 years ago

What did you exactly download from where?

Your fuel/core/classes/session/driver.php should look like this: https://github.com/fuel/core/blob/1.6/develop/classes/session/driver.php#L533

If your version doesn't have the additional check for 'driver' == 'cookie', you have an old version.

mirahost commented 11 years ago

My file looks exactly like that. I downloaded the 1.5.1 zip straight from the fuelphp.com home page.

The issue was resolved by changing the cookie name as others mentioned.

WanWizard commented 11 years ago

Which what version of Fuel were the session cookies created?

mirahost commented 11 years ago

The cookie was created by version 1.3

WanWizard commented 11 years ago

Can you test something for me? Can you replace that if structure (under // validate cookie format) by

if ( is_array($cookie) and $this->config['driver'] !== 'cookie')
{
    $cookie = false;
}
mirahost commented 11 years ago

I am not getting the error anymore since I changed the cookie name, as suggested by the others in this thread.

I changed the cookie name back to what it was previously and there are still no errors.

I updated the code with what you supplied (after the presence of no errors above) and there are still no errors.

WanWizard commented 11 years ago

ok. thanks for your confirmation. I'll make some changes.

mirahost commented 11 years ago

Great. Thank you.