Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Add checks that session management doesn't overlap

  • Loading branch information...
commit 26d229f1a107ce3f9bcf2c101e29a77feaf5530d 1 parent e9ec6e6
Brian Birtles authored March 13, 2013
64  wall/lib/walls.inc
@@ -296,35 +296,71 @@ function getWallIdFromPath($path) {
296 296
   return $res;
297 297
 }
298 298
 
299  
-function startNewSession($wallId, $datetime) {
  299
+function startNewSession($wallId, $sessionId, $datetime) {
  300
+  // Sanitize input
  301
+  $wallId = toId($wallId);
  302
+  $sessionId = toId($sessionId);
  303
+
  304
+  // Work out the latest session and if it needs closing
  305
+  $latestSession = getLatestSession($wallId);
  306
+
  307
+  // Is the passed-in session ID correct?
  308
+  if (@$latestSession['id'] !== $sessionId) {
  309
+    return false;
  310
+  }
  311
+
  312
+  // Close the latest session if it's open
  313
+  if (@$latestSession['endDate'] !== null) {
  314
+    closeLastSession($wallId, $sessionId, $datetime);
  315
+  }
  316
+
  317
+  // Start a new session
300 318
   $conn =& getDbConnection();
301 319
   $qwallid = $conn->quote($wallId, 'integer');
302 320
   $qdatetime = $conn->quote($datetime, 'timestamp');
303  
-  $query = 'INSERT INTO sessions(wallId,beginDate) VALUES('.$wallId.','.$qdatetime.')';
304  
-  $res =& $conn->query($query);
  321
+  $query = 'INSERT INTO sessions(wallId, beginDate)'
  322
+    . ' VALUES(' . $conn->quote($wallId, 'integer')
  323
+    . ',' . $conn->quote($datetime, 'timestamp')
  324
+    . ')';
  325
+  $res =& $conn->exec($query);
305 326
   if (PEAR::isError($res)) {
306 327
     error_log($res->getMessage() . ', ' . $res->getDebugInfo());
307 328
     throw new KeyedException('db-error');
308 329
   }
309 330
   $conn->disconnect();
  331
+
  332
+  return true;
310 333
 }
311 334
 
312  
-function closeLastSession($wallId, $datetime) {
  335
+function closeLastSession($wallId, $sessionId, $datetime) {
  336
+  // Sanitize input
  337
+  $wallId = toId($wallId);
  338
+  $sessionId = toId($sessionId);
  339
+
  340
+  // Check if the passed in sessionID actually matches the latest session (and 
  341
+  // that session is not already closed)
  342
+  if ($sessionId !== getActiveSessionId($wallId)) {
  343
+    return false;
  344
+  }
  345
+
  346
+  // Close session
313 347
   $conn =& getDbConnection();
314  
-  $qwallid = $conn->quote($wallId, 'integer');
315  
-  $qdatetime = $conn->quote($datetime, 'timestamp');
316  
-  //makes temp table for (You can't specify target table 'sessions' for update in FROM clause)
317  
-  $query = 'UPDATE sessions SET endDate='.$qdatetime.' WHERE endDate IS NULL AND wallId='.$qwallid.' AND sessionId=(SELECT temp.sessionId FROM (SELECT MAX(sessionId) AS sessionID FROM sessions WHERE wallId='.$qwallid.') AS temp)';
  348
+  $query = 'UPDATE sessions'
  349
+    . ' SET endDate=' . $conn->quote($datetime, 'timestamp')
  350
+    . ' WHERE sessionId=' . $conn->quote($sessionId, 'integer');
318 351
   $res =& $conn->exec($query);
319 352
   if (PEAR::isError($res)) {
320 353
     error_log($res->getMessage() . ', ' . $res->getDebugInfo());
321 354
     throw new KeyedException('db-error');
322 355
   }
323  
-
324 356
   $conn->disconnect();
325 357
 
326  
-  // Return true if we made some change, false if the session was already closed
327  
-  return $res > 0;
  358
+  // Return true since we made a change
  359
+  return true;
  360
+}
  361
+
  362
+function toId($idLike) {
  363
+  return is_null($idLike) ? null : intval($idLike);
328 364
 }
329 365
 
330 366
 // Returns NULL if there is no active session for the wall
@@ -336,7 +372,7 @@ function getActiveSessionId($wallId) {
336 372
   $res =& $conn->queryOne(
337 373
       'SELECT sessionId FROM sessions WHERE'
338 374
       . ' wallId = ' . $conn->quote($wallId, 'integer')
339  
-      . ' AND beginDate < ' . $qdate
  375
+      . ' AND beginDate <= ' . $qdate
340 376
       . ' AND (endDate IS NULL OR endDate > ' . $qdate . ')',
341 377
       'integer');
342 378
   if (PEAR::isError($res)) {
@@ -361,8 +397,10 @@ function getLatestSession($wallId) {
361 397
     error_log($row->getMessage() . ', ' . $row->getDebugInfo());
362 398
     throw new KeyedException('db-error');
363 399
   }
  400
+  if (!$row)
  401
+    return null;
364 402
   return array(
365  
-    'id' => $row['sessionid'],
  403
+    'id' => intval($row['sessionid']),
366 404
     'start' => $row['begindate'],
367 405
     'end' => $row['enddate'],
368 406
   );
24  wall/public/wall-maker/api/closeSession.php
@@ -23,15 +23,27 @@
23 23
 fclose($handle);
24 24
 
25 25
 // Prepare parameters
26  
-$wallId = @$json['wallId'];
27  
-if (!isset($wallId)) {
28  
-  bailWithError('logged-out');
  26
+$wallId    = @$json['wallId'];
  27
+$sessionId = @$json['sessionId'];
  28
+if (!isset($wallId) || !isset($sessionId)) {
  29
+  bailWithError('bad-request');
29 30
 }
30 31
 
  32
+// Close session
31 33
 $currentdatetime = gmdate("Y-m-d H:i:s");
32  
-if (!closeLastSession($wallId, $currentdatetime))
33  
-  bailWithError('already-closed');
  34
+$madeChange = closeLastSession($wallId, $sessionId, $currentdatetime);
34 35
 
35 36
 // Return the result
36  
-print json_encode(getLatestSession($wallId));
  37
+// - If we made a change then return the latest session.
  38
+// - Otherwise return a parallel-change notification with the latest session in 
  39
+//   the detail.
  40
+$latestSession = getLatestSession($wallId);
  41
+if ($madeChange) {
  42
+  print json_encode($latestSession);
  43
+} else {
  44
+  $result['error_key'] = 'parallel-change';
  45
+  $result['error_detail'] = $latestSession;
  46
+  print json_encode($result);
  47
+}
  48
+
37 49
 ?>
2  wall/public/wall-maker/api/createWall.php
@@ -34,7 +34,7 @@
34 34
 $result = array('wallId' => $wallId);
35 35
 // start session
36 36
 $currentdatetime = gmdate("Y-m-d H:i:s");
37  
-startNewSession($wallId, $currentdatetime);
  37
+startNewSession($wallId, null, $currentdatetime);
38 38
 
39 39
 // Return the result
40 40
 print json_encode($result);
23  wall/public/wall-maker/api/startSession.php
@@ -23,15 +23,26 @@
23 23
 fclose($handle);
24 24
 
25 25
 // Prepare parameters
26  
-$wallId = @$json['wallId'];
27  
-if (!isset($wallId)) {
28  
-  bailWithError('logged-out');
  26
+$wallId    = @$json['wallId'];
  27
+$sessionId = @$json['sessionId'];
  28
+if (!isset($wallId) || !isset($sessionId)) {
  29
+  bailWithError('bad-request');
29 30
 }
30 31
 
  32
+// Start new session
31 33
 $currentdatetime = gmdate("Y-m-d H:i:s");
32  
-closeLastSession($wallId, $currentdatetime);
33  
-startNewSession($wallId, $currentdatetime);
  34
+$madeChange = startNewSession($wallId, $sessionId, $currentdatetime);
34 35
 
35 36
 // Return the result
36  
-print json_encode(getLatestSession($wallId));
  37
+// - If we made a change then return the latest session.
  38
+// - Otherwise return a parallel-change notification with the latest session in 
  39
+//   the detail.
  40
+$latestSession = getLatestSession($wallId);
  41
+if ($madeChange) {
  42
+  print json_encode($latestSession);
  43
+} else {
  44
+  $result['error_key'] = 'parallel-change';
  45
+  $result['error_detail'] = $latestSession;
  46
+  print json_encode($result);
  47
+}
37 48
 ?>
12  wall/tests/api/ParaparaTestCase.php
@@ -21,6 +21,10 @@
21 21
   // Database connection singleton
22 22
   static private $conn = null;
23 23
 
  24
+  // An instance of SimpleTestCase so we can re-use its methods
  25
+  // (Why WebTestCase doesn't inherit from UnitTestCase is beyond me.)
  26
+  static protected $unitTestCase = null;
  27
+
24 28
   // Path to designs folder
25 29
   protected $designsPath;
26 30
 
@@ -29,6 +33,9 @@ function __construct($name = false) {
29 33
     if (self::$conn === null) {
30 34
       self::initDb();
31 35
     }
  36
+    if (self::$unitTestCase === null) {
  37
+      self::$unitTestCase = new UnitTestCase();
  38
+    }
32 39
     $this->designsPath = dirname(__FILE__) . '/../../public/designs/';
33 40
   }
34 41
 
@@ -99,6 +106,11 @@ public static function isNotEmpty($str) {
99 106
     return trim($str) != '';
100 107
   }
101 108
 
  109
+  public function assertEqual($first, $second, $message = '%s') {
  110
+    self::$unitTestCase->reporter = $this->reporter;
  111
+    return self::$unitTestCase->assertEqual($first, $second, $message);
  112
+  }
  113
+
102 114
   public function getConnection() {
103 115
     return self::$conn;
104 116
   }
89  wall/tests/api/TestSessions.php
@@ -45,15 +45,23 @@ function testCloseSession() {
45 45
     // Create wall
46 46
     $wallId = $this->createWall('Test wall', $this->testDesignId);
47 47
 
  48
+    // Get current session ID
  49
+    $wall = $this->getWall($wallId);
  50
+    $sessionId = $wall['session']['id'];
  51
+
48 52
     // Close session
49  
-    $response = $this->closeSession($wallId);
  53
+    $response = $this->closeSession($wallId, $sessionId);
50 54
 
51 55
     // Check we got the times and status
52 56
     $this->assertTrue(!array_key_exists('error_key', $response),
53  
-                      "Failed to close session:" . @$response['error_key']);
  57
+                      "Failed to close session: " . @$response['error_key']);
54 58
     $this->assertTrue($this->isClosedSession($response),
55 59
                       "Session does not appear to be closed.");
56 60
 
  61
+    // Check the ID is the same
  62
+    $this->assertEqual(@$response['id'], $sessionId,
  63
+                       "Got different session IDs: %s");
  64
+
57 65
     // Re-fetch wall
58 66
     $wall = $this->getWall($wallId);
59 67
     $this->assertTrue($wall['status'] == 'finished');
@@ -62,12 +70,14 @@ function testCloseSession() {
62 70
 
63 71
     // Logout and check it fails
64 72
     $this->logout();
65  
-    $response = $this->closeSession($wallId);
  73
+    $response = $this->closeSession($wallId, $sessionId);
66 74
     $this->assertTrue(array_key_exists('error_key', $response) &&
67 75
                       $response['error_key'] == 'logged-out',
68 76
                       "Closed session whilst logged out.");
69 77
 
70 78
     // XXX Check we can't close the session of someone else's wall
  79
+    // XXX Test we get a bad-request error if either wallId or sessionId is 
  80
+    // missing or bad
71 81
 
72 82
     // Tidy up by removing the wall
73 83
     $this->removeWall($wallId);
@@ -79,16 +89,26 @@ function testCloseClosedSession() {
79 89
 
80 90
     // Create wall
81 91
     $wallId = $this->createWall('Test wall', $this->testDesignId);
  92
+    $wall = $this->getWall($wallId); // In PHP < 5.4 we have to do this in two
  93
+                                     // steps
  94
+    $sessionId = $wall['session']['id'];
82 95
 
83 96
     // Close session
84  
-    $response = $this->closeSession($wallId);
  97
+    $response = $this->closeSession($wallId, $sessionId);
85 98
     $this->assertTrue($this->isClosedSession($response),
86  
-                      "Session does not appear to be closed.");
  99
+                      "Session does not appear to be closed");
87 100
 
88 101
     // Close again
89  
-    $response = $this->closeSession($wallId);
  102
+    $response = $this->closeSession($wallId, $sessionId);
90 103
     $this->assertTrue(array_key_exists('error_key', $response) &&
91  
-                      $response['error_key'] == 'already-closed');
  104
+                      $response['error_key'] == 'parallel-change',
  105
+                      "No error about parallel change when closing twice");
  106
+    $this->assertTrue($this->isClosedSession(@$response['error_detail']),
  107
+                      "Doubly-closed session does not appear to be closed");
  108
+
  109
+    // Check ID hasn't changed
  110
+    $this->assertEqual(@$response['error_detail']['id'], $sessionId,
  111
+                       "Got different session IDs: %s");
92 112
 
93 113
     // Re-fetch wall (to check we're in a consistent state)
94 114
     $wall = $this->getWall($wallId);
@@ -106,16 +126,23 @@ function testStartNew() {
106 126
 
107 127
     // Create wall
108 128
     $wallId = $this->createWall('Test wall', $this->testDesignId);
  129
+    $wall = $this->getWall($wallId);
  130
+    $sessionId = $wall['session']['id'];
109 131
 
110  
-    // Close session
111  
-    $response = $this->startNewSession($wallId);
  132
+    // Start new session
  133
+    $response = $this->startNewSession($wallId, $sessionId);
112 134
 
113 135
     // Check we got the times and status
114 136
     $this->assertTrue(!array_key_exists('error_key', $response),
115  
-                      "Failed to start new session:" . @$response['error_key']);
  137
+                      "Failed to start new session: " .
  138
+                      @$response['error_key']);
116 139
     $this->assertTrue($this->isOpenSession($response),
117 140
                       "Session does not appear to be open");
118 141
 
  142
+    // Check the IDs differ
  143
+    $this->assertEqual(@$response['id'], $sessionId+1,
  144
+                       "Got unexpected session ID: %s");
  145
+
119 146
     // Re-fetch wall
120 147
     $wall = $this->getWall($wallId);
121 148
     $this->assertTrue($wall['status'] == 'running');
@@ -124,7 +151,7 @@ function testStartNew() {
124 151
 
125 152
     // Logout and check it fails
126 153
     $this->logout();
127  
-    $response = $this->startNewSession($wallId);
  154
+    $response = $this->startNewSession($wallId, $sessionId);
128 155
     $this->assertTrue(array_key_exists('error_key', $response) &&
129 156
                       $response['error_key'] == 'logged-out',
130 157
                       "Started new session whilst logged out.");
@@ -135,6 +162,37 @@ function testStartNew() {
135 162
     $this->removeWall($wallId);
136 163
   }
137 164
 
  165
+  function testParallelStartNew() {
  166
+    // Login
  167
+    $this->login();
  168
+
  169
+    // Create wall
  170
+    $wallId = $this->createWall('Test wall', $this->testDesignId);
  171
+    $wall = $this->getWall($wallId);
  172
+    $sessionId = $wall['session']['id'];
  173
+
  174
+    // Start new session
  175
+    $responseA = $this->startNewSession($wallId, $sessionId);
  176
+
  177
+    // And do it again but with the OLD sessionId
  178
+    $responseB = $this->startNewSession($wallId, $sessionId);
  179
+    $this->assertTrue(array_key_exists('error_key', $responseB) &&
  180
+                      $responseB['error_key'] == 'parallel-change',
  181
+                      "No error about parallel change when starting with old "
  182
+                      . "session ID");
  183
+    $this->assertTrue($this->isOpenSession(@$responseB['error_detail']),
  184
+                      "Session returned after starting twice does not appear "
  185
+                      . " to be open");
  186
+
  187
+    // Check the session returned by the parallel change matches that returned 
  188
+    // when we first started a new session
  189
+    $this->assertEqual(@$responseA['id'], @$responseB['error_detail']['id'],
  190
+                       "Got unexpected session ID: %s");
  191
+
  192
+    // Tidy up by removing the wall
  193
+    $this->removeWall($wallId);
  194
+  }
  195
+
138 196
   function isOpenSession($session) {
139 197
     return $this->checkSession($session, true);
140 198
   }
@@ -161,10 +219,9 @@ function checkSession($session, $shouldBeOpen) {
161 219
                        preg_match($this->dateRegEx, @$session['end'])),
162 220
                       "Unexpected session end: " . @$session['end']);
163 221
 
164  
-    return $shouldBeOpen xor $session['end'] !== null;
  222
+    return $shouldBeOpen xor @$session['end'] !== null;
165 223
   }
166 224
 
167  
-
168 225
   function getWall($wallId) {
169 226
     // Set cookie
170 227
     if ($this->sessionId) {
@@ -225,7 +282,7 @@ function createWall($title, $designId) {
225 282
     return $wall['wallId'];
226 283
   }
227 284
 
228  
-  function closeSession($wallId) {
  285
+  function closeSession($wallId, $sessionId) {
229 286
     // Set cookie
230 287
     if ($this->sessionId) {
231 288
       $this->setCookie(WALLMAKER_SESSION_NAME, session_id());
@@ -233,6 +290,7 @@ function closeSession($wallId) {
233 290
 
234 291
     // Prepare payload
235 292
     $payload['wallId'] = $wallId;
  293
+    $payload['sessionId'] = $sessionId;
236 294
 
237 295
     // Make request
238 296
     global $config;
@@ -251,7 +309,7 @@ function closeSession($wallId) {
251 309
     return $parsedResponse;
252 310
   }
253 311
 
254  
-  function startNewSession($wallId) {
  312
+  function startNewSession($wallId, $sessionId) {
255 313
     // Set cookie
256 314
     if ($this->sessionId) {
257 315
       $this->setCookie(WALLMAKER_SESSION_NAME, session_id());
@@ -259,6 +317,7 @@ function startNewSession($wallId) {
259 317
 
260 318
     // Prepare payload
261 319
     $payload['wallId'] = $wallId;
  320
+    $payload['sessionId'] = $sessionId;
262 321
 
263 322
     // Make request
264 323
     global $config;

0 notes on commit 26d229f

Please sign in to comment.
Something went wrong with that request. Please try again.