From e7bf481cce34735a472d49f93612813c5716597b Mon Sep 17 00:00:00 2001 From: Igor Melnikov Date: Fri, 29 Apr 2016 18:20:59 -0500 Subject: [PATCH 1/5] MAGETWO-51809: Port Redis session adapter to Magento 2.0.* and make it make it backwards compatible Porting changes --- .../Backend/Model/Session/AdminConfig.php | 2 - app/etc/di.xml | 9 + composer.json | 4 +- composer.lock | 81 +- .../Magento/Framework/Session/ConfigTest.php | 21 +- .../Framework/Session/SaveHandlerTest.php | 89 ++ lib/internal/Credis/Client.php | 1113 ----------------- .../Magento/Framework/Session/Config.php | 17 - .../Magento/Framework/Session/SaveHandler.php | 54 +- .../Framework/Session/SaveHandler/Redis.php | 51 + .../Session/SaveHandler/Redis/Config.php | 291 +++++ .../Session/SaveHandler/Redis/Logger.php | 93 ++ .../Session/Test/Unit/ConfigTest.php | 33 +- .../Unit/SaveHandler/Redis/ConfigTest.php | 249 ++++ .../Unit/SaveHandler/Redis/LoggerTest.php | 95 ++ .../ConfigOptionsListTest.php | 4 +- 16 files changed, 1017 insertions(+), 1189 deletions(-) create mode 100644 dev/tests/integration/testsuite/Magento/Framework/Session/SaveHandlerTest.php delete mode 100644 lib/internal/Credis/Client.php create mode 100644 lib/internal/Magento/Framework/Session/SaveHandler/Redis.php create mode 100644 lib/internal/Magento/Framework/Session/SaveHandler/Redis/Config.php create mode 100644 lib/internal/Magento/Framework/Session/SaveHandler/Redis/Logger.php create mode 100644 lib/internal/Magento/Framework/Session/Test/Unit/SaveHandler/Redis/ConfigTest.php create mode 100644 lib/internal/Magento/Framework/Session/Test/Unit/SaveHandler/Redis/LoggerTest.php rename setup/src/Magento/Setup/Test/Unit/{Module => Model}/ConfigOptionsListTest.php (99%) diff --git a/app/code/Magento/Backend/Model/Session/AdminConfig.php b/app/code/Magento/Backend/Model/Session/AdminConfig.php index c23af37112b0a..3b5712fff986a 100644 --- a/app/code/Magento/Backend/Model/Session/AdminConfig.php +++ b/app/code/Magento/Backend/Model/Session/AdminConfig.php @@ -14,8 +14,6 @@ /** * Magento Backend session configuration - * - * @method Config setSaveHandler() */ class AdminConfig extends Config { diff --git a/app/etc/di.xml b/app/etc/di.xml index e4c26930083f4..69403de168ff4 100755 --- a/app/etc/di.xml +++ b/app/etc/di.xml @@ -141,6 +141,8 @@ + + @@ -186,9 +188,16 @@ Magento\Framework\Session\SaveHandler\DbTable + Magento\Framework\Session\SaveHandler\Redis + + + Cm\RedisSession\Handler\ConfigInterface + Cm\RedisSession\Handler\LoggerInterface + + global diff --git a/composer.json b/composer.json index cde51690c9784..f1416737eba87 100644 --- a/composer.json +++ b/composer.json @@ -38,6 +38,8 @@ "zendframework/zend-log": "~2.4.6", "zendframework/zend-http": "~2.4.6", "magento/zendframework1": "1.12.16", + "colinmollenhour/credis": "1.6", + "colinmollenhour/php-redis-session-abstract": "1.1", "composer/composer": "1.0.0-alpha10", "monolog/monolog": "1.16.0", "oyejorge/less.php": "1.7.0.3", @@ -188,7 +190,6 @@ "magento/framework": "100.0.7", "trentrichardson/jquery-timepicker-addon": "1.4.3", "colinmollenhour/cache-backend-redis": "1.8", - "colinmollenhour/credis": "1.5", "components/jquery": "1.11.0", "blueimp/jquery-file-upload": "5.6.14", "components/jqueryui": "1.10.4", @@ -199,7 +200,6 @@ "component_paths": { "trentrichardson/jquery-timepicker-addon": "lib/web/jquery/jquery-ui-timepicker-addon.js", "colinmollenhour/cache-backend-redis": "lib/internal/Cm/Cache/Backend/Redis.php", - "colinmollenhour/credis": "lib/internal/Credis", "components/jquery": [ "lib/web/jquery.js", "lib/web/jquery/jquery.min.js", diff --git a/composer.lock b/composer.lock index 806c35585bc90..14168999feb63 100644 --- a/composer.lock +++ b/composer.lock @@ -4,8 +4,8 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file", "This file is @generated automatically" ], - "hash": "abce4a98b88ce9a69fa9523a1a23cd6b", - "content-hash": "db8c1ae4bfd5ed6939d8b435513d8242", + "hash": "408e530eda6354cb3c58f50602e6baf8", + "content-hash": "21da9781095d5a52e26dc30f72587201", "packages": [ { "name": "braintree/braintree_php", @@ -52,6 +52,83 @@ "description": "Braintree PHP Client Library", "time": "2015-05-07 16:53:06" }, + { + "name": "colinmollenhour/credis", + "version": "1.6", + "source": { + "type": "git", + "url": "https://github.com/colinmollenhour/credis.git", + "reference": "409edfd0ea81f5cb74afbdb86df54890c207b5e4" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/colinmollenhour/credis/zipball/409edfd0ea81f5cb74afbdb86df54890c207b5e4", + "reference": "409edfd0ea81f5cb74afbdb86df54890c207b5e4", + "shasum": "" + }, + "require": { + "php": ">=5.3.0" + }, + "type": "library", + "autoload": { + "classmap": [ + "Client.php", + "Cluster.php", + "Sentinel.php" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Colin Mollenhour", + "email": "colin@mollenhour.com" + } + ], + "description": "Credis is a lightweight interface to the Redis key-value store which wraps the phpredis library when available for better performance.", + "homepage": "https://github.com/colinmollenhour/credis", + "time": "2015-11-28 01:20:04" + }, + { + "name": "colinmollenhour/php-redis-session-abstract", + "version": "v1.1", + "source": { + "type": "git", + "url": "https://github.com/colinmollenhour/php-redis-session-abstract.git", + "reference": "95330b7f29663dab81f53d1a438e4d927b6c5f66" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/colinmollenhour/php-redis-session-abstract/zipball/95330b7f29663dab81f53d1a438e4d927b6c5f66", + "reference": "95330b7f29663dab81f53d1a438e4d927b6c5f66", + "shasum": "" + }, + "require": { + "colinmollenhour/credis": "1.6", + "magento/zendframework1": "1.12.16", + "php": "~5.5.0|~5.6.0|~7.0.0" + }, + "type": "library", + "autoload": { + "psr-0": { + "Cm\\RedisSession\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "BSD-3-Clause" + ], + "authors": [ + { + "name": "Colin Mollenhour" + } + ], + "description": "A Redis-based session handler with optimistic locking", + "homepage": "https://github.com/colinmollenhour/php-redis-session-abstract", + "time": "2016-02-03 18:13:49" + }, { "name": "composer/composer", "version": "1.0.0-alpha10", diff --git a/dev/tests/integration/testsuite/Magento/Framework/Session/ConfigTest.php b/dev/tests/integration/testsuite/Magento/Framework/Session/ConfigTest.php index 869d9d3493056..187ac69451e93 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/Session/ConfigTest.php +++ b/dev/tests/integration/testsuite/Magento/Framework/Session/ConfigTest.php @@ -36,15 +36,12 @@ protected function setUp() $sessionManager->writeClose(); } $this->deploymentConfigMock = $this->getMock('Magento\Framework\App\DeploymentConfig', [], [], '', false); + $this->deploymentConfigMock->expects($this->at(0)) - ->method('get') - ->with($this->equalTo(Config::PARAM_SESSION_SAVE_METHOD), $this->anything()) - ->will($this->returnValue('files')); - $this->deploymentConfigMock->expects($this->at(1)) ->method('get') ->with(Config::PARAM_SESSION_SAVE_PATH) ->will($this->returnValue(null)); - $this->deploymentConfigMock->expects($this->at(2)) + $this->deploymentConfigMock->expects($this->at(1)) ->method('get') ->with(Config::PARAM_SESSION_CACHE_LIMITER) ->will($this->returnValue($this->_cacheLimiter)); @@ -86,11 +83,6 @@ public function testDefaultConfiguration() $this->assertEquals($this->_model->getSavePath(), $this->_model->getOption('save_path')); } - public function testGetSessionSaveMethod() - { - $this->assertEquals('files', $this->_model->getSaveHandler()); - } - /** * Unable to add integration tests for testGetLifetimePathNonDefault * @@ -122,7 +114,6 @@ public function optionsProvider() return [ ['save_path', 'getSavePath', __DIR__], ['name', 'getName', 'FOOBAR'], - ['save_handler', 'getSaveHandler', 'user'], ['gc_probability', 'getGcProbability', 42], ['gc_divisor', 'getGcDivisor', 3], ['gc_maxlifetime', 'getGcMaxlifetime', 180], @@ -152,12 +143,6 @@ public function testNameIsMutable() $this->assertEquals('FOOBAR', $this->_model->getName()); } - public function testSaveHandlerIsMutable() - { - $this->_model->setSaveHandler('user'); - $this->assertEquals('user', $this->_model->getSaveHandler()); - } - public function testCookieLifetimeIsMutable() { $this->_model->setCookieLifetime(20); @@ -295,7 +280,7 @@ public function testConstructorSavePath($existing, $given, $expected) $this->markTestSkipped('Cannot set session.save_path with ini_set'); } - $this->deploymentConfigMock->expects($this->at(1)) + $this->deploymentConfigMock->expects($this->at(0)) ->method('get') ->with(Config::PARAM_SESSION_SAVE_PATH) ->will($this->returnValue($given)); diff --git a/dev/tests/integration/testsuite/Magento/Framework/Session/SaveHandlerTest.php b/dev/tests/integration/testsuite/Magento/Framework/Session/SaveHandlerTest.php new file mode 100644 index 0000000000000..ea36ac027ce2b --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Framework/Session/SaveHandlerTest.php @@ -0,0 +1,89 @@ +sessionConfig = ObjectManager::getInstance()->get(ConfigInterface::class); + $this->deploymentConfig = ObjectManager::getInstance()->get(DeploymentConfig::class); + } + + /** + * Tests that the session handler is correctly set when object is created. + * + * @dataProvider saveHandlerProvider + * @param string $deploymentConfigHandler + * @param string $iniHandler + */ + public function testSetSaveHandler($deploymentConfigHandler, $iniHandler) + { + // Set expected session.save_handler config + if ($deploymentConfigHandler) { + if ($deploymentConfigHandler !== 'files') { + $expected = 'user'; + } else { + $expected = $deploymentConfigHandler; + } + } else if ($iniHandler) { + $expected = $iniHandler; + } else { + $expected = SaveHandlerInterface::DEFAULT_HANDLER; + } + + // Set ini configuration + if ($iniHandler) { + $oldIni = ini_set('session.save_handler', $iniHandler); + } + + /** @var DeploymentConfig | \PHPUnit_Framework_MockObject_MockObject $deploymentConfigMock */ + $deploymentConfigMock = $this->getMockBuilder(DeploymentConfig::class) + ->disableOriginalConstructor() + ->getMock(); + $deploymentConfigMock->expects($this->once()) + ->method('get') + ->with(Config::PARAM_SESSION_SAVE_METHOD, SaveHandlerInterface::DEFAULT_HANDLER) + ->willReturn($deploymentConfigHandler ?: SaveHandlerInterface::DEFAULT_HANDLER); + + new SaveHandler( + ObjectManager::getInstance()->get(SaveHandlerFactory::class), + $deploymentConfigMock + ); + + // Test expectation + $this->assertEquals( + $expected, + ObjectManager::getInstance()->get(ConfigInterface::class)->getOption('session.save_handler') + ); + + // Reset ini configuration + if (isset($oldIni)) { + ini_set('session.save_handler', $oldIni); + } + } + + public function saveHandlerProvider() + { + return [ + ['db', false], + ['db', 'files'], + [false, 'files'], + [false, false], + ]; + } +} diff --git a/lib/internal/Credis/Client.php b/lib/internal/Credis/Client.php deleted file mode 100644 index 5945709b837c8..0000000000000 --- a/lib/internal/Credis/Client.php +++ /dev/null @@ -1,1113 +0,0 @@ - - * @copyright 2011 Colin Mollenhour - * @license http://www.opensource.org/licenses/mit-license.php The MIT License - * @package Credis_Client - */ - -if( ! defined('CRLF')) define('CRLF', sprintf('%s%s', chr(13), chr(10))); - -/** - * Credis-specific errors, wraps native Redis errors - */ -class CredisException extends Exception -{ - - const CODE_TIMED_OUT = 1; - const CODE_DISCONNECTED = 2; - - public function __construct($message, $code = 0, $exception = NULL) - { - if ($exception && get_class($exception) == 'RedisException' && $message == 'read error on connection') { - $code = CredisException::CODE_DISCONNECTED; - } - parent::__construct($message, $code, $exception); - } - -} - -/** - * Credis_Client, a lightweight Redis PHP standalone client and phpredis wrapper - * - * Server/Connection: - * @method Credis_Client pipeline() - * @method Credis_Client multi() - * @method array exec() - * @method string flushAll() - * @method string flushDb() - * @method array info() - * @method bool|array config(string $setGet, string $key, string $value = null) - * - * Keys: - * @method int del(string $key) - * @method int exists(string $key) - * @method int expire(string $key, int $seconds) - * @method int expireAt(string $key, int $timestamp) - * @method array keys(string $key) - * @method int persist(string $key) - * @method bool rename(string $key, string $newKey) - * @method bool renameNx(string $key, string $newKey) - * @method array sort(string $key, string $arg1, string $valueN = null) - * @method int ttl(string $key) - * @method string type(string $key) - * - * Scalars: - * @method int append(string $key, string $value) - * @method int decr(string $key) - * @method int decrBy(string $key, int $decrement) - * @method bool|string get(string $key) - * @method int getBit(string $key, int $offset) - * @method string getRange(string $key, int $start, int $end) - * @method string getSet(string $key, string $value) - * @method int incr(string $key) - * @method int incrBy(string $key, int $decrement) - * @method array mGet(array $keys) - * @method bool mSet(array $keysValues) - * @method int mSetNx(array $keysValues) - * @method bool set(string $key, string $value) - * @method int setBit(string $key, int $offset, int $value) - * @method bool setEx(string $key, int $seconds, string $value) - * @method int setNx(string $key, string $value) - * @method int setRange(string $key, int $offset, int $value) - * @method int strLen(string $key) - * - * Sets: - * @method int sAdd(string $key, mixed $value, string $valueN = null) - * @method int sRem(string $key, mixed $value, string $valueN = null) - * @method array sMembers(string $key) - * @method array sUnion(mixed $keyOrArray, string $valueN = null) - * @method array sInter(mixed $keyOrArray, string $valueN = null) - * @method array sDiff(mixed $keyOrArray, string $valueN = null) - * @method string sPop(string $key) - * @method int sCard(string $key) - * @method int sIsMember(string $key, string $member) - * @method int sMove(string $source, string $dest, string $member) - * @method string|array sRandMember(string $key, int $count = null) - * @method int sUnionStore(string $dest, string $key1, string $key2 = null) - * @method int sInterStore(string $dest, string $key1, string $key2 = null) - * @method int sDiffStore(string $dest, string $key1, string $key2 = null) - * - * Hashes: - * @method bool|int hSet(string $key, string $field, string $value) - * @method bool hSetNx(string $key, string $field, string $value) - * @method bool|string hGet(string $key, string $field) - * @method bool|int hLen(string $key) - * @method bool hDel(string $key, string $field) - * @method array hKeys(string $key, string $field) - * @method array hVals(string $key, string $field) - * @method array hGetAll(string $key) - * @method bool hExists(string $key, string $field) - * @method int hIncrBy(string $key, string $field, int $value) - * @method bool hMSet(string $key, array $keysValues) - * @method array hMGet(string $key, array $fields) - * - * Lists: - * @method array|null blPop(string $keyN, int $timeout) - * @method array|null brPop(string $keyN, int $timeout) - * @method array|null brPoplPush(string $source, string $destination, int $timeout) - * @method string|null lIndex(string $key, int $index) - * @method int lInsert(string $key, string $beforeAfter, string $pivot, string $value) - * @method int lLen(string $key) - * @method string|null lPop(string $key) - * @method int lPush(string $key, mixed $value, mixed $valueN = null) - * @method int lPushX(string $key, mixed $value) - * @method array lRange(string $key, int $start, int $stop) - * @method int lRem(string $key, int $count, mixed $value) - * @method bool lSet(string $key, int $index, mixed $value) - * @method bool lTrim(string $key, int $start, int $stop) - * @method string|null rPop(string $key) - * @method string|null rPoplPush(string $source, string $destination) - * @method int rPush(string $key, mixed $value, mixed $valueN = null) - * @method int rPushX(string $key, mixed $value) - * - * Sorted Sets: - * TODO - * - * Pub/Sub - * @method array pUnsubscribe(mixed $pattern, string $patternN = NULL)) - * @method array unsubscribe(mixed $channel, string $channelN = NULL)) - * @method int publish(string $channel, string $message) - * @method int|array pubsub(string $subCommand, $arg = NULL) - * - * Scripting: - * @method string|int script(string $command, string $arg1 = null) - * @method string|int|array|bool eval(string $script, array $keys = NULL, array $args = NULL) - * @method string|int|array|bool evalSha(string $script, array $keys = NULL, array $args = NULL) - */ -class Credis_Client { - - const TYPE_STRING = 'string'; - const TYPE_LIST = 'list'; - const TYPE_SET = 'set'; - const TYPE_ZSET = 'zset'; - const TYPE_HASH = 'hash'; - const TYPE_NONE = 'none'; - const FREAD_BLOCK_SIZE = 8192; - - /** - * Socket connection to the Redis server or Redis library instance - * @var resource|Redis - */ - protected $redis; - protected $redisMulti; - - /** - * Host of the Redis server - * @var string - */ - protected $host; - - /** - * Port on which the Redis server is running - * @var integer - */ - protected $port; - - /** - * Timeout for connecting to Redis server - * @var float - */ - protected $timeout; - - /** - * Timeout for reading response from Redis server - * @var float - */ - protected $readTimeout; - - /** - * Unique identifier for persistent connections - * @var string - */ - protected $persistent; - - /** - * @var bool - */ - protected $closeOnDestruct = TRUE; - - /** - * @var bool - */ - protected $connected = FALSE; - - /** - * @var bool - */ - protected $standalone; - - /** - * @var int - */ - protected $maxConnectRetries = 0; - - /** - * @var int - */ - protected $connectFailures = 0; - - /** - * @var bool - */ - protected $usePipeline = FALSE; - - /** - * @var array - */ - protected $commandNames; - - /** - * @var string - */ - protected $commands; - - /** - * @var bool - */ - protected $isMulti = FALSE; - - /** - * @var bool - */ - protected $isWatching = FALSE; - - /** - * @var string - */ - protected $authPassword; - - /** - * @var int - */ - protected $selectedDb = 0; - - /** - * Aliases for backwards compatibility with phpredis - * @var array - */ - protected $wrapperMethods = array('delete' => 'del', 'getkeys' => 'keys', 'sremove' => 'srem'); - - /** - * @var array - */ - protected $renamedCommands; - - /** - * @var int - */ - protected $requests = 0; - - /** - * Creates a Redisent connection to the Redis server on host {@link $host} and port {@link $port}. - * $host may also be a path to a unix socket or a string in the form of tcp://[hostname]:[port] or unix://[path] - * - * @param string $host The hostname of the Redis server - * @param integer $port The port number of the Redis server - * @param float $timeout Timeout period in seconds - * @param string $persistent Flag to establish persistent connection - * @param int $db The selected datbase of the Redis server - * @param string $password The authentication password of the Redis server - */ - public function __construct($host = '127.0.0.1', $port = 6379, $timeout = null, $persistent = '', $db = 0, $password = null) - { - $this->host = (string) $host; - $this->port = (int) $port; - $this->timeout = $timeout; - $this->persistent = (string) $persistent; - $this->standalone = ! extension_loaded('redis'); - $this->authPassword = $password; - $this->selectedDb = (int)$db; - $this->convertHost(); - } - - public function __destruct() - { - if ($this->closeOnDestruct) { - $this->close(); - } - } - /** - * Return the host of the Redis instance - * @return string - */ - public function getHost() - { - return $this->host; - } - /** - * Return the port of the Redis instance - * @return int - */ - public function getPort() - { - return $this->port; - } - - /** - * Return the selected database - * @return int - */ - public function getSelectedDb() - { - return $this->selectedDb; - } - /** - * @return string - */ - public function getPersistence() - { - return $this->persistent; - } - /** - * @throws CredisException - * @return Credis_Client - */ - public function forceStandalone() - { - if($this->connected) { - throw new CredisException('Cannot force Credis_Client to use standalone PHP driver after a connection has already been established.'); - } - $this->standalone = TRUE; - return $this; - } - - /** - * @param int $retries - * @return Credis_Client - */ - public function setMaxConnectRetries($retries) - { - $this->maxConnectRetries = $retries; - return $this; - } - - /** - * @param bool $flag - * @return Credis_Client - */ - public function setCloseOnDestruct($flag) - { - $this->closeOnDestruct = $flag; - return $this; - } - protected function convertHost() - { - if (preg_match('#^(tcp|unix)://(.*)$#', $this->host, $matches)) { - if($matches[1] == 'tcp') { - if ( ! preg_match('#^([^:]+)(:([0-9]+))?(/(.+))?$#', $matches[2], $matches)) { - throw new CredisException('Invalid host format; expected tcp://host[:port][/persistence_identifier]'); - } - $this->host = $matches[1]; - $this->port = (int) (isset($matches[3]) ? $matches[3] : 6379); - $this->persistent = isset($matches[5]) ? $matches[5] : ''; - } else { - $this->host = $matches[2]; - $this->port = NULL; - if (substr($this->host,0,1) != '/') { - throw new CredisException('Invalid unix socket format; expected unix:///path/to/redis.sock'); - } - } - } - if ($this->port !== NULL && substr($this->host,0,1) == '/') { - $this->port = NULL; - } - } - /** - * @throws CredisException - * @return Credis_Client - */ - public function connect() - { - if ($this->connected) { - return $this; - } - if ($this->standalone) { - $flags = STREAM_CLIENT_CONNECT; - $remote_socket = $this->port === NULL - ? 'unix://'.$this->host - : 'tcp://'.$this->host.':'.$this->port; - if ($this->persistent) { - if ($this->port === NULL) { // Unix socket - throw new CredisException('Persistent connections to UNIX sockets are not supported in standalone mode.'); - } - $remote_socket .= '/'.$this->persistent; - $flags = $flags | STREAM_CLIENT_PERSISTENT; - } - $result = $this->redis = @stream_socket_client($remote_socket, $errno, $errstr, $this->timeout !== null ? $this->timeout : 2.5, $flags); - } - else { - if ( ! $this->redis) { - $this->redis = new Redis; - } - $result = $this->persistent - ? $this->redis->pconnect($this->host, $this->port, $this->timeout, $this->persistent) - : $this->redis->connect($this->host, $this->port, $this->timeout); - } - - // Use recursion for connection retries - if ( ! $result) { - $this->connectFailures++; - if ($this->connectFailures <= $this->maxConnectRetries) { - return $this->connect(); - } - $failures = $this->connectFailures; - $this->connectFailures = 0; - throw new CredisException("Connection to Redis failed after $failures failures." . (isset($errno) && isset($errstr) ? "Last Error : ({$errno}) {$errstr}" : "")); - } - - $this->connectFailures = 0; - $this->connected = TRUE; - - // Set read timeout - if ($this->readTimeout) { - $this->setReadTimeout($this->readTimeout); - } - - if($this->authPassword !== null) { - $this->auth($this->authPassword); - } - if($this->selectedDb !== 0) { - $this->select($this->selectedDb); - } - return $this; - } - /** - * @return bool - */ - public function isConnected() - { - return $this->connected; - } - /** - * Set the read timeout for the connection. Use 0 to disable timeouts entirely (or use a very long timeout - * if not supported). - * - * @param int $timeout 0 (or -1) for no timeout, otherwise number of seconds - * @throws CredisException - * @return Credis_Client - */ - public function setReadTimeout($timeout) - { - if ($timeout < -1) { - throw new CredisException('Timeout values less than -1 are not accepted.'); - } - $this->readTimeout = $timeout; - if ($this->connected) { - if ($this->standalone) { - $timeout = $timeout <= 0 ? 315360000 : $timeout; // Ten-year timeout - stream_set_blocking($this->redis, TRUE); - stream_set_timeout($this->redis, (int) floor($timeout), ($timeout - floor($timeout)) * 1000000); - } else if (defined('Redis::OPT_READ_TIMEOUT')) { - // supported in phpredis 2.2.3 - // a timeout value of -1 means reads will not timeout - $timeout = $timeout == 0 ? -1 : $timeout; - $this->redis->setOption(Redis::OPT_READ_TIMEOUT, $timeout); - } - } - return $this; - } - - /** - * @return bool - */ - public function close() - { - $result = TRUE; - if ($this->connected && ! $this->persistent) { - try { - $result = $this->standalone ? fclose($this->redis) : $this->redis->close(); - $this->connected = FALSE; - } catch (Exception $e) { - ; // Ignore exceptions on close - } - } - return $result; - } - - /** - * Enabled command renaming and provide mapping method. Supported methods are: - * - * 1. renameCommand('foo') // Salted md5 hash for all commands -> md5('foo'.$command) - * 2. renameCommand(function($command){ return 'my'.$command; }); // Callable - * 3. renameCommand('get', 'foo') // Single command -> alias - * 4. renameCommand(['get' => 'foo', 'set' => 'bar']) // Full map of [command -> alias] - * - * @param string|callable|array $command - * @param string|null $alias - * @return $this - */ - public function renameCommand($command, $alias = NULL) - { - if ( ! $this->standalone) { - $this->forceStandalone(); - } - if ($alias === NULL) { - $this->renamedCommands = $command; - } else { - if ( ! $this->renamedCommands) { - $this->renamedCommands = array(); - } - $this->renamedCommands[$command] = $alias; - } - return $this; - } - - /** - * @param $command - */ - public function getRenamedCommand($command) - { - static $map; - - // Command renaming not enabled - if ($this->renamedCommands === NULL) { - return $command; - } - - // Initialize command map - if ($map === NULL) { - if (is_array($this->renamedCommands)) { - $map = $this->renamedCommands; - } else { - $map = array(); - } - } - - // Generate and return cached result - if ( ! isset($map[$command])) { - // String means all commands are hashed with salted md5 - if (is_string($this->renamedCommands)) { - $map[$command] = md5($this->renamedCommands.$command); - } - // Would already be set in $map if it was intended to be renamed - else if (is_array($this->renamedCommands)) { - return $command; - } - // User-supplied function - else if (is_callable($this->renamedCommands)) { - $map[$command] = call_user_func($this->renamedCommands, $command); - } - } - return $map[$command]; - } - - /** - * @param string $password - * @return bool - */ - public function auth($password) - { - $response = $this->__call('auth', array($password)); - $this->authPassword = $password; - return $response; - } - - /** - * @param int $index - * @return bool - */ - public function select($index) - { - $response = $this->__call('select', array($index)); - $this->selectedDb = (int) $index; - return $response; - } - - /** - * @param string|array $patterns - * @param $callback - * @return $this|array|bool|Credis_Client|mixed|null|string - * @throws CredisException - */ - public function pSubscribe($patterns, $callback) - { - if ( ! $this->standalone) { - return $this->__call('pSubscribe', array((array)$patterns, $callback)); - } - - // Standalone mode: use infinite loop to subscribe until timeout - $patternCount = is_array($patterns) ? count($patterns) : 1; - while ($patternCount--) { - if (isset($status)) { - list($command, $pattern, $status) = $this->read_reply(); - } else { - list($command, $pattern, $status) = $this->__call('psubscribe', array($patterns)); - } - if ( ! $status) { - throw new CredisException('Invalid pSubscribe response.'); - } - } - try { - while (1) { - list($type, $pattern, $channel, $message) = $this->read_reply(); - if ($type != 'pmessage') { - throw new CredisException('Received non-pmessage reply.'); - } - $callback($this, $pattern, $channel, $message); - } - } catch (CredisException $e) { - if ($e->getCode() == CredisException::CODE_TIMED_OUT) { - try { - list($command, $pattern, $status) = $this->pUnsubscribe($patterns); - while ($status !== 0) { - list($command, $pattern, $status) = $this->read_reply(); - } - } catch (CredisException $e2) { - throw $e2; - } - } - throw $e; - } - } - - /** - * @param string|array $channels - * @param $callback - * @throws CredisException - * @return $this|array|bool|Credis_Client|mixed|null|string - */ - public function subscribe($channels, $callback) - { - if ( ! $this->standalone) { - return $this->__call('subscribe', array((array)$channels, $callback)); - } - - // Standalone mode: use infinite loop to subscribe until timeout - $channelCount = is_array($channels) ? count($channels) : 1; - while ($channelCount--) { - if (isset($status)) { - list($command, $channel, $status) = $this->read_reply(); - } else { - list($command, $channel, $status) = $this->__call('subscribe', array($channels)); - } - if ( ! $status) { - throw new CredisException('Invalid subscribe response.'); - } - } - try { - while (1) { - list($type, $channel, $message) = $this->read_reply(); - if ($type != 'message') { - throw new CredisException('Received non-message reply.'); - } - $callback($this, $channel, $message); - } - } catch (CredisException $e) { - if ($e->getCode() == CredisException::CODE_TIMED_OUT) { - try { - list($command, $channel, $status) = $this->unsubscribe($channels); - while ($status !== 0) { - list($command, $channel, $status) = $this->read_reply(); - } - } catch (CredisException $e2) { - throw $e2; - } - } - throw $e; - } - } - - public function __call($name, $args) - { - // Lazy connection - $this->connect(); - - $name = strtolower($name); - - // Send request via native PHP - if($this->standalone) - { - switch ($name) { - case 'eval': - case 'evalsha': - $script = array_shift($args); - $keys = (array) array_shift($args); - $eArgs = (array) array_shift($args); - $args = array($script, count($keys), $keys, $eArgs); - break; - } - // Flatten arguments - $argsFlat = NULL; - foreach($args as $index => $arg) { - if(is_array($arg)) { - if($argsFlat === NULL) { - $argsFlat = array_slice($args, 0, $index); - } - if($name == 'mset' || $name == 'msetnx' || $name == 'hmset') { - foreach($arg as $key => $value) { - $argsFlat[] = $key; - $argsFlat[] = $value; - } - } else { - $argsFlat = array_merge($argsFlat, $arg); - } - } else if($argsFlat !== NULL) { - $argsFlat[] = $arg; - } - } - if($argsFlat !== NULL) { - $args = $argsFlat; - $argsFlat = NULL; - } - - // In pipeline mode - if($this->usePipeline) - { - if($name == 'pipeline') { - throw new CredisException('A pipeline is already in use and only one pipeline is supported.'); - } - else if($name == 'exec') { - if($this->isMulti) { - $this->commandNames[] = $name; - $this->commands .= self::_prepare_command(array($this->getRenamedCommand($name))); - } - - // Write request - if($this->commands) { - $this->write_command($this->commands); - } - $this->commands = NULL; - - // Read response - $response = array(); - foreach($this->commandNames as $command) { - $response[] = $this->read_reply($command); - } - $this->commandNames = NULL; - - if($this->isMulti) { - $response = array_pop($response); - } - $this->usePipeline = $this->isMulti = FALSE; - return $response; - } - else { - if($name == 'multi') { - $this->isMulti = TRUE; - } - array_unshift($args, $this->getRenamedCommand($name)); - $this->commandNames[] = $name; - $this->commands .= self::_prepare_command($args); - return $this; - } - } - - // Start pipeline mode - if($name == 'pipeline') - { - $this->usePipeline = TRUE; - $this->commandNames = array(); - $this->commands = ''; - return $this; - } - - // If unwatching, allow reconnect with no error thrown - if($name == 'unwatch') { - $this->isWatching = FALSE; - } - - // Non-pipeline mode - array_unshift($args, $this->getRenamedCommand($name)); - $command = self::_prepare_command($args); - $this->write_command($command); - $response = $this->read_reply($name); - - // Watch mode disables reconnect so error is thrown - if($name == 'watch') { - $this->isWatching = TRUE; - } - // Transaction mode - else if($this->isMulti && ($name == 'exec' || $name == 'discard')) { - $this->isMulti = FALSE; - } - // Started transaction - else if($this->isMulti || $name == 'multi') { - $this->isMulti = TRUE; - $response = $this; - } - } - - // Send request via phpredis client - else - { - // Tweak arguments - switch($name) { - case 'get': // optimize common cases - case 'set': - case 'hget': - case 'hset': - case 'setex': - case 'mset': - case 'msetnx': - case 'hmset': - case 'hmget': - case 'del': - break; - case 'mget': - if(isset($args[0]) && ! is_array($args[0])) { - $args = array($args); - } - break; - case 'lrem': - $args = array($args[0], $args[2], $args[1]); - break; - case 'eval': - case 'evalsha': - if (isset($args[1]) && is_array($args[1])) { - $cKeys = $args[1]; - } elseif (isset($args[1]) && is_string($args[1])) { - $cKeys = array($args[1]); - } else { - $cKeys = array(); - } - if (isset($args[2]) && is_array($args[2])) { - $cArgs = $args[2]; - } elseif (isset($args[2]) && is_string($args[2])) { - $cArgs = array($args[2]); - } else { - $cArgs = array(); - } - $args = array($args[0], array_merge($cKeys, $cArgs), count($cKeys)); - break; - case 'subscribe': - case 'psubscribe': - break; - default: - // Flatten arguments - $argsFlat = NULL; - foreach($args as $index => $arg) { - if(is_array($arg)) { - if($argsFlat === NULL) { - $argsFlat = array_slice($args, 0, $index); - } - $argsFlat = array_merge($argsFlat, $arg); - } else if($argsFlat !== NULL) { - $argsFlat[] = $arg; - } - } - if($argsFlat !== NULL) { - $args = $argsFlat; - $argsFlat = NULL; - } - } - - try { - // Proxy pipeline mode to the phpredis library - if($name == 'pipeline' || $name == 'multi') { - if($this->isMulti) { - return $this; - } else { - $this->isMulti = TRUE; - $this->redisMulti = call_user_func_array(array($this->redis, $name), $args); - } - } - else if($name == 'exec' || $name == 'discard') { - $this->isMulti = FALSE; - $response = $this->redisMulti->$name(); - $this->redisMulti = NULL; - #echo "> $name : ".substr(print_r($response, TRUE),0,100)."\n"; - return $response; - } - - // Use aliases to be compatible with phpredis wrapper - if(isset($this->wrapperMethods[$name])) { - $name = $this->wrapperMethods[$name]; - } - - // Multi and pipeline return self for chaining - if($this->isMulti) { - call_user_func_array(array($this->redisMulti, $name), $args); - return $this; - } - - // Send request, retry one time when using persistent connections on the first request only - $this->requests++; - try { - $response = call_user_func_array(array($this->redis, $name), $args); - } catch (RedisException $e) { - if ($this->persistent && $this->requests == 1 && $e->getMessage() == 'read error on connection') { - $this->connected = FALSE; - $this->connect(); - $response = call_user_func_array(array($this->redis, $name), $args); - } else { - throw $e; - } - } - } - // Wrap exceptions - catch(RedisException $e) { - $code = 0; - if ( ! ($result = $this->redis->IsConnected())) { - $this->connected = FALSE; - $code = CredisException::CODE_DISCONNECTED; - } - throw new CredisException($e->getMessage(), $code, $e); - } - - #echo "> $name : ".substr(print_r($response, TRUE),0,100)."\n"; - - // change return values where it is too difficult to minim in standalone mode - switch($name) - { - case 'hmget': - $response = array_values($response); - break; - - case 'type': - $typeMap = array( - self::TYPE_NONE, - self::TYPE_STRING, - self::TYPE_SET, - self::TYPE_LIST, - self::TYPE_ZSET, - self::TYPE_HASH, - ); - $response = $typeMap[$response]; - break; - - // Handle scripting errors - case 'eval': - case 'evalsha': - case 'script': - $error = $this->redis->getLastError(); - $this->redis->clearLastError(); - if ($error && substr($error,0,8) == 'NOSCRIPT') { - $response = NULL; - } else if ($error) { - throw new CredisException($error); - } - break; - default: - $error = $this->redis->getLastError(); - $this->redis->clearLastError(); - if ($error) { - throw new CredisException($error); - } - break; - } - } - - return $response; - } - - protected function write_command($command) - { - // Reconnect on lost connection (Redis server "timeout" exceeded since last command) - if(feof($this->redis)) { - $this->close(); - // If a watch or transaction was in progress and connection was lost, throw error rather than reconnect - // since transaction/watch state will be lost. - if(($this->isMulti && ! $this->usePipeline) || $this->isWatching) { - $this->isMulti = $this->isWatching = FALSE; - throw new CredisException('Lost connection to Redis server during watch or transaction.'); - } - $this->connected = FALSE; - $this->connect(); - if($this->authPassword) { - $this->auth($this->authPassword); - } - if($this->selectedDb != 0) { - $this->select($this->selectedDb); - } - } - - $commandLen = strlen($command); - for ($written = 0; $written < $commandLen; $written += $fwrite) { - $fwrite = fwrite($this->redis, substr($command, $written)); - if ($fwrite === FALSE || $fwrite == 0 ) { - $this->connected = FALSE; - throw new CredisException('Failed to write entire command to stream'); - } - } - } - - protected function read_reply($name = '') - { - $reply = fgets($this->redis); - if($reply === FALSE) { - $info = stream_get_meta_data($this->redis); - if ($info['timed_out']) { - throw new CredisException('Read operation timed out.', CredisException::CODE_TIMED_OUT); - } else { - $this->connected = FALSE; - throw new CredisException('Lost connection to Redis server.', CredisException::CODE_DISCONNECTED); - } - } - $reply = rtrim($reply, CRLF); - #echo "> $name: $reply\n"; - $replyType = substr($reply, 0, 1); - switch ($replyType) { - /* Error reply */ - case '-': - if($this->isMulti || $this->usePipeline) { - $response = FALSE; - } else if ($name == 'evalsha' && substr($reply,0,9) == '-NOSCRIPT') { - $response = NULL; - } else { - throw new CredisException(substr($reply,0,4) == '-ERR' ? substr($reply, 5) : substr($reply,1)); - } - break; - /* Inline reply */ - case '+': - $response = substr($reply, 1); - if($response == 'OK' || $response == 'QUEUED') { - return TRUE; - } - break; - /* Bulk reply */ - case '$': - if ($reply == '$-1') return FALSE; - $size = (int) substr($reply, 1); - $response = stream_get_contents($this->redis, $size + 2); - if( ! $response) { - $this->connected = FALSE; - throw new CredisException('Error reading reply.'); - } - $response = substr($response, 0, $size); - break; - /* Multi-bulk reply */ - case '*': - $count = substr($reply, 1); - if ($count == '-1') return FALSE; - - $response = array(); - for ($i = 0; $i < $count; $i++) { - $response[] = $this->read_reply(); - } - break; - /* Integer reply */ - case ':': - $response = intval(substr($reply, 1)); - break; - default: - throw new CredisException('Invalid response: '.print_r($reply, TRUE)); - break; - } - - // Smooth over differences between phpredis and standalone response - switch($name) - { - case '': // Minor optimization for multi-bulk replies - break; - case 'config': - case 'hgetall': - $keys = $values = array(); - while($response) { - $keys[] = array_shift($response); - $values[] = array_shift($response); - } - $response = count($keys) ? array_combine($keys, $values) : array(); - break; - case 'info': - $lines = explode(CRLF, trim($response,CRLF)); - $response = array(); - foreach($lines as $line) { - if ( ! $line || substr($line, 0, 1) == '#') { - continue; - } - list($key, $value) = explode(':', $line, 2); - $response[$key] = $value; - } - break; - case 'ttl': - if($response === -1) { - $response = FALSE; - } - break; - } - - return $response; - } - - /** - * Build the Redis unified protocol command - * - * @param array $args - * @return string - */ - private static function _prepare_command($args) - { - return sprintf('*%d%s%s%s', count($args), CRLF, implode(array_map(array('self', '_map'), $args), CRLF), CRLF); - } - - private static function _map($arg) - { - return sprintf('$%d%s%s', strlen($arg), CRLF, $arg); - } - -} diff --git a/lib/internal/Magento/Framework/Session/Config.php b/lib/internal/Magento/Framework/Session/Config.php index 4fe892a7e47b6..fafc0e53e8741 100644 --- a/lib/internal/Magento/Framework/Session/Config.php +++ b/lib/internal/Magento/Framework/Session/Config.php @@ -15,8 +15,6 @@ /** * Magento session configuration - * - * @method Config setSaveHandler() */ class Config implements ConfigInterface { @@ -129,21 +127,6 @@ public function __construct( $this->_httpRequest = $request; $this->_scopeType = $scopeType; - /** - * Session handler - * - * Save handler may be set to custom value in deployment config, which will override everything else. - * Otherwise, try to read PHP settings for session.save_handler value. Otherwise, use 'files' as default. - */ - $defaultSaveHandler = $this->getStorageOption('session.save_handler') - ?: SaveHandlerInterface::DEFAULT_HANDLER; - $saveMethod = $deploymentConfig->get( - self::PARAM_SESSION_SAVE_METHOD, - $defaultSaveHandler - ); - $saveMethod = $saveMethod === 'db' ? 'user' : $saveMethod; - $this->setSaveHandler($saveMethod); - /** * Session path */ diff --git a/lib/internal/Magento/Framework/Session/SaveHandler.php b/lib/internal/Magento/Framework/Session/SaveHandler.php index 5fedbac9e32f9..59503167a348f 100644 --- a/lib/internal/Magento/Framework/Session/SaveHandler.php +++ b/lib/internal/Magento/Framework/Session/SaveHandler.php @@ -5,8 +5,10 @@ */ namespace Magento\Framework\Session; -use Magento\Framework\App\DeploymentConfig; +use Magento\Framework\Session\Config; +use Magento\Framework\Session\Config\ConfigInterface; use Magento\Framework\Exception\SessionException; +use Magento\Framework\App\DeploymentConfig; /** * Magento session save handler @@ -20,6 +22,13 @@ class SaveHandler implements SaveHandlerInterface */ protected $saveHandlerAdapter; + /** + * Config + * + * @var ConfigInterface + */ + private $config; + /** * Constructor * @@ -32,11 +41,21 @@ public function __construct( DeploymentConfig $deploymentConfig, $default = self::DEFAULT_HANDLER ) { - $saveMethod = $deploymentConfig->get(\Magento\Framework\Session\Config::PARAM_SESSION_SAVE_METHOD); + /** + * Session handler + * + * Save handler may be set to custom value in deployment config, which will override everything else. + * Otherwise, try to read PHP settings for session.save_handler value. Otherwise, use 'files' as default. + */ + $defaultSaveHandler = ini_get('session.save_handler') ?: SaveHandlerInterface::DEFAULT_HANDLER; + $saveMethod = $deploymentConfig->get(Config::PARAM_SESSION_SAVE_METHOD, $defaultSaveHandler); + $this->setSaveHandler($saveMethod); + try { $connection = $saveHandlerFactory->create($saveMethod); } catch (SessionException $e) { $connection = $saveHandlerFactory->create($default); + $this->setSaveHandler($default); } $this->saveHandlerAdapter = $connection; } @@ -109,4 +128,35 @@ public function gc($maxLifetime) { return $this->saveHandlerAdapter->gc($maxLifetime); } + + /** + * Get config + * + * @return ConfigInterface + * @deprecated + */ + private function getConfig() + { + if (!($this->config instanceof ConfigInterface)) { + return \Magento\Framework\App\ObjectManager::getInstance()->get( + ConfigInterface::class + ); + } + return $this->config; + } + + /** + * Set session.save_handler option + * + * @param string $saveHandler + * @return $this + */ + private function setSaveHandler($saveHandler) + { + if ($saveHandler === 'db' || $saveHandler === 'redis') { + $saveHandler = 'user'; + } + $this->getConfig()->setOption('session.save_handler', $saveHandler); + return $this; + } } diff --git a/lib/internal/Magento/Framework/Session/SaveHandler/Redis.php b/lib/internal/Magento/Framework/Session/SaveHandler/Redis.php new file mode 100644 index 0000000000000..f1142f0fa31ca --- /dev/null +++ b/lib/internal/Magento/Framework/Session/SaveHandler/Redis.php @@ -0,0 +1,51 @@ +filesystem = $filesystem; + try { + parent::__construct($config, $logger); + } catch (ConnectionFailedException $e) { + throw new SessionException(new Phrase($e->getMessage())); + } + } + + /** + * {@inheritdoc} + */ + public function read($sessionId) + { + try { + return parent::read($sessionId); + } catch (ConcurrentConnectionsExceededException $e) { + require $this->filesystem->getDirectoryRead(DirectoryList::PUB)->getAbsolutePath('errors/503.php'); + } + } +} diff --git a/lib/internal/Magento/Framework/Session/SaveHandler/Redis/Config.php b/lib/internal/Magento/Framework/Session/SaveHandler/Redis/Config.php new file mode 100644 index 0000000000000..1a23e2974f568 --- /dev/null +++ b/lib/internal/Magento/Framework/Session/SaveHandler/Redis/Config.php @@ -0,0 +1,291 @@ +deploymentConfig = $deploymentConfig; + $this->appState = $appState; + $this->scopeConfig = $scopeConfig; + } + + /** + * {@inheritdoc} + */ + public function getLogLevel() + { + return $this->deploymentConfig->get(self::PARAM_LOG_LEVEL); + } + + /** + * {@inheritdoc} + */ + public function getHost() + { + return $this->deploymentConfig->get(self::PARAM_HOST); + } + + /** + * {@inheritdoc} + */ + public function getPort() + { + return $this->deploymentConfig->get(self::PARAM_PORT); + } + + /** + * {@inheritdoc} + */ + public function getDatabase() + { + return $this->deploymentConfig->get(self::PARAM_DATABASE); + } + + /** + * {@inheritdoc} + */ + public function getPassword() + { + return $this->deploymentConfig->get(self::PARAM_PASSWORD); + } + + /** + * {@inheritdoc} + */ + public function getTimeout() + { + return $this->deploymentConfig->get(self::PARAM_TIMEOUT); + } + + /** + * {@inheritdoc} + */ + public function getPersistentIdentifier() + { + return $this->deploymentConfig->get(self::PARAM_PERSISTENT_IDENTIFIER); + } + + /** + * {@inheritdoc} + */ + public function getCompressionThreshold() + { + return $this->deploymentConfig->get(self::PARAM_COMPRESSION_THRESHOLD); + } + + /** + * {@inheritdoc} + */ + public function getCompressionLibrary() + { + return $this->deploymentConfig->get(self::PARAM_COMPRESSION_LIBRARY); + } + + /** + * {@inheritdoc} + */ + public function getMaxConcurrency() + { + return $this->deploymentConfig->get(self::PARAM_MAX_CONCURRENCY); + } + + /** + * {@inheritdoc} + */ + public function getMaxLifetime() + { + return self::SESSION_MAX_LIFETIME; + } + + /** + * {@inheritdoc} + */ + public function getMinLifetime() + { + return $this->deploymentConfig->get(self::PARAM_MIN_LIFETIME); + } + + /** + * {@inheritdoc} + */ + public function getDisableLocking() + { + return $this->deploymentConfig->get(self::PARAM_DISABLE_LOCKING); + } + + /** + * {@inheritdoc} + */ + public function getBotLifetime() + { + return $this->deploymentConfig->get(self::PARAM_BOT_LIFETIME); + } + + /** + * {@inheritdoc} + */ + public function getBotFirstLifetime() + { + return $this->deploymentConfig->get(self::PARAM_BOT_FIRST_LIFETIME); + } + + /** + * {@inheritdoc} + */ + public function getFirstLifetime() + { + return $this->deploymentConfig->get(self::PARAM_FIRST_LIFETIME); + } + + /** + * {@inheritdoc} + */ + public function getBreakAfter() + { + return $this->deploymentConfig->get(self::PARAM_BREAK_AFTER . '_' . $this->appState->getAreaCode()); + } + + /** + * {@inheritdoc} + */ + public function getLifetime() + { + if ($this->appState->getAreaCode() == \Magento\Framework\App\Area::AREA_ADMINHTML) { + return (int)$this->scopeConfig->getValue(self::XML_PATH_ADMIN_SESSION_LIFETIME); + } + return (int)$this->scopeConfig->getValue(self::XML_PATH_COOKIE_LIFETIME, StoreScopeInterface::SCOPE_STORE); + } +} diff --git a/lib/internal/Magento/Framework/Session/SaveHandler/Redis/Logger.php b/lib/internal/Magento/Framework/Session/SaveHandler/Redis/Logger.php new file mode 100644 index 0000000000000..993c492e59476 --- /dev/null +++ b/lib/internal/Magento/Framework/Session/SaveHandler/Redis/Logger.php @@ -0,0 +1,93 @@ +logger = $logger; + $this->request = $request; + $this->logLevel = $config->getLogLevel() ?: self::ALERT; + } + + /** + * {@inheritdoc} + */ + public function setLogLevel($level) + { + $this->logLevel = $level; + } + + /** + * {@inheritdoc} + */ + public function log($message, $level) + { + $message .= ' ' . $this->request->getRequestUri(); + if ($this->logLevel >= $level) { + switch ($level) { + case self::EMERGENCY: + $this->logger->emergency($message); + break; + case self::ALERT: + $this->logger->alert($message); + break; + case self::CRITICAL: + $this->logger->critical($message); + break; + case self::ERROR: + $this->logger->error($message); + break; + case self::WARNING: + $this->logger->warning($message); + break; + case self::NOTICE: + $this->logger->notice($message); + break; + case self::INFO: + $this->logger->info($message); + break; + default: + $this->logger->debug($message); + } + } + } + + /** + * {@inheritdoc} + */ + public function logException(\Exception $e) + { + $this->logger->critical($e->getMessage()); + } +} diff --git a/lib/internal/Magento/Framework/Session/Test/Unit/ConfigTest.php b/lib/internal/Magento/Framework/Session/Test/Unit/ConfigTest.php index 5698d1a8752c1..2940db7301bd9 100644 --- a/lib/internal/Magento/Framework/Session/Test/Unit/ConfigTest.php +++ b/lib/internal/Magento/Framework/Session/Test/Unit/ConfigTest.php @@ -15,9 +15,6 @@ class ConfigTest extends \PHPUnit_Framework_TestCase { - /** mock session.save_handler value from deployment config */ - const SESSION_HANDLER_CONFIG = 'files'; - /** * @var \Magento\Framework\TestFramework\Unit\Helper\ObjectManager */ @@ -89,7 +86,6 @@ public function optionsProvider() return [ ['save_path', 'getSavePath', __DIR__], ['name', 'getName', 'FOOBAR'], - ['save_handler', 'getSaveHandler', 'user'], ['gc_probability', 'getGcProbability', 42], ['gc_divisor', 'getGcDivisor', 3], ['gc_maxlifetime', 'getGcMaxlifetime', 180], @@ -135,23 +131,6 @@ public function testNameIsMutable() $this->assertEquals('FOOBAR', $this->config->getName()); } - public function testSaveHandlerFromConfig() - { - $this->getModel($this->validatorMock); - $this->assertSame( - self::SESSION_HANDLER_CONFIG, - $this->config->getSaveHandler(), - var_export($this->config->toArray(), 1) - ); - } - - public function testSaveHandlerIsMutable() - { - $this->getModel($this->validatorMock); - $this->config->setSaveHandler('user'); - $this->assertEquals('user', $this->config->getSaveHandler()); - } - public function testCookieLifetimeIsMutable() { $this->getModel($this->validatorMock); @@ -370,7 +349,6 @@ public function constructorDataProvider() true, true, [ - 'session.save_handler' => 'files', 'session.cache_limiter' => 'files', 'session.cookie_lifetime' => 7200, 'session.cookie_path' => '/', @@ -382,7 +360,6 @@ public function constructorDataProvider() true, false, [ - 'session.save_handler' => 'files', 'session.cache_limiter' => 'files', 'session.cookie_httponly' => false, ], @@ -391,7 +368,6 @@ public function constructorDataProvider() false, true, [ - 'session.save_handler' => 'files', 'session.cache_limiter' => 'files', 'session.cookie_lifetime' => 3600, 'session.cookie_path' => '/', @@ -453,14 +429,10 @@ protected function getModel($validator) $deploymentConfigMock = $this->getMock('Magento\Framework\App\DeploymentConfig', [], [], '', false); $deploymentConfigMock->expects($this->at(0)) - ->method('get') - ->with(Config::PARAM_SESSION_SAVE_METHOD, ini_get('session.save_handler') ?: 'files') - ->willReturn(self::SESSION_HANDLER_CONFIG); - $deploymentConfigMock->expects($this->at(1)) ->method('get') ->with(Config::PARAM_SESSION_SAVE_PATH) ->will($this->returnValue(null)); - $deploymentConfigMock->expects($this->at(2)) + $deploymentConfigMock->expects($this->at(1)) ->method('get') ->with(Config::PARAM_SESSION_CACHE_LIMITER) ->will($this->returnValue('files')); @@ -471,13 +443,12 @@ protected function getModel($validator) 'scopeConfig' => $this->configMock, 'validatorFactory' => $this->validatorFactoryMock, 'scopeType' => \Magento\Store\Model\ScopeInterface::SCOPE_STORE, - 'cacheLimiter' => \Magento\Framework\Session\SaveHandlerInterface::DEFAULT_HANDLER, + 'cacheLimiter' => 'files', 'lifetimePath' => 'test_web/test_cookie/test_cookie_lifetime', 'request' => $this->requestMock, 'filesystem' => $filesystemMock, 'deploymentConfig' => $deploymentConfigMock, ] - ); return $this->config; } diff --git a/lib/internal/Magento/Framework/Session/Test/Unit/SaveHandler/Redis/ConfigTest.php b/lib/internal/Magento/Framework/Session/Test/Unit/SaveHandler/Redis/ConfigTest.php new file mode 100644 index 0000000000000..c66c31d2fcdff --- /dev/null +++ b/lib/internal/Magento/Framework/Session/Test/Unit/SaveHandler/Redis/ConfigTest.php @@ -0,0 +1,249 @@ +deploymentConfigMock = $this->getMock(\Magento\Framework\App\DeploymentConfig::class, [], [], '', false); + $this->appStateMock = $this->getMock(\Magento\Framework\App\State::class, [], [], '', false); + $this->scopeConfigMock = $this->getMock(\Magento\Framework\App\Config::class, [], [], '', false); + + $objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this); + $this->config = $objectManager->getObject( + Config::class, + [ + 'deploymentConfig' => $this->deploymentConfigMock, + 'appState' => $this->appStateMock, + 'scopeConfig' => $this->scopeConfigMock + ] + ); + } + + public function testGetLogLevel() + { + $expected = 2; + $this->deploymentConfigMock->expects($this->once()) + ->method('get') + ->with(Config::PARAM_LOG_LEVEL) + ->willReturn($expected); + $this->assertEquals($this->config->getLogLevel(), $expected); + } + + public function testGetHost() + { + $expected = '127.0.0.1'; + $this->deploymentConfigMock->expects($this->once()) + ->method('get') + ->with(Config::PARAM_HOST) + ->willReturn($expected); + $this->assertEquals($this->config->getHost(), $expected); + } + + public function testGetPort() + { + $expected = 1234; + $this->deploymentConfigMock->expects($this->once()) + ->method('get') + ->with(Config::PARAM_PORT) + ->willReturn($expected); + $this->assertEquals($this->config->getPort(), $expected); + } + + public function testGetDatabase() + { + $expected = 2; + $this->deploymentConfigMock->expects($this->once()) + ->method('get') + ->with(Config::PARAM_DATABASE) + ->willReturn($expected); + $this->assertEquals($this->config->getDatabase(), $expected); + } + + public function testGetPassword() + { + $expected = 'password'; + $this->deploymentConfigMock->expects($this->once()) + ->method('get') + ->with(Config::PARAM_PASSWORD) + ->willReturn($expected); + $this->assertEquals($this->config->getPassword(), $expected); + } + + public function testGetTimeout() + { + $expected = 10; + $this->deploymentConfigMock->expects($this->once()) + ->method('get') + ->with(Config::PARAM_TIMEOUT) + ->willReturn($expected); + $this->assertEquals($this->config->getTimeout(), $expected); + } + + public function testGetPersistentIdentifier() + { + $expected = 'sess01'; + $this->deploymentConfigMock->expects($this->once()) + ->method('get') + ->with(Config::PARAM_PERSISTENT_IDENTIFIER) + ->willReturn($expected); + $this->assertEquals($this->config->getPersistentIdentifier(), $expected); + } + + public function testGetCompressionThreshold() + { + $expected = 2; + $this->deploymentConfigMock->expects($this->once()) + ->method('get') + ->with(Config::PARAM_COMPRESSION_THRESHOLD) + ->willReturn($expected); + $this->assertEquals($this->config->getCompressionThreshold(), $expected); + } + + public function testGetCompressionLibrary() + { + $expected = 'gzip'; + $this->deploymentConfigMock->expects($this->once()) + ->method('get') + ->with(Config::PARAM_COMPRESSION_LIBRARY) + ->willReturn($expected); + $this->assertEquals($this->config->getCompressionLibrary(), $expected); + } + + public function testGetMaxConcurrency() + { + $expected = 6; + $this->deploymentConfigMock->expects($this->once()) + ->method('get') + ->with(Config::PARAM_MAX_CONCURRENCY) + ->willReturn($expected); + $this->assertEquals($this->config->getMaxConcurrency(), $expected); + } + + public function testGetMaxLifetime() + { + $this->assertEquals($this->config->getMaxLifetime(), Config::SESSION_MAX_LIFETIME); + } + + public function testGetMinLifetime() + { + $expected = 30; + $this->deploymentConfigMock->expects($this->once()) + ->method('get') + ->with(Config::PARAM_MIN_LIFETIME) + ->willReturn($expected); + $this->assertEquals($this->config->getMinLifetime(), $expected); + } + + public function testGetDisableLocking() + { + $expected = false; + $this->deploymentConfigMock->expects($this->once()) + ->method('get') + ->with(Config::PARAM_DISABLE_LOCKING) + ->willReturn($expected); + $this->assertEquals($this->config->getDisableLocking(), $expected); + } + + public function testGetBotLifetime() + { + $expected = 30; + $this->deploymentConfigMock->expects($this->once()) + ->method('get') + ->with(Config::PARAM_BOT_LIFETIME) + ->willReturn($expected); + $this->assertEquals($this->config->getBotLifetime(), $expected); + } + + public function testGetBotFirstLifetime() + { + $expected = 30; + $this->deploymentConfigMock->expects($this->once()) + ->method('get') + ->with(Config::PARAM_BOT_FIRST_LIFETIME) + ->willReturn($expected); + $this->assertEquals($this->config->getBotFirstLifetime(), $expected); + } + + public function testGetFirstLifetime() + { + $expected = 30; + $this->deploymentConfigMock->expects($this->once()) + ->method('get') + ->with(Config::PARAM_FIRST_LIFETIME) + ->willReturn($expected); + $this->assertEquals($this->config->getFirstLifetime(), $expected); + } + + public function testBreakAfter() + { + $areaCode = 'frontend'; + $breakAfter = 5; + $this->deploymentConfigMock->expects($this->once()) + ->method('get') + ->with(Config::PARAM_BREAK_AFTER . '_' . $areaCode) + ->willReturn($breakAfter); + $this->appStateMock->expects($this->once()) + ->method('getAreaCode') + ->willReturn($areaCode); + $this->assertEquals($this->config->getBreakAfter(), $breakAfter); + } + + public function testGetLifetimeAdmin() + { + $areaCode = 'adminhtml'; + $expectedLifetime = 123; + $this->appStateMock->expects($this->once()) + ->method('getAreaCode') + ->willReturn($areaCode); + $this->scopeConfigMock->expects($this->once()) + ->method('getValue') + ->with(Config::XML_PATH_ADMIN_SESSION_LIFETIME) + ->willReturn($expectedLifetime); + $this->assertEquals($this->config->getLifetime(), $expectedLifetime); + } + + public function testGetLifetimeFrontend() + { + $areaCode = 'frontend'; + $expectedLifetime = 234; + $this->appStateMock->expects($this->once()) + ->method('getAreaCode') + ->willReturn($areaCode); + $this->scopeConfigMock->expects($this->once()) + ->method('getValue') + ->with( + Config::XML_PATH_COOKIE_LIFETIME, + ScopeInterface::SCOPE_STORE + ) + ->willReturn($expectedLifetime); + $this->assertEquals($this->config->getLifetime(), $expectedLifetime); + } +} diff --git a/lib/internal/Magento/Framework/Session/Test/Unit/SaveHandler/Redis/LoggerTest.php b/lib/internal/Magento/Framework/Session/Test/Unit/SaveHandler/Redis/LoggerTest.php new file mode 100644 index 0000000000000..1badb26afd7a7 --- /dev/null +++ b/lib/internal/Magento/Framework/Session/Test/Unit/SaveHandler/Redis/LoggerTest.php @@ -0,0 +1,95 @@ +config = $this->getMock('Cm\RedisSession\Handler\ConfigInterface', [], [], '', false); + $this->config->expects($this->once()) + ->method('getLogLevel') + ->willReturn(LoggerInterface::DEBUG); + $this->psrLogger = $this->getMock('Psr\Log\LoggerInterface', [], [], '', false); + $this->request = $this->getMock('Magento\Framework\App\Request\Http', [], [], '', false); + //$this->logger = new Logger($this->config, $this->psrLogger, $this->request); + $objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this); + $this->logger = $objectManager->getObject( + 'Magento\Framework\Session\SaveHandler\Redis\Logger', + [ + 'config' => $this->config, + 'logger' => $this->psrLogger, + 'request' => $this->request + ] + ); + } + + /** + * @dataProvider logDataProvider + */ + public function testLog($logLevel, $method) + { + $message = 'Error message'; + $this->request->expects($this->once()) + ->method('getRequestUri') + ->willReturn($this->requestUri); + $this->psrLogger->expects($this->once()) + ->method($method) + ->with($message . ' ' . $this->requestUri); + $this->logger->log($message, $logLevel); + } + + public function logDataProvider() + { + return [ + [LoggerInterface::EMERGENCY, 'emergency'], + [LoggerInterface::ALERT, 'alert'], + [LoggerInterface::CRITICAL, 'critical'], + [LoggerInterface::ERROR, 'error'], + [LoggerInterface::WARNING, 'warning'], + [LoggerInterface::NOTICE, 'notice'], + [LoggerInterface::INFO, 'info'], + [LoggerInterface::DEBUG, 'debug'], + ]; + } + + public function testLogException() + { + $exception = new \Exception('Error message'); + $this->psrLogger->expects($this->once()) + ->method('critical') + ->with($exception->getMessage()); + $this->logger->logException($exception); + } +} diff --git a/setup/src/Magento/Setup/Test/Unit/Module/ConfigOptionsListTest.php b/setup/src/Magento/Setup/Test/Unit/Model/ConfigOptionsListTest.php similarity index 99% rename from setup/src/Magento/Setup/Test/Unit/Module/ConfigOptionsListTest.php rename to setup/src/Magento/Setup/Test/Unit/Model/ConfigOptionsListTest.php index 1cf524bb961b6..b961e3f07b9de 100644 --- a/setup/src/Magento/Setup/Test/Unit/Module/ConfigOptionsListTest.php +++ b/setup/src/Magento/Setup/Test/Unit/Model/ConfigOptionsListTest.php @@ -4,7 +4,7 @@ * See COPYING.txt for license details. */ -namespace Magento\Setup\Test\Unit\Module; +namespace Magento\Setup\Test\Unit\Model; use Magento\Setup\Model\ConfigGenerator; use Magento\Setup\Model\ConfigOptionsList; @@ -149,7 +149,7 @@ private function prepareValidationMocks() $this->dbValidator->expects($this->once())->method('checkDatabaseTablePrefix')->willReturn($configDataMock); $this->dbValidator->expects($this->once())->method('checkDatabaseConnection')->willReturn($configDataMock); } - + /** * @param string $hosts * @param bool $expectedError From bf11e2e87ebe194947b81a90bde4ba50db9687c4 Mon Sep 17 00:00:00 2001 From: David Alger Date: Mon, 8 Feb 2016 15:48:45 -0600 Subject: [PATCH 2/5] Update unit test to properly test corrected implementation of getUris --- .../Test/Unit/Model/Cache/ServerTest.php | 35 ++++++++++--------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/app/code/Magento/PageCache/Test/Unit/Model/Cache/ServerTest.php b/app/code/Magento/PageCache/Test/Unit/Model/Cache/ServerTest.php index 6d2517f2cbab0..d0356efe262af 100644 --- a/app/code/Magento/PageCache/Test/Unit/Model/Cache/ServerTest.php +++ b/app/code/Magento/PageCache/Test/Unit/Model/Cache/ServerTest.php @@ -5,11 +5,13 @@ */ namespace Magento\PageCache\Test\Unit\Model\Cache; +use \Magento\Framework\TestFramework\Unit\Helper\ObjectManager; +use \Magento\PageCache\Model\Cache\Server; use \Zend\Uri\UriFactory; class ServerTest extends \PHPUnit_Framework_TestCase { - /** @var \Magento\PageCache\Model\Cache\Server */ + /** @var Server */ protected $model; /** @var \PHPUnit_Framework_MockObject_MockObject | \Magento\Framework\App\DeploymentConfig */ @@ -30,7 +32,7 @@ public function setUp() ->disableOriginalConstructor() ->getMock(); - $objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this); + $objectManager = new ObjectManager($this); $this->model = $objectManager->getObject( 'Magento\PageCache\Model\Cache\Server', [ @@ -56,12 +58,9 @@ public function testGetUris( $url, $hostConfig = null ) { - $this->configMock->expects($this->once()) - ->method('get') - ->willReturn($hostConfig); - $this->requestMock->expects($this->exactly($getHttpHostCallCtr)) - ->method('getHttpHost') - ->willReturn($httpHost); + $this->configMock->expects($this->once())->method('get')->willReturn($hostConfig); + $this->requestMock->expects($this->exactly($getHttpHostCallCtr))->method('getHttpHost')->willReturn($httpHost); + $this->urlBuilderMock->expects($this->exactly($getUrlCallCtr)) ->method('getUrl') ->with('*', ['_nosid' => true]) @@ -70,30 +69,32 @@ public function testGetUris( $uris = []; if (null === $hostConfig) { if (!empty($httpHost)) { - $uris[] = UriFactory::factory('')->setHost($httpHost) - ->setPort(\Magento\PageCache\Model\Cache\Server::DEFAULT_PORT) - ->setScheme('http'); + $uris[] = UriFactory::factory('')->setHost($httpHost)->setPort(Server::DEFAULT_PORT); } if (!empty($url)) { $uris[] = UriFactory::factory($url); } } else { foreach ($hostConfig as $host) { - $port = isset($host['port']) ? $host['port'] : \Magento\PageCache\Model\Cache\Server::DEFAULT_PORT; - $uris[] = UriFactory::factory('')->setHost($host['host']) - ->setPort($port) - ->setScheme('http'); + $port = isset($host['port']) ? $host['port'] : Server::DEFAULT_PORT; + $uris[] = UriFactory::factory('')->setHost($host['host'])->setPort($port); } } + foreach ($uris as $key => $value) { + $uris[$key]->setScheme('http') + ->setPath('/') + ->setQuery(null); + } + $this->assertEquals($uris, $this->model->getUris()); } public function getUrisDataProvider() { return [ - 'http host' => [1, '127.0.0.1', 0, '',], - 'url' => [1, '', 1, 'http://host',], + 'http host' => [2, '127.0.0.1', 0, ''], + 'url' => [1, '', 1, 'http://host'], 'config' => [ 0, '', From 4a3e7d32fb04859a1495cf0514c73409bf90e1ed Mon Sep 17 00:00:00 2001 From: David Alger Date: Mon, 8 Feb 2016 15:09:34 -0600 Subject: [PATCH 3/5] Resolves #2705: add explicit set of path component to generated request uris --- .../Magento/PageCache/Model/Cache/Server.php | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/app/code/Magento/PageCache/Model/Cache/Server.php b/app/code/Magento/PageCache/Model/Cache/Server.php index 9ac2280512a94..ee08b71069e5b 100644 --- a/app/code/Magento/PageCache/Model/Cache/Server.php +++ b/app/code/Magento/PageCache/Model/Cache/Server.php @@ -5,16 +5,17 @@ */ namespace Magento\PageCache\Model\Cache; -use Zend\Uri\Uri; +use Magento\Framework\UrlInterface; use Magento\Framework\App\DeploymentConfig; use Magento\Framework\Config\ConfigOptionsListConstants; use Magento\Framework\App\RequestInterface; +use Zend\Uri\Uri; use Zend\Uri\UriFactory; class Server { /** - * @var \Magento\Framework\UrlInterface + * @var UrlInterface */ protected $urlBuilder; @@ -33,12 +34,12 @@ class Server /** * Constructor * - * @param \Magento\Framework\UrlInterface $urlBuilder + * @param UrlInterface $urlBuilder * @param DeploymentConfig $config * @param RequestInterface $request */ public function __construct( - \Magento\Framework\UrlInterface $urlBuilder, + UrlInterface $urlBuilder, DeploymentConfig $config, RequestInterface $request ) { @@ -56,21 +57,25 @@ public function getUris() { $servers = []; $configuredHosts = $this->config->get(ConfigOptionsListConstants::CONFIG_PATH_CACHE_HOSTS); - if (null == $configuredHosts) { - $httpHost = $this->request->getHttpHost(); - $servers[] = $httpHost ? - UriFactory::factory('')->setHost($httpHost)->setPort(self::DEFAULT_PORT)->setScheme('http') : - UriFactory::factory($this->urlBuilder->getUrl('*', ['_nosid' => true])) // Don't use SID in building URL - ->setScheme('http') - ->setPath(null) - ->setQuery(null); - } else { + if (is_array($configuredHosts)) { foreach ($configuredHosts as $host) { - $servers[] = UriFactory::factory('')->setHost($host['host']) + $servers[] = UriFactory::factory('') + ->setHost($host['host']) ->setPort(isset($host['port']) ? $host['port'] : self::DEFAULT_PORT) - ->setScheme('http'); + ; } + } elseif ($this->request->getHttpHost()) { + $servers[] = UriFactory::factory('')->setHost($this->request->getHttpHost())->setPort(self::DEFAULT_PORT); + } else { + $servers[] = UriFactory::factory($this->urlBuilder->getUrl('*', ['_nosid' => true])); + } + + foreach ($servers as $key => $value) { + $servers[$key]->setScheme('http') + ->setPath('/') + ->setQuery(null) + ; } return $servers; } From 1ee87034c5dbfb9767c00b4bb27dc9718dd27363 Mon Sep 17 00:00:00 2001 From: Igor Melnikov Date: Fri, 29 Apr 2016 12:03:39 -0500 Subject: [PATCH 4/5] MAGETWO-51847: 400 Bad request error when Magento clears Varnish cache on GoDaddy Fixing static test --- app/code/Magento/PageCache/Model/Cache/Server.php | 12 ++++++------ .../PageCache/Test/Unit/Model/Cache/ServerTest.php | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/code/Magento/PageCache/Model/Cache/Server.php b/app/code/Magento/PageCache/Model/Cache/Server.php index ee08b71069e5b..bd19db61cff13 100644 --- a/app/code/Magento/PageCache/Model/Cache/Server.php +++ b/app/code/Magento/PageCache/Model/Cache/Server.php @@ -62,20 +62,20 @@ public function getUris() foreach ($configuredHosts as $host) { $servers[] = UriFactory::factory('') ->setHost($host['host']) - ->setPort(isset($host['port']) ? $host['port'] : self::DEFAULT_PORT) - ; + ->setPort(isset($host['port']) ? $host['port'] : self::DEFAULT_PORT); } } elseif ($this->request->getHttpHost()) { - $servers[] = UriFactory::factory('')->setHost($this->request->getHttpHost())->setPort(self::DEFAULT_PORT); + $servers[] = UriFactory::factory('') + ->setHost($this->request->getHttpHost()) + ->setPort(self::DEFAULT_PORT); } else { $servers[] = UriFactory::factory($this->urlBuilder->getUrl('*', ['_nosid' => true])); } - foreach ($servers as $key => $value) { + foreach (array_keys($servers) as $key) { $servers[$key]->setScheme('http') ->setPath('/') - ->setQuery(null) - ; + ->setQuery(null); } return $servers; } diff --git a/app/code/Magento/PageCache/Test/Unit/Model/Cache/ServerTest.php b/app/code/Magento/PageCache/Test/Unit/Model/Cache/ServerTest.php index d0356efe262af..4e947b8fc778b 100644 --- a/app/code/Magento/PageCache/Test/Unit/Model/Cache/ServerTest.php +++ b/app/code/Magento/PageCache/Test/Unit/Model/Cache/ServerTest.php @@ -81,7 +81,7 @@ public function testGetUris( } } - foreach ($uris as $key => $value) { + foreach (array_keys($uris) as $key) { $uris[$key]->setScheme('http') ->setPath('/') ->setQuery(null); From 28caa4fafdbef94b8f8f4b8798d74d202583072a Mon Sep 17 00:00:00 2001 From: Dale Sikkema Date: Tue, 3 May 2016 11:20:01 -0500 Subject: [PATCH 5/5] MAGETWO-52322: port file permissions fixes to 2.0 branch --- .htaccess | 6 +++ .htaccess.sample | 6 +++ app/bootstrap.php | 5 ++- .../Magento/Backup/Model/Fs/Collection.php | 1 - app/code/Magento/Captcha/Helper/Data.php | 2 - .../Attribute/Backend/AbstractMedia.php | 2 - app/code/Magento/Deploy/Model/Filesystem.php | 16 +++---- app/code/Magento/Deploy/Model/Mode.php | 1 - .../Util/Generate/Factory/AbstractFactory.php | 4 +- .../Magento/TestFramework/Application.php | 2 +- .../Cms/Model/Wysiwyg/Images/StorageTest.php | 2 +- .../Filesystem/Directory/WriteTest.php | 44 +++++++++---------- .../Framework/Filesystem/Driver/FileTest.php | 2 +- .../Test/Integrity/Di/CompilerTest.php | 5 +-- .../Magento/Test/Js/Exemplar/JsHintTest.php | 2 +- .../Magento/Test/Js/LiveCodeTest.php | 2 +- .../Magento/Test/Php/LiveCodeTest.php | 2 +- .../Framework/App/Cache/Frontend/Factory.php | 2 +- .../Magento/Framework/Archive/Helper/File.php | 4 +- .../Magento/Framework/Archive/Tar.php | 2 +- .../Magento/Framework/Backup/Filesystem.php | 1 - .../Backup/Filesystem/Rollback/Ftp.php | 2 +- .../Magento/Framework/Code/Generator/Io.php | 2 +- .../Magento/Framework/File/Uploader.php | 7 +-- .../Framework/Filesystem/Directory/Write.php | 2 +- .../Framework/Filesystem/Driver/File.php | 4 +- .../Framework/Filesystem/DriverInterface.php | 6 ++- .../Magento/Framework/Filesystem/Io/File.php | 4 +- .../Magento/Framework/Filesystem/Io/Ftp.php | 2 +- .../Framework/Filesystem/Io/IoInterface.php | 2 +- .../Magento/Framework/Filesystem/Io/Sftp.php | 2 +- .../Magento/Framework/Logger/Handler/Base.php | 2 +- .../Framework/Setup/BackupRollback.php | 4 +- .../Setup/Test/Unit/BackupRollbackTest.php | 4 +- .../Magento/Framework/System/Dirs.php | 2 +- lib/internal/Magento/Framework/System/Ftp.php | 4 +- pub/.htaccess | 8 +++- pub/errors/processor.php | 3 +- .../Command/DiCompileMultiTenantCommand.php | 4 +- .../Setup/Model/MarketplaceManager.php | 3 -- .../Di/Compiler/Config/Writer/Filesystem.php | 2 +- .../I18n/Pack/Writer/File/AbstractFile.php | 2 +- 42 files changed, 99 insertions(+), 85 deletions(-) diff --git a/.htaccess b/.htaccess index 50c06d4e3aef9..60cfcc41c9fe8 100644 --- a/.htaccess +++ b/.htaccess @@ -4,6 +4,12 @@ # SetEnv MAGE_MODE developer +############################################ +## overrides default umask value to allow using different +## file permissions + +# SetEnv MAGE_UMASK 022 + ############################################ ## uncomment these lines for CGI mode ## make sure to specify the correct cgi php binary file name diff --git a/.htaccess.sample b/.htaccess.sample index 891dad19d642b..186c51ddf4211 100644 --- a/.htaccess.sample +++ b/.htaccess.sample @@ -3,6 +3,12 @@ # SetEnv MAGE_MODE developer +############################################ +## overrides default umask value to allow using different +## file permissions + +# SetEnv MAGE_UMASK 022 + ############################################ ## uncomment these lines for CGI mode ## make sure to specify the correct cgi php binary file name diff --git a/app/bootstrap.php b/app/bootstrap.php index 63fe76b456297..ae15b1ad4c0c2 100644 --- a/app/bootstrap.php +++ b/app/bootstrap.php @@ -9,7 +9,10 @@ */ error_reporting(E_ALL); #ini_set('display_errors', 1); -umask(0); + +/* Custom umask value may be provided in MAGE_UMASK environment variable */ +$mask = isset($_SERVER['MAGE_UMASK']) ? octdec($_SERVER['MAGE_UMASK']) : 002; +umask($mask); /* PHP version validation */ if (version_compare(phpversion(), '5.5.0', '<') === true) { diff --git a/app/code/Magento/Backup/Model/Fs/Collection.php b/app/code/Magento/Backup/Model/Fs/Collection.php index ffe89eeb6dd29..888a072a57d66 100644 --- a/app/code/Magento/Backup/Model/Fs/Collection.php +++ b/app/code/Magento/Backup/Model/Fs/Collection.php @@ -91,7 +91,6 @@ protected function _hideBackupsForApache() $filename = '.htaccess'; if (!$this->_varDirectory->isFile($filename)) { $this->_varDirectory->writeFile($filename, 'deny from all'); - $this->_varDirectory->changePermissions($filename, 0640); } } diff --git a/app/code/Magento/Captcha/Helper/Data.php b/app/code/Magento/Captcha/Helper/Data.php index 05d33045eaafc..8892ddc1ed375 100644 --- a/app/code/Magento/Captcha/Helper/Data.php +++ b/app/code/Magento/Captcha/Helper/Data.php @@ -150,8 +150,6 @@ public function getImgDir($website = null) $mediaDir = $this->_filesystem->getDirectoryWrite(DirectoryList::MEDIA); $captchaDir = '/captcha/' . $this->_getWebsiteCode($website); $mediaDir->create($captchaDir); - $mediaDir->changePermissions($captchaDir, DriverInterface::WRITEABLE_DIRECTORY_MODE); - return $mediaDir->getAbsolutePath($captchaDir) . '/'; } diff --git a/app/code/Magento/Catalog/Model/Product/Attribute/Backend/AbstractMedia.php b/app/code/Magento/Catalog/Model/Product/Attribute/Backend/AbstractMedia.php index 21585e45560aa..24b10d12310a8 100644 --- a/app/code/Magento/Catalog/Model/Product/Attribute/Backend/AbstractMedia.php +++ b/app/code/Magento/Catalog/Model/Product/Attribute/Backend/AbstractMedia.php @@ -170,9 +170,7 @@ public function addImage( $storageHelper->saveFile($this->mediaConfig->getTmpMediaShortUrl($fileName)); } else { $this->mediaDirectory->copyFile($file, $destinationFile); - $storageHelper->saveFile($this->mediaConfig->getTmpMediaShortUrl($fileName)); - $this->mediaDirectory->changePermissions($destinationFile, DriverInterface::WRITEABLE_FILE_MODE); } } catch (\Exception $e) { throw new LocalizedException(__('We couldn\'t move this file: %1.', $e->getMessage())); diff --git a/app/code/Magento/Deploy/Model/Filesystem.php b/app/code/Magento/Deploy/Model/Filesystem.php index f412df23461e2..4d8435ec46075 100644 --- a/app/code/Magento/Deploy/Model/Filesystem.php +++ b/app/code/Magento/Deploy/Model/Filesystem.php @@ -18,11 +18,15 @@ class Filesystem { /** * File access permissions + * + * @deprecated */ const PERMISSIONS_FILE = 0640; /** * Directory access permissions + * + * @deprecated */ const PERMISSIONS_DIR = 0750; @@ -107,19 +111,11 @@ public function regenerateStatic( DirectoryList::TMP_MATERIALIZATION_DIR ] ); - $this->changePermissions( - [ - DirectoryList::STATIC_VIEW - ], - self::PERMISSIONS_DIR, - self::PERMISSIONS_DIR - ); // Trigger static assets compilation and deployment $this->deployStaticContent($output); // Trigger code generation $this->compile($output); - $this->lockStaticResources(); } /** @@ -215,6 +211,8 @@ public function cleanupFilesystem($directoryCodeList) * @param int $dirPermissions * @param int $filePermissions * @return void + * + * @deprecated */ protected function changePermissions($directoryCodeList, $dirPermissions, $filePermissions) { @@ -233,6 +231,8 @@ protected function changePermissions($directoryCodeList, $dirPermissions, $fileP * Chenge permissions on static resources * * @return void + * + * @deprecated */ public function lockStaticResources() { diff --git a/app/code/Magento/Deploy/Model/Mode.php b/app/code/Magento/Deploy/Model/Mode.php index bcfe9bce215f1..788fb897a8810 100644 --- a/app/code/Magento/Deploy/Model/Mode.php +++ b/app/code/Magento/Deploy/Model/Mode.php @@ -92,7 +92,6 @@ public function enableProductionMode() */ public function enableProductionModeMinimal() { - $this->filesystem->lockStaticResources(); $this->setStoreMode(State::MODE_PRODUCTION); } diff --git a/dev/tests/functional/lib/Magento/Mtf/Util/Generate/Factory/AbstractFactory.php b/dev/tests/functional/lib/Magento/Mtf/Util/Generate/Factory/AbstractFactory.php index 8d6443846d98b..d8a0e9c8d2229 100644 --- a/dev/tests/functional/lib/Magento/Mtf/Util/Generate/Factory/AbstractFactory.php +++ b/dev/tests/functional/lib/Magento/Mtf/Util/Generate/Factory/AbstractFactory.php @@ -105,7 +105,7 @@ protected function endFactory($type) * @return bool * @throws \Exception */ - protected function checkAndCreateFolder($folder, $mode = DriverInterface::WRITEABLE_DIRECTORY_MODE) + protected function checkAndCreateFolder($folder, $mode = 0777) { if (is_dir($folder)) { return true; @@ -127,7 +127,7 @@ protected function checkAndCreateFolder($folder, $mode = DriverInterface::WRITEA * @param bool $recursive * @return bool */ - protected function mkDir($dir, $mode = 0770, $recursive = true) + protected function mkDir($dir, $mode = 0777, $recursive = true) { return @mkdir($dir, $mode, $recursive); } diff --git a/dev/tests/integration/framework/Magento/TestFramework/Application.php b/dev/tests/integration/framework/Magento/TestFramework/Application.php index 0747b609af45b..9762b233b9205 100644 --- a/dev/tests/integration/framework/Magento/TestFramework/Application.php +++ b/dev/tests/integration/framework/Magento/TestFramework/Application.php @@ -573,7 +573,7 @@ protected function _ensureDirExists($dir) { if (!file_exists($dir)) { $old = umask(0); - mkdir($dir, DriverInterface::WRITEABLE_DIRECTORY_MODE); + mkdir($dir); umask($old); } elseif (!is_dir($dir)) { throw new \Magento\Framework\Exception\LocalizedException(__("'%1' is not a directory.", $dir)); diff --git a/dev/tests/integration/testsuite/Magento/Cms/Model/Wysiwyg/Images/StorageTest.php b/dev/tests/integration/testsuite/Magento/Cms/Model/Wysiwyg/Images/StorageTest.php index e367ed3441d04..a3baf43c54949 100644 --- a/dev/tests/integration/testsuite/Magento/Cms/Model/Wysiwyg/Images/StorageTest.php +++ b/dev/tests/integration/testsuite/Magento/Cms/Model/Wysiwyg/Images/StorageTest.php @@ -25,7 +25,7 @@ public static function setUpBeforeClass() 'Magento\Cms\Helper\Wysiwyg\Images' )->getCurrentPath() . 'MagentoCmsModelWysiwygImagesStorageTest'; if (!file_exists(self::$_baseDir)) { - mkdir(self::$_baseDir, 0770); + mkdir(self::$_baseDir); } touch(self::$_baseDir . '/1.swf'); } diff --git a/dev/tests/integration/testsuite/Magento/Framework/Filesystem/Directory/WriteTest.php b/dev/tests/integration/testsuite/Magento/Framework/Filesystem/Directory/WriteTest.php index 03f627d480f8a..ea3348b51ceda 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/Filesystem/Directory/WriteTest.php +++ b/dev/tests/integration/testsuite/Magento/Framework/Filesystem/Directory/WriteTest.php @@ -28,7 +28,7 @@ class WriteTest extends \PHPUnit_Framework_TestCase */ public function testInstance() { - $dir = $this->getDirectoryInstance('newDir1', 0770); + $dir = $this->getDirectoryInstance('newDir1', 0777); $this->assertTrue($dir instanceof ReadInterface); $this->assertTrue($dir instanceof WriteInterface); } @@ -56,10 +56,10 @@ public function testCreate($basePath, $permissions, $path) public function createProvider() { return [ - ['newDir1', 0770, "newDir1"], - ['newDir1', 0770, "root_dir1/subdir1/subdir2"], - ['newDir2', 0750, "root_dir2/subdir"], - ['newDir1', 0770, "."] + ['newDir1', 0777, "newDir1"], + ['newDir1', 0777, "root_dir1/subdir1/subdir2"], + ['newDir2', 0777, "root_dir2/subdir"], + ['newDir1', 0777, "."] ]; } @@ -71,7 +71,7 @@ public function createProvider() */ public function testDelete($path) { - $directory = $this->getDirectoryInstance('newDir', 0770); + $directory = $this->getDirectoryInstance('newDir', 0777); $directory->create($path); $this->assertTrue($directory->isExist($path)); $directory->delete($path); @@ -116,7 +116,7 @@ public function testRename($basePath, $permissions, $name, $newName) */ public function renameProvider() { - return [['newDir1', 0770, 'first_name.txt', 'second_name.txt']]; + return [['newDir1', 0777, 'first_name.txt', 'second_name.txt']]; } /** @@ -150,7 +150,7 @@ public function testRenameTargetDir($firstDir, $secondDir, $permission, $name, $ */ public function renameTargetDirProvider() { - return [['dir1', 'dir2', 0770, 'first_name.txt', 'second_name.txt']]; + return [['dir1', 'dir2', 0777, 'first_name.txt', 'second_name.txt']]; } /** @@ -180,8 +180,8 @@ public function testCopy($basePath, $permissions, $name, $newName) public function copyProvider() { return [ - ['newDir1', 0770, 'first_name.txt', 'second_name.txt'], - ['newDir1', 0770, 'subdir/first_name.txt', 'subdir/second_name.txt'] + ['newDir1', 0777, 'first_name.txt', 'second_name.txt'], + ['newDir1', 0777, 'subdir/first_name.txt', 'subdir/second_name.txt'] ]; } @@ -216,8 +216,8 @@ public function testCopyTargetDir($firstDir, $secondDir, $permission, $name, $ne public function copyTargetDirProvider() { return [ - ['dir1', 'dir2', 0770, 'first_name.txt', 'second_name.txt'], - ['dir1', 'dir2', 0770, 'subdir/first_name.txt', 'subdir/second_name.txt'] + ['dir1', 'dir2', 0777, 'first_name.txt', 'second_name.txt'], + ['dir1', 'dir2', 0777, 'subdir/first_name.txt', 'subdir/second_name.txt'] ]; } @@ -226,9 +226,9 @@ public function copyTargetDirProvider() */ public function testChangePermissions() { - $directory = $this->getDirectoryInstance('newDir1', 0770); + $directory = $this->getDirectoryInstance('newDir1', 0777); $directory->create('test_directory'); - $this->assertTrue($directory->changePermissions('test_directory', 0640)); + $this->assertTrue($directory->changePermissions('test_directory', 0644)); } /** @@ -241,7 +241,7 @@ public function testChangePermissionsRecursively() $directory->create('test_directory/subdirectory'); $directory->writeFile('test_directory/subdirectory/test_file.txt', 'Test Content'); - $this->assertTrue($directory->changePermissionsRecursively('test_directory', 0750, 0640)); + $this->assertTrue($directory->changePermissionsRecursively('test_directory', 0777, 0644)); } /** @@ -269,8 +269,8 @@ public function testTouch($basePath, $permissions, $path, $time) public function touchProvider() { return [ - ['test_directory', 0770, 'touch_file.txt', time() - 3600], - ['test_directory', 0770, 'subdirectory/touch_file.txt', time() - 3600] + ['test_directory', 0777, 'touch_file.txt', time() - 3600], + ['test_directory', 0777, 'subdirectory/touch_file.txt', time() - 3600] ]; } @@ -279,7 +279,7 @@ public function touchProvider() */ public function testIsWritable() { - $directory = $this->getDirectoryInstance('newDir1', 0770); + $directory = $this->getDirectoryInstance('newDir1', 0777); $directory->create('bar'); $this->assertFalse($directory->isWritable('not_existing_dir')); $this->assertTrue($directory->isWritable('bar')); @@ -310,8 +310,8 @@ public function testOpenFile($basePath, $permissions, $path, $mode) public function openFileProvider() { return [ - ['newDir1', 0770, 'newFile.txt', 'w+'], - ['newDir1', 0770, 'subdirectory/newFile.txt', 'w+'] + ['newDir1', 0777, 'newFile.txt', 'w+'], + ['newDir1', 0777, 'subdirectory/newFile.txt', 'w+'] ]; } @@ -325,7 +325,7 @@ public function openFileProvider() */ public function testWriteFile($path, $content, $extraContent) { - $directory = $this->getDirectoryInstance('writeFileDir', 0770); + $directory = $this->getDirectoryInstance('writeFileDir', 0777); $directory->writeFile($path, $content); $this->assertEquals($content, $directory->readFile($path)); $directory->writeFile($path, $extraContent); @@ -342,7 +342,7 @@ public function testWriteFile($path, $content, $extraContent) */ public function testWriteFileAppend($path, $content, $extraContent) { - $directory = $this->getDirectoryInstance('writeFileDir', 0770); + $directory = $this->getDirectoryInstance('writeFileDir', 0777); $directory->writeFile($path, $content, 'a+'); $this->assertEquals($content, $directory->readFile($path)); $directory->writeFile($path, $extraContent, 'a+'); diff --git a/dev/tests/integration/testsuite/Magento/Framework/Filesystem/Driver/FileTest.php b/dev/tests/integration/testsuite/Magento/Framework/Filesystem/Driver/FileTest.php index eaac14f38f26a..0bdcefb3d4883 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/Filesystem/Driver/FileTest.php +++ b/dev/tests/integration/testsuite/Magento/Framework/Filesystem/Driver/FileTest.php @@ -75,7 +75,7 @@ public function testCreateDirectory() if (is_dir($generatedPath)) { $this->assertTrue($this->driver->deleteDirectory($generatedPathBase)); } - $this->assertTrue($this->driver->createDirectory($generatedPath, '755')); + $this->assertTrue($this->driver->createDirectory($generatedPath)); $this->assertTrue(is_dir($generatedPath)); } } diff --git a/dev/tests/static/testsuite/Magento/Test/Integrity/Di/CompilerTest.php b/dev/tests/static/testsuite/Magento/Test/Integrity/Di/CompilerTest.php index e2f0f208843ce..36d0a8b2194b9 100644 --- a/dev/tests/static/testsuite/Magento/Test/Integrity/Di/CompilerTest.php +++ b/dev/tests/static/testsuite/Magento/Test/Integrity/Di/CompilerTest.php @@ -69,15 +69,14 @@ protected function setUp() $basePath = Files::init()->getPathToSource(); $basePath = str_replace('\\', '/', $basePath); - $this->_tmpDir = realpath(__DIR__) . '/tmp'; $this->_generationDir = $this->_tmpDir . '/generation'; if (!file_exists($this->_generationDir)) { - mkdir($this->_generationDir, 0770, true); + mkdir($this->_generationDir, 0777, true); } $this->_compilationDir = $this->_tmpDir . '/di'; if (!file_exists($this->_compilationDir)) { - mkdir($this->_compilationDir, 0770, true); + mkdir($this->_compilationDir, 0777, true); } $this->_command = 'php ' . $basePath . '/bin/magento setup:di:compile-multi-tenant --generation=%s --di=%s'; diff --git a/dev/tests/static/testsuite/Magento/Test/Js/Exemplar/JsHintTest.php b/dev/tests/static/testsuite/Magento/Test/Js/Exemplar/JsHintTest.php index 41f50f985167d..64cdf4906e2ca 100644 --- a/dev/tests/static/testsuite/Magento/Test/Js/Exemplar/JsHintTest.php +++ b/dev/tests/static/testsuite/Magento/Test/Js/Exemplar/JsHintTest.php @@ -29,7 +29,7 @@ protected function setUp() { $reportFile = self::$_cmd->getReportFile(); if (!is_dir(dirname($reportFile))) { - mkdir(dirname($reportFile), 0770); + mkdir(dirname($reportFile)); } } diff --git a/dev/tests/static/testsuite/Magento/Test/Js/LiveCodeTest.php b/dev/tests/static/testsuite/Magento/Test/Js/LiveCodeTest.php index ed2f9664c00a9..592fa70cc9edc 100644 --- a/dev/tests/static/testsuite/Magento/Test/Js/LiveCodeTest.php +++ b/dev/tests/static/testsuite/Magento/Test/Js/LiveCodeTest.php @@ -60,7 +60,7 @@ public static function setUpBeforeClass() { $reportDir = Files::init()->getPathToSource() . '/dev/tests/static/report'; if (!is_dir($reportDir)) { - mkdir($reportDir, 0770); + mkdir($reportDir); } self::$_reportFile = $reportDir . '/js_report.txt'; @unlink(self::$_reportFile); diff --git a/dev/tests/static/testsuite/Magento/Test/Php/LiveCodeTest.php b/dev/tests/static/testsuite/Magento/Test/Php/LiveCodeTest.php index df6c415fcbdbc..33a063c59bbaa 100644 --- a/dev/tests/static/testsuite/Magento/Test/Php/LiveCodeTest.php +++ b/dev/tests/static/testsuite/Magento/Test/Php/LiveCodeTest.php @@ -42,7 +42,7 @@ public static function setUpBeforeClass() self::$pathToSource = Utility\Files::init()->getPathToSource(); self::$reportDir = self::$pathToSource . '/dev/tests/static/report'; if (!is_dir(self::$reportDir)) { - mkdir(self::$reportDir, 0770); + mkdir(self::$reportDir); } } diff --git a/lib/internal/Magento/Framework/App/Cache/Frontend/Factory.php b/lib/internal/Magento/Framework/App/Cache/Frontend/Factory.php index 169b350161d02..c2a5046f78ff2 100644 --- a/lib/internal/Magento/Framework/App/Cache/Frontend/Factory.php +++ b/lib/internal/Magento/Framework/App/Cache/Frontend/Factory.php @@ -67,7 +67,7 @@ class Factory */ protected $_backendOptions = [ 'hashed_directory_level' => 1, - 'hashed_directory_umask' => DriverInterface::WRITEABLE_DIRECTORY_MODE, + 'hashed_directory_umask' => 0777, 'file_name_prefix' => 'mage', ]; diff --git a/lib/internal/Magento/Framework/Archive/Helper/File.php b/lib/internal/Magento/Framework/Archive/Helper/File.php index 4a72a2846a75b..a0e8b3084e855 100644 --- a/lib/internal/Magento/Framework/Archive/Helper/File.php +++ b/lib/internal/Magento/Framework/Archive/Helper/File.php @@ -91,7 +91,7 @@ public function __destruct() * @throws LocalizedException * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ - public function open($mode = 'w+', $chmod = DriverInterface::WRITEABLE_FILE_MODE) + public function open($mode = 'w+', $chmod = null) { $this->_isInWriteMode = $this->_isWritableMode($mode); @@ -182,7 +182,7 @@ public function close() $this->_close(); $this->_fileHandler = false; - if ($this->_isInWriteMode) { + if ($this->_isInWriteMode && isset($this->_chmod)) { @chmod($this->_filePath, $this->_chmod); } } diff --git a/lib/internal/Magento/Framework/Archive/Tar.php b/lib/internal/Magento/Framework/Archive/Tar.php index d02fc9e24cb3d..fc57ef134023f 100644 --- a/lib/internal/Magento/Framework/Archive/Tar.php +++ b/lib/internal/Magento/Framework/Archive/Tar.php @@ -404,7 +404,7 @@ protected function _unpackCurrentTar($destination) if (in_array($header['type'], ["0", chr(0), ''])) { if (!file_exists($dirname)) { - $mkdirResult = @mkdir($dirname, DriverInterface::WRITEABLE_DIRECTORY_MODE, true); + $mkdirResult = @mkdir($dirname, 0777, true); if (false === $mkdirResult) { throw new \Magento\Framework\Exception\LocalizedException( diff --git a/lib/internal/Magento/Framework/Backup/Filesystem.php b/lib/internal/Magento/Framework/Backup/Filesystem.php index 4577bdda32e32..11156210b76e7 100644 --- a/lib/internal/Magento/Framework/Backup/Filesystem.php +++ b/lib/internal/Magento/Framework/Backup/Filesystem.php @@ -279,7 +279,6 @@ protected function _checkBackupsDir() } mkdir($backupsDir); - chmod($backupsDir, DriverInterface::WRITEABLE_DIRECTORY_MODE); } if (!is_writable($backupsDir)) { diff --git a/lib/internal/Magento/Framework/Backup/Filesystem/Rollback/Ftp.php b/lib/internal/Magento/Framework/Backup/Filesystem/Rollback/Ftp.php index 44ec96f4dcae3..11c689884cb03 100644 --- a/lib/internal/Magento/Framework/Backup/Filesystem/Rollback/Ftp.php +++ b/lib/internal/Magento/Framework/Backup/Filesystem/Rollback/Ftp.php @@ -126,7 +126,7 @@ protected function _createTmpDir() { $tmpDir = $this->_snapshot->getBackupsDir() . '/~tmp-' . microtime(true); - $result = @mkdir($tmpDir, DriverInterface::WRITEABLE_DIRECTORY_MODE); + $result = @mkdir($tmpDir); if (false === $result) { throw new \Magento\Framework\Backup\Exception\NotEnoughPermissions( diff --git a/lib/internal/Magento/Framework/Code/Generator/Io.php b/lib/internal/Magento/Framework/Code/Generator/Io.php index 4bdd3851eeb8f..aac1eea8d6cbe 100644 --- a/lib/internal/Magento/Framework/Code/Generator/Io.php +++ b/lib/internal/Magento/Framework/Code/Generator/Io.php @@ -169,7 +169,7 @@ private function _makeDirectory($directory) } try { if (!$this->filesystemDriver->isDirectory($directory)) { - $this->filesystemDriver->createDirectory($directory, DriverInterface::WRITEABLE_DIRECTORY_MODE); + $this->filesystemDriver->createDirectory($directory); } return true; } catch (FileSystemException $e) { diff --git a/lib/internal/Magento/Framework/File/Uploader.php b/lib/internal/Magento/Framework/File/Uploader.php index 573b2efdd4d8d..14077be8aeba2 100644 --- a/lib/internal/Magento/Framework/File/Uploader.php +++ b/lib/internal/Magento/Framework/File/Uploader.php @@ -223,7 +223,6 @@ public function save($destinationFolder, $newFileName = null) $this->_result = $this->_moveFile($this->_file['tmp_name'], $destinationFile); if ($this->_result) { - $this->chmod($destinationFile); if ($this->_enableFilesDispersion) { $fileName = str_replace('\\', '/', self::_addDirSeparator($this->_dispretionPath)) . $fileName; } @@ -242,10 +241,12 @@ public function save($destinationFolder, $newFileName = null) /** * @param string $file * @return void + * + * @deprecated */ protected function chmod($file) { - chmod($file, DriverInterface::WRITEABLE_DIRECTORY_MODE); + chmod($file, 0777); } /** @@ -554,7 +555,7 @@ private function _createDestinationFolder($destinationFolder) } if (!(@is_dir($destinationFolder) - || @mkdir($destinationFolder, DriverInterface::WRITEABLE_DIRECTORY_MODE, true) + || @mkdir($destinationFolder, 0777, true) )) { throw new \Exception("Unable to create directory '{$destinationFolder}'."); } diff --git a/lib/internal/Magento/Framework/Filesystem/Directory/Write.php b/lib/internal/Magento/Framework/Filesystem/Directory/Write.php index 3695898a5509c..483434b1d8fc9 100644 --- a/lib/internal/Magento/Framework/Filesystem/Directory/Write.php +++ b/lib/internal/Magento/Framework/Filesystem/Directory/Write.php @@ -15,7 +15,7 @@ class Write extends Read implements WriteInterface * * @var int */ - protected $permissions = DriverInterface::WRITEABLE_DIRECTORY_MODE; + protected $permissions = 0777; /** * Constructor diff --git a/lib/internal/Magento/Framework/Filesystem/Driver/File.php b/lib/internal/Magento/Framework/Filesystem/Driver/File.php index 22f77a3484826..acf9d48b09221 100644 --- a/lib/internal/Magento/Framework/Filesystem/Driver/File.php +++ b/lib/internal/Magento/Framework/Filesystem/Driver/File.php @@ -193,7 +193,7 @@ public function getParentDirectory($path) * @return bool * @throws FileSystemException */ - public function createDirectory($path, $permissions) + public function createDirectory($path, $permissions = 0777) { return $this->mkdirRecursive($path, $permissions); } @@ -206,7 +206,7 @@ public function createDirectory($path, $permissions) * @return bool * @throws FileSystemException */ - private function mkdirRecursive($path, $permissions) + private function mkdirRecursive($path, $permissions = 0777) { $path = $this->getScheme() . $path; if (is_dir($path)) { diff --git a/lib/internal/Magento/Framework/Filesystem/DriverInterface.php b/lib/internal/Magento/Framework/Filesystem/DriverInterface.php index 2d1748afb15ce..5e297b237e242 100644 --- a/lib/internal/Magento/Framework/Filesystem/DriverInterface.php +++ b/lib/internal/Magento/Framework/Filesystem/DriverInterface.php @@ -16,11 +16,15 @@ interface DriverInterface { /** * Permissions to give read/write/execute access to owner and owning group, but not to all users + * + * @deprecated */ const WRITEABLE_DIRECTORY_MODE = 0770; /** * Permissions to give read/write access to owner and owning group, but not to all users + * + * @deprecated */ const WRITEABLE_FILE_MODE = 0660; @@ -104,7 +108,7 @@ public function getParentDirectory($path); * @return bool * @throws FileSystemException */ - public function createDirectory($path, $permissions); + public function createDirectory($path, $permissions = 0777); /** * Read directory diff --git a/lib/internal/Magento/Framework/Filesystem/Io/File.php b/lib/internal/Magento/Framework/Filesystem/Io/File.php index 65c92dfd37e17..a835c4b25fe9d 100644 --- a/lib/internal/Magento/Framework/Filesystem/Io/File.php +++ b/lib/internal/Magento/Framework/Filesystem/Io/File.php @@ -301,7 +301,7 @@ public function close() * @param bool $recursive * @return bool */ - public function mkdir($dir, $mode = DriverInterface::WRITEABLE_DIRECTORY_MODE, $recursive = true) + public function mkdir($dir, $mode = 0777, $recursive = true) { $this->_cwd(); $result = @mkdir($dir, $mode, $recursive); @@ -553,7 +553,7 @@ public function createDestinationDir($path) * @return true * @throws \Exception */ - public function checkAndCreateFolder($folder, $mode = DriverInterface::WRITEABLE_DIRECTORY_MODE) + public function checkAndCreateFolder($folder, $mode = 0777) { if (is_dir($folder)) { return true; diff --git a/lib/internal/Magento/Framework/Filesystem/Io/Ftp.php b/lib/internal/Magento/Framework/Filesystem/Io/Ftp.php index 85d4c5fcd7604..eb14a261b48b6 100644 --- a/lib/internal/Magento/Framework/Filesystem/Io/Ftp.php +++ b/lib/internal/Magento/Framework/Filesystem/Io/Ftp.php @@ -159,7 +159,7 @@ public function close() * @return bool * @SuppressWarnings(PHPMD.UnusedFormalParameter) */ - public function mkdir($dir, $mode = DriverInterface::WRITEABLE_DIRECTORY_MODE, $recursive = true) + public function mkdir($dir, $mode = 0777, $recursive = true) { return @ftp_mkdir($this->_conn, $dir); } diff --git a/lib/internal/Magento/Framework/Filesystem/Io/IoInterface.php b/lib/internal/Magento/Framework/Filesystem/Io/IoInterface.php index b3b01736e2b2f..1adba174ede3e 100644 --- a/lib/internal/Magento/Framework/Filesystem/Io/IoInterface.php +++ b/lib/internal/Magento/Framework/Filesystem/Io/IoInterface.php @@ -35,7 +35,7 @@ public function close(); * @param bool $recursive * @return bool */ - public function mkdir($dir, $mode = DriverInterface::WRITEABLE_DIRECTORY_MODE, $recursive = true); + public function mkdir($dir, $mode = 0777, $recursive = true); /** * Delete a directory diff --git a/lib/internal/Magento/Framework/Filesystem/Io/Sftp.php b/lib/internal/Magento/Framework/Filesystem/Io/Sftp.php index db5ae3af625e7..f655442691423 100644 --- a/lib/internal/Magento/Framework/Filesystem/Io/Sftp.php +++ b/lib/internal/Magento/Framework/Filesystem/Io/Sftp.php @@ -78,7 +78,7 @@ public function close() * @return bool * @SuppressWarnings(PHPMD.UnusedFormalParameter) */ - public function mkdir($dir, $mode = DriverInterface::WRITEABLE_DIRECTORY_MODE, $recursive = true) + public function mkdir($dir, $mode = 0777, $recursive = true) { if ($recursive) { $no_errors = true; diff --git a/lib/internal/Magento/Framework/Logger/Handler/Base.php b/lib/internal/Magento/Framework/Logger/Handler/Base.php index 36a8cbb4a09ee..21c71ef4d6475 100644 --- a/lib/internal/Magento/Framework/Logger/Handler/Base.php +++ b/lib/internal/Magento/Framework/Logger/Handler/Base.php @@ -54,7 +54,7 @@ public function write(array $record) { $logDir = $this->filesystem->getParentDirectory($this->url); if (!$this->filesystem->isDirectory($logDir)) { - $this->filesystem->createDirectory($logDir, DriverInterface::WRITEABLE_DIRECTORY_MODE); + $this->filesystem->createDirectory($logDir); } parent::write($record); diff --git a/lib/internal/Magento/Framework/Setup/BackupRollback.php b/lib/internal/Magento/Framework/Setup/BackupRollback.php index fc50f1c5f1aa3..acec0cc7c14f6 100644 --- a/lib/internal/Magento/Framework/Setup/BackupRollback.php +++ b/lib/internal/Magento/Framework/Setup/BackupRollback.php @@ -121,7 +121,7 @@ public function codeBackup($time, $type = Factory::TYPE_FILESYSTEM) throw new LocalizedException(new Phrase("This backup type \'$type\' is not supported.")); } if (!$this->file->isExists($this->backupsDir)) { - $this->file->createDirectory($this->backupsDir, DriverInterface::WRITEABLE_DIRECTORY_MODE); + $this->file->createDirectory($this->backupsDir); } $fsBackup->setBackupsDir($this->backupsDir); $fsBackup->setBackupExtension('tgz'); @@ -201,7 +201,7 @@ public function dbBackup($time) $dbBackup = $this->objectManager->create('Magento\Framework\Backup\Db'); $dbBackup->setRootDir($this->directoryList->getRoot()); if (!$this->file->isExists($this->backupsDir)) { - $this->file->createDirectory($this->backupsDir, DriverInterface::WRITEABLE_DIRECTORY_MODE); + $this->file->createDirectory($this->backupsDir); } $dbBackup->setBackupsDir($this->backupsDir); $dbBackup->setBackupExtension('gz'); diff --git a/lib/internal/Magento/Framework/Setup/Test/Unit/BackupRollbackTest.php b/lib/internal/Magento/Framework/Setup/Test/Unit/BackupRollbackTest.php index 2013c5ce240a0..625addf46dfda 100644 --- a/lib/internal/Magento/Framework/Setup/Test/Unit/BackupRollbackTest.php +++ b/lib/internal/Magento/Framework/Setup/Test/Unit/BackupRollbackTest.php @@ -107,7 +107,7 @@ public function testCodeBackup() $this->filesystem->expects($this->once()) ->method('create'); $this->file->expects($this->once())->method('isExists')->with($this->path . '/backups')->willReturn(false); - $this->file->expects($this->once())->method('createDirectory')->with($this->path . '/backups', 0770); + $this->file->expects($this->once())->method('createDirectory')->with($this->path . '/backups'); $this->model->codeBackup(time()); } @@ -158,7 +158,7 @@ public function testMediaBackup() $this->filesystem->expects($this->once()) ->method('create'); $this->file->expects($this->once())->method('isExists')->with($this->path . '/backups')->willReturn(false); - $this->file->expects($this->once())->method('createDirectory')->with($this->path . '/backups', 0770); + $this->file->expects($this->once())->method('createDirectory')->with($this->path . '/backups'); $this->model->codeBackup(time(), Factory::TYPE_MEDIA); } diff --git a/lib/internal/Magento/Framework/System/Dirs.php b/lib/internal/Magento/Framework/System/Dirs.php index 0e16ddfbf11fa..5e3e9040ef751 100644 --- a/lib/internal/Magento/Framework/System/Dirs.php +++ b/lib/internal/Magento/Framework/System/Dirs.php @@ -75,7 +75,7 @@ public static function rm($dirname) * @return true * @throws \Exception */ - public static function mkdirStrict($path, $recursive = true, $mode = DriverInterface::WRITEABLE_DIRECTORY_MODE) + public static function mkdirStrict($path, $recursive = true, $mode = 0777) { $exists = file_exists($path); if ($exists && is_dir($path)) { diff --git a/lib/internal/Magento/Framework/System/Ftp.php b/lib/internal/Magento/Framework/System/Ftp.php index 932afef19e389..564f4473eae3c 100644 --- a/lib/internal/Magento/Framework/System/Ftp.php +++ b/lib/internal/Magento/Framework/System/Ftp.php @@ -52,7 +52,7 @@ public function mdkir($name) * @param int $mode * @return bool */ - public function mkdirRecursive($path, $mode = DriverInterface::WRITEABLE_DIRECTORY_MODE) + public function mkdirRecursive($path, $mode = 0777) { $this->checkConnected(); $dir = explode("/", $path); @@ -219,7 +219,7 @@ public function raw($cmd) * @return bool * @throws \Exception */ - public function upload($remote, $local, $dirMode = DriverInterface::WRITEABLE_DIRECTORY_MODE, $ftpMode = FTP_BINARY) + public function upload($remote, $local, $dirMode = 0777, $ftpMode = FTP_BINARY) { $this->checkConnected(); diff --git a/pub/.htaccess b/pub/.htaccess index 087064b0373c8..73b76bb10b4b4 100644 --- a/pub/.htaccess +++ b/pub/.htaccess @@ -3,6 +3,12 @@ # SetEnv MAGE_MODE developer +############################################ +## overrides default umask value to allow using different +## file permissions + +# SetEnv MAGE_UMASK 022 + ############################################ ## uncomment these lines for CGI mode ## make sure to specify the correct cgi php binary file name @@ -213,4 +219,4 @@ ############################################ ## prevent clickjacking Header set X-Frame-Options SAMEORIGIN - \ No newline at end of file + diff --git a/pub/errors/processor.php b/pub/errors/processor.php index ade81ea169e85..285de81d42ada 100644 --- a/pub/errors/processor.php +++ b/pub/errors/processor.php @@ -454,11 +454,10 @@ public function saveReport($reportData) $this->_setReportData($reportData); if (!file_exists($this->_reportDir)) { - @mkdir($this->_reportDir, DriverInterface::WRITEABLE_DIRECTORY_MODE, true); + @mkdir($this->_reportDir, 0777, true); } @file_put_contents($this->_reportFile, serialize($reportData)); - @chmod($this->_reportFile, DriverInterface::WRITEABLE_FILE_MODE); if (isset($reportData['skin']) && self::DEFAULT_SKIN != $reportData['skin']) { $this->_setSkin($reportData['skin']); diff --git a/setup/src/Magento/Setup/Console/Command/DiCompileMultiTenantCommand.php b/setup/src/Magento/Setup/Console/Command/DiCompileMultiTenantCommand.php index 7e020f4260e00..bed2b971758e6 100644 --- a/setup/src/Magento/Setup/Console/Command/DiCompileMultiTenantCommand.php +++ b/setup/src/Magento/Setup/Console/Command/DiCompileMultiTenantCommand.php @@ -407,7 +407,7 @@ private function compileCode($generationDir, $fileExcludePatterns, $input) // 2.2 Compression $relationsFileDir = dirname($relationsFile); if (!file_exists($relationsFileDir)) { - mkdir($relationsFileDir, DriverInterface::WRITEABLE_DIRECTORY_MODE, true); + mkdir($relationsFileDir, 0777, true); } $relations = array_filter($relations); file_put_contents($relationsFile, $serializer->serialize($relations)); @@ -425,7 +425,7 @@ private function compileCode($generationDir, $fileExcludePatterns, $input) $outputContent = $serializer->serialize($pluginDefinitions); $pluginDefFileDir = dirname($pluginDefFile); if (!file_exists($pluginDefFileDir)) { - mkdir($pluginDefFileDir, DriverInterface::WRITEABLE_DIRECTORY_MODE, true); + mkdir($pluginDefFileDir, 0777, true); } file_put_contents($pluginDefFile, $outputContent); } diff --git a/setup/src/Magento/Setup/Model/MarketplaceManager.php b/setup/src/Magento/Setup/Model/MarketplaceManager.php index 79d613302aeb3..b1d4e5c0d425a 100644 --- a/setup/src/Magento/Setup/Model/MarketplaceManager.php +++ b/setup/src/Magento/Setup/Model/MarketplaceManager.php @@ -313,9 +313,6 @@ public function saveAuthJson($username, $password) return $this->getDirectory()->writeFile( DirectoryList::COMPOSER_HOME . DIRECTORY_SEPARATOR . $this->pathToAuthFile, $jsonContent - ) && $this->getDirectory()->changePermissions( - DirectoryList::COMPOSER_HOME . DIRECTORY_SEPARATOR . $this->pathToAuthFile, - \Magento\Framework\Filesystem\DriverInterface::WRITEABLE_FILE_MODE ); } diff --git a/setup/src/Magento/Setup/Module/Di/Compiler/Config/Writer/Filesystem.php b/setup/src/Magento/Setup/Module/Di/Compiler/Config/Writer/Filesystem.php index ab8780a804553..daf8425802ac5 100644 --- a/setup/src/Magento/Setup/Module/Di/Compiler/Config/Writer/Filesystem.php +++ b/setup/src/Magento/Setup/Module/Di/Compiler/Config/Writer/Filesystem.php @@ -51,7 +51,7 @@ public function write($key, array $config) private function initialize() { if (!file_exists($this->directoryList->getPath(DirectoryList::DI))) { - mkdir($this->directoryList->getPath(DirectoryList::DI), DriverInterface::WRITEABLE_DIRECTORY_MODE); + mkdir($this->directoryList->getPath(DirectoryList::DI)); } } } diff --git a/setup/src/Magento/Setup/Module/I18n/Pack/Writer/File/AbstractFile.php b/setup/src/Magento/Setup/Module/I18n/Pack/Writer/File/AbstractFile.php index 4733634069ea7..31b376487a110 100644 --- a/setup/src/Magento/Setup/Module/I18n/Pack/Writer/File/AbstractFile.php +++ b/setup/src/Magento/Setup/Module/I18n/Pack/Writer/File/AbstractFile.php @@ -143,7 +143,7 @@ abstract protected function _getFileExtension(); * @param bool $recursive Allows the creation of nested directories specified in the $destinationPath * @return void */ - protected function _createDirectoryIfNotExist($destinationPath, $mode = 0750, $recursive = true) + protected function _createDirectoryIfNotExist($destinationPath, $mode = 0777, $recursive = true) { if (!is_dir($destinationPath)) { mkdir($destinationPath, $mode, $recursive);