Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Removed bug producing insert from JSessionStorageDatabase::write() #454

Closed
wants to merge 1 commit into from

7 participants

@garyamort

If update results in an error[such as a non existant row] then write will already return false. If however it succeeds but is identical to the current data, affectedRows will be 0, which then invokes insert while will ALWAYS fail because it is a duplicate row.

An insert is already performed when the session is created in JSession, there is no need for this one and it can only result in an error.

@LouisLandry LouisLandry commented on the diff
libraries/joomla/session/storage/database.php
@@ -117,13 +117,16 @@ class JSessionStorageDatabase extends JSessionStorage
}
else
{
- // If the session does not exist, we need to insert the session.
- $db->setQuery(
- 'INSERT INTO ' . $db->quoteName('#__session') .
- ' (' . $db->quoteName('session_id') . ', ' . $db->quoteName('data') . ', ' . $db->quoteName('time') . ')' .
- ' VALUES (' . $db->quote($id) . ', ' . $db->quote($data) . ', ' . (int) time() . ')'
- );
- return (boolean) $db->query();
+ // update did not error, so it succeeded but was identical
+ if (JDEBUG) {
+ jimport('joomla.error.log');

This is deprecated usage of JLog. You'll want to have a look at JPATH_PLATFORM/joomla/log/log.php. The syntax is something like:

<?php
JLog::add(sprintf('Session ID %s update is identical to current record', $id));
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@elkuku

Checkstyle error details:
libraries/joomla/session/storage/database.php:120, 121
Tabs must be used to indent lines; spaces are not allowed

libraries/joomla/session/storage/database.php:121
Expected "if (...)\n"; found "if (...) "

libraries/joomla/session/storage/database.php:122 - 129
Tabs must be used to indent lines; spaces are not allowed

@joomla-jenkins
Collaborator

Build triggered by changes to the base.

Test log missing. Tests failed to execute.
Checkstyle analysis reported 233 warnings and 4165 errors.

@joomla-jenkins
Collaborator

Build triggered by changes to the base.

Test log missing. Tests failed to execute.
Checkstyle analysis reported 235 warnings and 11 errors.

@eddieajau

Any followup on this issue?

@chdemko

@garyamort The pull request cannot be merged

@garyamort

hmm, thought this was fixed in another pull request... Elin reminded me it's been sitting here. Ok, I';ll have to pull the latest code and redo the update, this time without using depreciated JLog.

@garyamort

I re-arranged my repos into orgs for my own clarity and this branch was lost. I've recreated the fix and submitted it as pull request 1209.
#1209

@garyamort garyamort closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 10 additions and 7 deletions.
  1. +10 −7 libraries/joomla/session/storage/database.php
View
17 libraries/joomla/session/storage/database.php
@@ -117,13 +117,16 @@ public function write($id, $data)
}
else
{
- // If the session does not exist, we need to insert the session.
- $db->setQuery(
- 'INSERT INTO ' . $db->quoteName('#__session') .
- ' (' . $db->quoteName('session_id') . ', ' . $db->quoteName('data') . ', ' . $db->quoteName('time') . ')' .
- ' VALUES (' . $db->quote($id) . ', ' . $db->quote($data) . ', ' . (int) time() . ')'
- );
- return (boolean) $db->query();
+ // update did not error, so it succeeded but was identical
+ if (JDEBUG) {
+ jimport('joomla.error.log');

This is deprecated usage of JLog. You'll want to have a look at JPATH_PLATFORM/joomla/log/log.php. The syntax is something like:

<?php
JLog::add(sprintf('Session ID %s update is identical to current record', $id));
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ $log = JLog::getInstance('jcontroller.log.php')->addEntry(
+ array(
+ 'comment' => sprintf('Session ID %s update is identical to current record', $id)
+ )
+ );
+ }
+ return true;
}
}
Something went wrong with that request. Please try again.