Pending

User tests: Successful: Unsuccessful:

avatar JonahBraun
JonahBraun
14 Mar 2012

This is the preferred mysql specific fix, I am opening another pull request with the vendor neutral fix.

Abstract

A bug exists in JSessionStorageDatabase::write() (libraries/joomla/session/storage/database.php:94). The logic assumes that an update will have one affected row if a session exists. However, this is not true. The timestamp is only granular in seconds and may be the same with multiple ajax requests, refreshes on different tabs, etc. When the timestamp is the same the entry is exactly the same and the update will not affect any rows. With the current logic an insert will be attempted and this results in a 500.

The fix

This fix uses the mysql specific ON DUPLICATE KEY UPDATE. I consider this the preferred fix because it reduces the sql statements to one. I have however prepared a second vendor neutral fix that uses an additional select statement.

Example Error

500 - JDatabaseMySQLi::query: 1062 - Duplicate entry 'e33c3ab2b05a9dc97dc25187a823b8e6' for key 'PRIMARY' SQL=INSERT INTO `jos_session` (`session_id`, `data`, `time`) VALUES ('e33c3ab2b05a9dc97dc25187a823b8e6', '__default|a:8:{s:22:\"session.client.browser\";s:119:\"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_6_8) AppleWebKit/535.19 (KHTML, like Gecko) Chrome/18.0.1025.54 Safari/535.19\";s:15:\"session.counter\";i:43;s:8:\"registry\";O:9:\"JRegistry\":1:{s:7:\"\0*\0data\";O:8:\"stdClass\":0:{}}s:4:\"user\";O:5:\"JUser\":23:{s:9:\"\0*\0isRoot\";b:0;s:2:\"id\";i:0;s:4:\"name\";N;s:8:\"username\";N;s:5:\"email\";N;s:8:\"password\";N;s:14:\"password_clear\";s:0:\"\";s:8:\"usertype\";N;s:5:\"block\";N;s:9:\"sendEmail\";i:0;s:12:\"registerDate\";N;s:13:\"lastvisitDate\";N;s:10:\"activation\";N;s:6:\"params\";N;s:6:\"groups\";a:0:{}s:5:\"guest\";i:1;s:10:\"\0*\0_params\";O:9:\"JRegistry\":1:{s:7:\"\0*\0data\";O:8:\"stdClass\":0:{}}s:14:\"\0*\0_authGroups\";a:1:{i:0;i:1;}s:14:\"\0*\0_authLevels\";a:2:{i:0;i:1;i:1;i:1;}s:15:\"\0*\0_authActions\";N;s:12:\"\0*\0_errorMsg\";N;s:10:\"\0*\0_errors\";a:0:{}s:3:\"aid\";i:0;}s:13:\"session.token\";s:32:\"cc3f66462deb72a899115b2ea51f8608\";s:19:\"session.timer.start\";i:1331743210;s:18:\"session.timer.last\";i:1331744856;s:17:\"session.timer.now\";i:1331744857;}', '1331744857')
avatar JonahBraun JonahBraun - open - 14 Mar 2012
avatar infograf768
infograf768 - comment - 14 Mar 2012

Please create a tracker on joomlacode and post link here

avatar piotr-cz
piotr-cz - comment - 15 Mar 2012

Thanks for the fix.
Actually I think the commit should go to Joomla Platform github repository (https://github.com/joomla/joomla-platform).

There was already an effort here: joomla/joomla-platform#454, with tracker: http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=26956

I hope your commit will be accepted, this is a serious bug.

avatar azrul
azrul - comment - 16 May 2012

The problem is real and does need fixing, but the commit is actually bad, and I am pretty strongly against merging this into the core.

avatar piotr-cz
piotr-cz - comment - 17 May 2012

There is another commit for the issule: joomla/joomla-platform#1209

avatar elinw
elinw - comment - 17 May 2012

Jonah and Azrul could you comment on #1209 as an alternative solution?

avatar realityking
realityking - comment - 1 Jun 2012

Shouldn't we rather try to make sure that every key is unique? Or is there a reason why we can't guarantee that?

avatar realityking
realityking - comment - 1 Jun 2012

Shouldn't we rather try to make sure that every key is unique? Or is there a reason why we can't guarantee that?

avatar elinw
elinw - comment - 1 Jun 2012

That's the approach of #1209

avatar elinw
elinw - comment - 7 Jul 2013

It's over a year since the last comment, can we close this? It seems like no one is following up at all.

avatar nonumber nonumber - reference | 5b456ee - 31 Aug 13
avatar phproberto
phproberto - comment - 3 Oct 2013

I'm closing this as it's not mergeable and 2 years old. Feel free to open if you update it and are still interested in get it merged.

Thanks for contributing.

avatar phproberto phproberto - close - 3 Oct 2013

Add a Comment

Login with GitHub to post a comment