Tickets :: Comment on [#7931] Left Logout button throws "malicious request"

* Your Email Address
* Spam protection	Enter the letters below: \ /. ,. ..__ .__. >< \./ \|\/\|[__)[__] / \ \| \| \|[__)\| \|
Comment	> Hello. > > > > I have been struggling with a problem similar this for a few days: > basically a non-very-deterministic behavior of the two logout links > with the horde_logout_token in the main page. My scenario is Horde > 3.3.5 using MS SQL Server as session data storage through PEAR's DB > abstraction layer. > > > > Maybe this could give some ideas to others with the same problem, and > I think it could be at least a bug report and fix for my specific > situation. > > > > Finally it seems to turn out that it is a race condition problem > during the situations of simultaneous update of the session_data, > because of DB transaction mechanisms that are not so obvious in their > inner working in PEAR DB. The first thing that I noticed is that the > problem arose in reloads of the main page, which has two frames, each > one with a logout button with a horde_logout_token (those values are > stored in the session_data field as "horde_form_secrets"). If you > reload first one frame, then the other, everything works as expected > (slow down one of the participants in the race so no problems appear > :-) ). > > > > PEAR DB uses the concept of autocommit as a way of start > automatically a transaction, but the real transaction (issuing the > "BEGIN TRANSACTION" to the DB) starts just with a so called > "manipulation query" (INSERT, UPDATE, DELETE). That means that the > intended atomic process: > > > > 1. Read session data from DB > > 2. Update session data in DB > > > > is not actually a transaction, because the BEGIN TRANSACTION starts > in the 2nd step. > > That leads to a situation with no atomicity and hence a really nice > "race condition" with not so really nice results. > > > > As I didn't plan to fix anything in PEAR DB, I found a workaround > that could be acceptable, even if it's not the most elegant way. > > > > I forced a "manipulation query" in the _read() function at > lib/Horde/SessionHandler/sql.php just before the 1st step of > obtaining the session data, in order to force the begin of the > transaction: > > > > /* Force manipulation query to effectively begin the transaction */ > > $table = $this->_params['table']; > > $updatestring = 'session_id=session_id'; > > $wherestring = 'session_id = ?'; > > $query = sprintf('UPDATE %s SET %s WHERE %s', > > $table, > > $updatestring, > > $wherestring); > > $values[] = $id; > > $result = $this->_write_db->query($query, $values); > > if (is_a($result, 'PEAR_Error')) { > > Horde::logMessage($result, __FILE__, __LINE__, PEAR_LOG_ERR); > > return false; > > } > > > > This makes transaction work and no more race condition appeared in my > scenario so far. > > See patch attached. > > > > > > > >
Attachment
Watch this ticket

Saved Queries