Fix Redis read/write timeout units

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]

docs/manual/mod/mod_socache_redis.xml

@@ -104,6 +104,10 @@ RedisConnPoolTTL 60
     <p>Valid values for <directive>RedisTimeout</directive> are times
     up to one hour. 0 means no timeout.</p>
 
+    <p>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.</p>
+
     <note><p>This timeout defaults to units of seconds, but accepts
     suffixes for milliseconds (ms), seconds (s), minutes (min), and hours (h).
     </p></note>

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;
 }

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):

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"