Fixed Session\Adapter::destroy to correctly clear the $_SESSION

.travis.yml

@@ -1,23 +1,23 @@
 language: php
 
+# TODO To use `sudo: false` you have to use `precise` until this issue is fixed:
+# https://github.com/travis-ci/travis-ci/issues/5837
 dist: precise
 sudo: false
-# TODO use `precise` for `sudo: false` until this issue not fixed:
-# https://github.com/travis-ci/travis-ci/issues/5837
 # dist: trusty
 # sudo: required
 
 php:
   - 5.5
   - 5.6
+  - 7.0
+  - 7.1
 
 matrix:
   fast_finish: true
   allow_failures:
     - php: nightly
   include:
-    - php: 7.0
-    - php: 7.1
     - php: nightly
 
 git:
@@ -101,13 +101,8 @@ install:
   # Uncomment for debug
   # - (cd ext; export CFLAGS="-g3 -O0 -std=gnu90 -Wall"; $(phpenv which phpize) &> /dev/null && ./configure --silent --with-php-config=$(phpenv which php-config) --enable-phalcon &> /dev/null && make --silent -j"$(getconf _NPROCESSORS_ONLN)" &> /dev/null && make --silent install)
   - phpenv config-add tests/_ci/phalcon.ini
-  - phpenv config-add tests/_ci/redis.ini
+  - phpenv config-add tests/_ci/ci.ini
   - beanstalkd -l ${TEST_BT_HOST} -p ${TEST_BT_PORT} & # start queue listener
-  # Debug only
-  #- $(phpenv which php) -m
-  #- pecl list
-  #- beanstalkd -v
-  #- mysql --version
 
 before_script:
   # Uncomment for debug

CHANGELOG.md

