From c6de97c5a833bfec2b38cbc82e03097677539416 Mon Sep 17 00:00:00 2001 From: Aleksander Machniak Date: Tue, 24 Sep 2019 10:16:17 +0200 Subject: [PATCH] Redis: Improve error handling and phpredis 5.X support (#6888) --- CHANGELOG | 1 + program/lib/Roundcube/cache/memcache.php | 2 +- program/lib/Roundcube/cache/redis.php | 72 ++++++++++++--------- program/lib/Roundcube/session/memcache.php | 2 +- program/lib/Roundcube/session/memcached.php | 2 +- program/lib/Roundcube/session/redis.php | 53 +++++++++++---- 6 files changed, 88 insertions(+), 44 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 6c2b75d34..949b16fc9 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,6 +2,7 @@ CHANGELOG Roundcube Webmail =========================== - Elastic: Resizeable columns (#6929) +- Redis: Improve error handling and phpredis 5.X support (#6888) RELEASE 1.4-rc2 --------------- diff --git a/program/lib/Roundcube/cache/memcache.php b/program/lib/Roundcube/cache/memcache.php index cf84f2bd0..371796d3d 100644 --- a/program/lib/Roundcube/cache/memcache.php +++ b/program/lib/Roundcube/cache/memcache.php @@ -96,7 +96,7 @@ class rcube_cache_memcache extends rcube_cache if (!$seen["$host:$port"]++) { $available--; rcube::raise_error(array( - 'code' => 604, 'type' => 'db', + 'code' => 604, 'type' => 'memcache', 'line' => __LINE__, 'file' => __FILE__, 'message' => "Memcache failure on host $host:$port"), true, false); diff --git a/program/lib/Roundcube/cache/redis.php b/program/lib/Roundcube/cache/redis.php index a359ec21d..6972799d4 100644 --- a/program/lib/Roundcube/cache/redis.php +++ b/program/lib/Roundcube/cache/redis.php @@ -111,6 +111,7 @@ class rcube_cache_redis extends rcube_cache } self::$redis = new Redis; + $failures = 0; foreach ($hosts as $redis_host) { // explode individual fields @@ -128,47 +129,40 @@ class rcube_cache_redis extends rcube_cache try { if (self::$redis->connect($host, $port) === false) { - rcube::raise_error(array( - 'code' => 604, - 'type' => 'redis', - 'line' => __LINE__, - 'file' => __FILE__, - 'message' => "Could not connect to Redis server. Please check host and port" - ), - true, true); + throw new Exception("Could not connect to Redis server. Please check host and port."); } if ($password !== null && self::$redis->auth($password) === false) { - rcube::raise_error(array( - 'code' => 604, - 'type' => 'redis', - 'line' => __LINE__, - 'file' => __FILE__, - 'message' => "Could not authenticate with Redis server. Please check password." - ), - true, true); + throw new Exception("Could not authenticate with Redis server. Please check password."); } if ($database !== null && self::$redis->select($database) === false) { - rcube::raise_error(array( - 'code' => 604, - 'type' => 'redis', - 'line' => __LINE__, - 'file' => __FILE__, - 'message' => "Could not select Redis database. Please check database setting." - ), - true, true); + throw new Exception("Could not select Redis database. Please check database setting."); } } catch (Exception $e) { - rcube::raise_error($e, true, true); + rcube::raise_error($e, true, false); + $failures++; } } - if (self::$redis->ping() !== "+PONG") { + if (count($hosts) === $failures) { self::$redis = false; } + if (self::$redis) { + try { + $ping = self::$redis->ping(); + if ($ping !== true && $ping !== "+PONG") { + throw new Exception("Redis connection failure. Ping failed."); + } + } + catch (Exception $e) { + self::$redis = false; + rcube::raise_error($e, true, false); + } + } + return self::$redis; } @@ -201,7 +195,13 @@ class rcube_cache_redis extends rcube_cache return false; } - $data = self::$redis->get($key); + try { + $data = self::$redis->get($key); + } + catch (Exception $e) { + raise_error($e, true, false); + return false; + } if ($this->debug) { $this->debug('get', $key, $data); @@ -224,7 +224,13 @@ class rcube_cache_redis extends rcube_cache return false; } - $result = self::$redis->setEx($key, $this->ttl, $data); + try { + $result = self::$redis->setEx($key, $this->ttl, $data); + } + catch (Exception $e) { + raise_error($e, true, false); + return false; + } if ($this->debug) { $this->debug('set', $key, $data, $result); @@ -246,8 +252,14 @@ class rcube_cache_redis extends rcube_cache return false; } - $fname = method_exists(self::$redis, 'del') ? 'del' : 'delete'; - $result = self::$redis->$fname($key); + try { + $fname = method_exists(self::$redis, 'del') ? 'del' : 'delete'; + $result = self::$redis->$fname($key); + } + catch (Exception $e) { + raise_error($e, true, false); + return false; + } if ($this->debug) { $this->debug('delete', $key, null, $result); diff --git a/program/lib/Roundcube/session/memcache.php b/program/lib/Roundcube/session/memcache.php index 7b8b31f87..be22dda88 100644 --- a/program/lib/Roundcube/session/memcache.php +++ b/program/lib/Roundcube/session/memcache.php @@ -48,7 +48,7 @@ class rcube_session_memcache extends rcube_session if (!$this->memcache) { rcube::raise_error(array( - 'code' => 604, 'type' => 'db', + 'code' => 604, 'type' => 'memcache', 'line' => __LINE__, 'file' => __FILE__, 'message' => "Failed to connect to memcached. Please check configuration"), true, true); diff --git a/program/lib/Roundcube/session/memcached.php b/program/lib/Roundcube/session/memcached.php index 5e06e3141..cb404514c 100644 --- a/program/lib/Roundcube/session/memcached.php +++ b/program/lib/Roundcube/session/memcached.php @@ -48,7 +48,7 @@ class rcube_session_memcached extends rcube_session if (!$this->memcache) { rcube::raise_error(array( - 'code' => 604, 'type' => 'db', + 'code' => 604, 'type' => 'memcache', 'line' => __LINE__, 'file' => __FILE__, 'message' => "Failed to connect to memcached. Please check configuration"), true, true); diff --git a/program/lib/Roundcube/session/redis.php b/program/lib/Roundcube/session/redis.php index 7671519d8..51953b5bb 100644 --- a/program/lib/Roundcube/session/redis.php +++ b/program/lib/Roundcube/session/redis.php @@ -42,6 +42,14 @@ class rcube_session_redis extends rcube_session { $this->redis = rcube::get_instance()->get_redis(); $this->debug = $config->get('redis_debug'); + if (!$this->redis) { + rcube::raise_error(array( + 'code' => 604, 'type' => 'redis', + 'line' => __LINE__, 'file' => __FILE__, + 'message' => "Failed to connect to redis. Please check configuration"), + true, true); + } + // register sessions handler $this->register_session_handler(); } @@ -79,8 +87,13 @@ class rcube_session_redis extends rcube_session { public function destroy($key) { if ($key) { - $fname = method_exists($this->redis, 'del') ? 'del' : 'delete'; - $result = $this->redis->$fname($key); + try { + $fname = method_exists($this->redis, 'del') ? 'del' : 'delete'; + $result = $this->redis->$fname($key); + } + catch (Exception $e) { + rcube::raise_error($e, true, true); + } if ($this->debug) { $this->debug('delete', $key, null, $result); @@ -99,7 +112,18 @@ class rcube_session_redis extends rcube_session { */ public function read($key) { - if ($value = $this->redis->get($key)) { + try { + $value = $this->redis->get($key); + } + catch (Exception $e) { + rcube::raise_error($e, true, true); + } + + if ($this->debug) { + $this->debug('get', $key, $value); + } + + if ($value) { $arr = unserialize($value); $this->changed = $arr['changed']; $this->ip = $arr['ip']; @@ -107,10 +131,6 @@ class rcube_session_redis extends rcube_session { $this->key = $key; } - if ($this->debug) { - $this->debug('get', $key, $value); - } - return $this->vars ?: ''; } @@ -128,8 +148,14 @@ class rcube_session_redis extends rcube_session { $ts = microtime(true); if ($newvars !== $oldvars || $ts - $this->changed > $this->lifetime / 3) { - $data = serialize(array('changed' => time(), 'ip' => $this->ip, 'vars' => $newvars)); - $result = $this->redis->setex($key, $this->lifetime + 60, $data); + $data = serialize(array('changed' => time(), 'ip' => $this->ip, 'vars' => $newvars)); + + try { + $result = $this->redis->setex($key, $this->lifetime + 60, $data); + } + catch (Exception $e) { + rcube::raise_error($e, true, true); + } if ($this->debug) { $this->debug('set', $key, $data, $result); @@ -155,8 +181,13 @@ class rcube_session_redis extends rcube_session { return true; } - $data = serialize(array('changed' => time(), 'ip' => $this->ip, 'vars' => $vars)); - $result = $this->redis->setex($key, $this->lifetime + 60, $data); + try { + $data = serialize(array('changed' => time(), 'ip' => $this->ip, 'vars' => $vars)); + $result = $this->redis->setex($key, $this->lifetime + 60, $data); + } + catch (Exception $e) { + rcube::raise_error($e, true, true); + } if ($this->debug) { $this->debug('set', $key, $data, $result);