From f5bfa260193da72f9c3948ac0593c6a91dc7a811 Mon Sep 17 00:00:00 2001 From: Ori Livneh Date: Fri, 6 Jun 2014 14:12:59 -0700 Subject: [PATCH] Fix behavior of Redis client Because HHVM forces auto_detect_line_ending to be enabled (which is a separate bug), if a CRLF is split across two TCP packets, fgets() is liable to get confused and assume that the stream uses CR line ending. As a result, the return value of Redis::sockReadLine() is liable to have either a trailing CR or a leading LF, both of which are corruptions. This causes the client to throw an exception, complaining of a protocol error. The behavior of fgets() is documented here: http://www.php.net/manual/en/function.fgets.php#101963 The Redis protocol spec affirms that responses terminate with CRLF and are guaranteed not to contain either CR or LF in the response body: http://redis.io/topics/protocol --- hphp/system/php/redis/Redis.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/hphp/system/php/redis/Redis.php b/hphp/system/php/redis/Redis.php index dd90dc6625713..6a96ed16e2f4f 100644 --- a/hphp/system/php/redis/Redis.php +++ b/hphp/system/php/redis/Redis.php @@ -966,6 +966,15 @@ protected function sockReadLine() { $line = fgets($this->connection); if (substr($line, -2) == "\r\n") { $line = substr($line, 0, -2); + } else if (substr($line, -1) == "\r") { + $line = substr($line, 0, -1); + $lf = fgetc($this->connection); + if ($lf === false) { + // A response must terminate with both CR and LF. Refuse to guess. + return false; + } else if ($lf !== "\n") { + throw new RedisException("Protocol error: CR not followed by LF"); + } } return $line;