diff --git a/changes-entries/redis-timeout-unit.txt b/changes-entries/redis-timeout-unit.txt new file mode 100644 index 00000000000..82cb9faa195 --- /dev/null +++ b/changes-entries/redis-timeout-unit.txt @@ -0,0 +1,3 @@ + *) mod_socache_redis: Fix the unit of the RedisTimeout passed to the Redis + client, which made the read/write timeout practically infinite. + PR 70193. [Christophe JAILLET, Arturo Bernal] diff --git a/docs/manual/mod/mod_socache_redis.xml b/docs/manual/mod/mod_socache_redis.xml index 453900e46f3..ec652455e8f 100644 --- a/docs/manual/mod/mod_socache_redis.xml +++ b/docs/manual/mod/mod_socache_redis.xml @@ -104,6 +104,10 @@ RedisConnPoolTTL 60

Valid values for RedisTimeout are times up to one hour. 0 means no timeout.

+

The Redis client has a granularity of whole seconds for the + Read/Write timeout. A positive value with a fractional second is + rounded up to the next second.

+

This timeout defaults to units of seconds, but accepts suffixes for milliseconds (ms), seconds (s), minutes (min), and hours (h).

diff --git a/modules/cache/mod_socache_redis.c b/modules/cache/mod_socache_redis.c index 450f406fbba..99bf97f0728 100644 --- a/modules/cache/mod_socache_redis.c +++ b/modules/cache/mod_socache_redis.c @@ -55,11 +55,13 @@ typedef struct { #endif #ifndef RD_DEFAULT_SERVER_TTL +/* In usec. */ #define RD_DEFAULT_SERVER_TTL apr_time_from_sec(15) #endif #ifndef RD_DEFAULT_SERVER_RWTO -#define RD_DEFAULT_SERVER_RWTO apr_time_from_sec(5) +/* In seconds */ +#define RD_DEFAULT_SERVER_RWTO 5 #endif module AP_MODULE_DECLARE_DATA socache_redis_module; @@ -449,8 +451,9 @@ static const char *socache_rd_set_rwto(cmd_parms *cmd, void *dummy, " can only be 0 or up to one hour.", NULL); } - /* apr_redis_server_create needs a ttl in usec. */ - sconf->rwto = rwto; + /* apr_redis_server_create needs a rwto in seconds, round up such that + * a positive timeout does not become 0. */ + sconf->rwto = apr_time_sec(rwto + apr_time_from_sec(1) - 1); return NULL; } diff --git a/test/modules/ssl/env.py b/test/modules/ssl/env.py index a8a8848b9fb..3e5554cd44e 100644 --- a/test/modules/ssl/env.py +++ b/test/modules/ssl/env.py @@ -13,6 +13,7 @@ def __init__(self, env: 'HttpdTestEnv'): super().__init__(env=env) self.add_source_dir(os.path.dirname(inspect.getfile(SSLTestSetup))) self.add_modules(["ssl"]) + self.add_optional_modules(["socache_redis"]) class SSLTestEnv(HttpdTestEnv): diff --git a/test/modules/ssl/test_004_socache_redis.py b/test/modules/ssl/test_004_socache_redis.py new file mode 100644 index 00000000000..d4e3f849537 --- /dev/null +++ b/test/modules/ssl/test_004_socache_redis.py @@ -0,0 +1,89 @@ +import socket +import time +from threading import Thread + +import pytest + +from pyhttpd.conf import HttpdConf +from .env import SSLTestEnv + + +class SilentRedis: + # accepts connections and never answers, so a client waits for its + # read timeout + + def __init__(self): + self._socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + self._socket.bind(('127.0.0.1', 0)) + self._socket.listen(5) + self._socket.settimeout(0.2) + self._done = False + self._conns = [] + self._thread = Thread(target=self._run, daemon=True) + + @property + def port(self): + return self._socket.getsockname()[1] + + def _run(self): + while not self._done: + try: + self._conns.append(self._socket.accept()[0]) + except socket.timeout: + pass + + def start(self): + self._thread.start() + + def stop(self): + self._done = True + self._thread.join(timeout=5) + for c in self._conns: + c.close() + self._socket.close() + + +@pytest.mark.skipif(condition=not SSLTestEnv.has_shared_module("socache_redis"), + reason="mod_socache_redis not available") +class TestSocacheRedis: + + @pytest.fixture(autouse=True, scope='class') + def _class_scope(self, env): + # storing the session of a TLSv1.2 handshake waits for the redis reply + env.httpd_error_log.add_ignored_lognos(["AH03478"]) + yield + env.httpd_error_log.remove_ignored_lognos(["AH03478"]) + + # RedisTimeout is passed to the redis client in whole seconds, a + # positive value is rounded up, 0 does not wait at all + @pytest.mark.parametrize(["timeout", "seconds"], [ + ["0", 0], + ["1ms", 1], + ["1s", 1], + ["1500ms", 2], + ]) + def test_ssl_004_01(self, env, timeout, seconds): + redis = SilentRedis() + redis.start() + try: + conf = HttpdConf(env, extras={ + "base": [ + f"SSLSessionCache redis:127.0.0.1:{redis.port}", + f"RedisTimeout {timeout}", + "SSLProtocol TLSv1.2", + ] + }) + conf.add_vhost_test1() + conf.install() + assert env.apache_restart() == 0 + url = env.mkurl("https", "test1", "/") + start = time.monotonic() + r = env.curl_get(url, options=["--tlsv1.2", "--tls-max", "1.2", + "--max-time", "10"]) + elapsed = time.monotonic() - start + finally: + redis.stop() + assert r.exit_code == 0, f"{r.stdout}{r.stderr}" + assert r.response["status"] == 200 + assert elapsed >= seconds - 0.2, f"waited only {elapsed:.1f}s" + assert elapsed < seconds + 1.5, f"waited {elapsed:.1f}s"