@@ -29,6 +29,7 @@
 - Fixed `Phalcon\Mvc\Micro:handle` to correctly handle `afterBinding` handlers
 - Fixed `Phalcon\Mvc\Model::hasChanged` to correctly use it with arrays [#12669](https://github.com/phalcon/cphalcon/issues/12669)
 - Fixed `Phalcon\Mvc\Model\Resultset::delete` to return result depending on success [#11133](https://github.com/phalcon/cphalcon/issues/11133)
+- Fixed `Phalcon\Session\Adapter::destroy` to  correctly clear the `$_SESSION` superglobal [#12326](https://github.com/phalcon/cphalcon/pull/12326), [#12835](https://github.com/phalcon/cphalcon/pull/12835)
 
 # [3.1.2](https://github.com/phalcon/cphalcon/releases/tag/v3.1.2) (2017-04-05)
 - Fixed PHP 7.1 issues [#12055](https://github.com/phalcon/cphalcon/issues/12055)

phalcon/session/adapter.zep

@@ -123,9 +123,9 @@ abstract class Adapter implements AdapterInterface
 	/**
 	 * Gets a session variable from an application context
 	 *
-	 *<code>
+	 * <code>
 	 * $session->get("auth", "yes");
-	 *</code>
+	 * </code>
 	 */
 	public function get(string index, var defaultValue = null, boolean remove = false) -> var
 	{
@@ -192,11 +192,11 @@ abstract class Adapter implements AdapterInterface
 	/**
 	 * Removes a session variable from an application context
 	 *
-	 *<code>
+	 * <code>
 	 * $session->remove("auth");
-	 *</code>
+	 * </code>
 	 */
-	public function remove(string index)
+	public function remove(string index) -> void
 	{
 		var uniqueId;
 
@@ -262,19 +262,8 @@ abstract class Adapter implements AdapterInterface
 	 */
 	public function destroy(boolean removeData = false) -> boolean
 	{
-		var uniqueId, key;
-
 		if removeData {
-			let uniqueId = this->_uniqueId;
-			if !empty uniqueId {
-				for key, _ in _SESSION {
-					if starts_with(key, uniqueId . "#") {
-						unset _SESSION[key];
-					}
-				}
-			} else {
-				let _SESSION = [];
-			}
+			this->removeSessionData();
 		}
 
 		let this->_started = false;
@@ -337,10 +326,14 @@ abstract class Adapter implements AdapterInterface
 
 	/**
 	 * Alias: Removes a session variable from an application context
+	 *
+	 * <code>
+	 * unset($session->auth);
+	 * </code>
 	 */
 	public function __unset(string index)
 	{
-		return this->remove(index);
+		this->remove(index);
 	}
 
 	public function __destruct()
@@ -350,4 +343,25 @@ abstract class Adapter implements AdapterInterface
 			let this->_started = false;
 		}
 	}
+
+	protected function removeSessionData() -> void
+	{
+		var uniqueId, key;
+
+		let uniqueId = this->_uniqueId;
+
+		if empty _SESSION {
+			return;
+		}
+
+		if !empty uniqueId {
+			for key, _ in _SESSION {
+				if starts_with(key, uniqueId . "#") {
+					unset _SESSION[key];
+				}
+			}
+		} else {
+			let _SESSION = [];
+		}
+	}
 }

phalcon/session/adapter/files.zep

@@ -26,7 +26,7 @@ use Phalcon\Session\Adapter;
  *
  * This adapter store sessions in plain files
  *
- *<code>
+ * <code>
  * use Phalcon\Session\Adapter\Files;
  *
  * $session = new Files(
@@ -40,7 +40,7 @@ use Phalcon\Session\Adapter;
  * $session->set("var", "some-value");
  *
  * echo $session->get("var");
- *</code>
+ * </code>
  */
 class Files extends Adapter
 {

phalcon/session/adapter/libmemcached.zep

@@ -29,7 +29,7 @@ use Phalcon\Cache\Frontend\Data as FrontendData;
  *
  * This adapter store sessions in libmemcached
  *
- *<code>
+ * <code>
  * use Phalcon\Session\Adapter\Libmemcached;
  *
  * $session = new Libmemcached(
@@ -55,7 +55,7 @@ use Phalcon\Cache\Frontend\Data as FrontendData;
  * $session->set("var", "some-value");
  *
  * echo $session->get("var");
- *</code>
+ * </code>
  */
 class Libmemcached extends Adapter
 {
@@ -153,19 +153,21 @@ class Libmemcached extends Adapter
 	 */
 	public function destroy(string sessionId = null) -> boolean
 	{
-		var id, key;
+		var id;
 
 		if sessionId === null {
 			let id = this->getId();
 		} else {
 			let id = sessionId;
 		}
 
-		for key, _ in _SESSION {
-			unset _SESSION[key];
+		this->removeSessionData();
+
+		if !empty id && this->_libmemcached->exists(id) {
+			return (bool) this->_libmemcached->delete(id);
 		}
 
-		return this->_libmemcached->exists(id) ? this->_libmemcached->delete(id) : true;
+		return true;
 	}
 
 	/**

phalcon/session/adapter/memcache.zep

@@ -28,7 +28,7 @@ use Phalcon\Cache\Frontend\Data as FrontendData;
  *
  * This adapter store sessions in memcache
  *
- *<code>
+ * <code>
  * use Phalcon\Session\Adapter\Memcache;
  *
  * $session = new Memcache(
@@ -47,7 +47,7 @@ use Phalcon\Cache\Frontend\Data as FrontendData;
  * $session->set("var", "some-value");
  *
  * echo $session->get("var");
- *</code>
+ * </code>
  */
 class Memcache extends Adapter
 {
@@ -134,7 +134,13 @@ class Memcache extends Adapter
 			let id = sessionId;
 		}
 
-		return this->_memcache->exists(id) ? this->_memcache->delete(id) : true;
+		this->removeSessionData();
+
+		if !empty id && this->_memcache->exists(id) {
+			return (bool) this->_memcache->delete(id);
+		}
+
+		return true;
 	}
 
 	/**

phalcon/session/adapter/redis.zep

@@ -28,7 +28,7 @@ use Phalcon\Cache\Frontend\None as FrontendNone;
  *
  * This adapter store sessions in Redis
  *
- *<code>
+ * <code>
  * use Phalcon\Session\Adapter\Redis;
  *
  * $session = new Redis(
@@ -49,7 +49,7 @@ use Phalcon\Cache\Frontend\None as FrontendNone;
  * $session->set("var", "some-value");
  *
  * echo $session->get("var");
- *</code>
+ * </code>
  */
 class Redis extends Adapter
 {
@@ -142,6 +142,8 @@ class Redis extends Adapter
 			let id = sessionId;
 		}
 
+		this->removeSessionData();
+
 		return this->_redis->exists(id) ? this->_redis->delete(id) : true;
 	}
 

phalcon/session/bag.zep

@@ -29,12 +29,12 @@ use Phalcon\Di\InjectionAwareInterface;
  * This component helps to separate session data into "namespaces". Working by this way
  * you can easily create groups of session variables into the application
  *
- *<code>
+ * <code>
  * $user = new \Phalcon\Session\Bag("user");
  *
  * $user->name = "Kimbra Johnson";
  * $user->age  = 22;
- *</code>
+ * </code>
  */
 class Bag implements InjectionAwareInterface, BagInterface, \IteratorAggregate, \ArrayAccess, \Countable
 {

tests/_ci/ci.ini

@@ -0,0 +1,23 @@
+;
+;  Phalcon Framework
+;
+;  Copyright (c) 2011-2017 Phalcon Team (https://www.phalconphp.com)
+;
+;  This source file is subject to the New BSD License that is bundled
+;  with this package in the file LICENSE.txt.
+;
+;  If you did not receive a copy of the license and are unable to
+;  obtain it through the world-wide-web, please send an email
+;  to [email protected] so we can send you a copy immediately.
+
+; These confings are necessary for CI
+
+[opcache]
+opcache.enable_cli=1
+
+[apc]
+apc.enabled=1
+apc.enable_cli=1
+
+[redis]
+extension=redis.so

tests/_ci/pear_setup.sh

@@ -69,9 +69,4 @@ fi
 
 pear config-set php_ini "$(phpenv root)/versions/$(phpenv version-name)/etc/php.ini"
 
-echo "opcache.enable_cli=1" >> "$(phpenv root)/versions/$(phpenv version-name)/etc/php.ini"
-echo "opcache.fast_shutdown=0" >> "$(phpenv root)/versions/$(phpenv version-name)/etc/php.ini"
-echo 'apc.enabled=1' >> "$(phpenv root)/versions/$(phpenv version-name)/etc/php.ini"
-echo 'apc.enable_cli=1' >> "$(phpenv root)/versions/$(phpenv version-name)/etc/php.ini"
-
 pecl channel-update pecl.php.net || true

tests/unit/Session/Adapter/FilesTest.php

@@ -39,6 +39,10 @@ protected function _before()
             'save_path'         => ini_get('session.save_path'),
             'serialize_handler' => ini_get('session.serialize_handler'),
         ];
+
+        if (!isset($_SESSION)) {
+            $_SESSION = [];
+        }
     }
 
     /**
@@ -269,4 +273,36 @@ function () {
             }
         );
     }
+
+    /**
+     * Tests the destroy with cleanning $_SESSION
+     *
+     * @test
+     * @issue  12326
+     * @issue  12835
+     * @author Serghei Iakovelev <[email protected]>
+     * @since  2017-05-08
+     */
+    public function destroyDataFromSessionSuperGlobal()
+    {
+        $this->specify(
+            'The files adapter does not clear session superglobal after destroy',
+            function () {
+                $session = new Files([
+                    'uniqueId' => 'session',
+                    'lifetime' => 3600,
+                ]);
+
+                $session->start();
+
+                $session->test1 = __METHOD__;
+                expect($_SESSION)->hasKey('session#test1');
+                expect($_SESSION['session#test1'])->contains(__METHOD__);
+
+                // @deprecated See: https://github.com/phalcon/cphalcon/issues/12833
+                $session->destroy(true);
+                expect($_SESSION)->hasntKey('session#test1');
+            }
+        );
+    }
 }