* Introduce a new `RedisCmd` struct to dynamically append RESP arguments
such that we don't have to precalculate the number of arguments the
command will have up front.
Additionally the new `RedisCmd allows both a `void *` context pointer
but also can attach a `void (*ctx_dtor)(void*)` destructor so we are
still able to clean up any allocated context when commands fail.
This moves the context cleanup out of every individual reply handler
and into the generic processing wrappers.
* Create a small group of `resp_str` helper functions for lower level
concatination of RESP protocol data over the wire.
* Lots of small modernization of the codebase such as using
`zend_string*` instead of (`char *`, `size_t`) pairs.
* Greatly simplify `crosslot` handling logic
redis_sock_read_bulk_reply called redis_check_eof(), which issues a
php_stream_eof() probe (a recv(MSG_PEEK) syscall when the stream buffer
is drained), before reading the bulk body. Every caller reaches this
function only after successfully reading the bulk-length header on the
same socket, which already proved the stream live, so the probe is
redundant and adds a kernel round-trip to the dominant GET/HGET/MGET
read path.
Replace it with a cheap NULL-stream guard. A disconnect that happens
mid-read is still caught by the existing php_stream_eof() check inside
the read loop.
cluster_dist_write emalloc'd a small int array (master + slaves) on every
read command issued under a failover-distribution mode, then freed it.
A shard's replica count is small in practice, so build the index list on
a 16-element stack array and only fall back to emalloc for larger
fan-outs. Both free sites are now guarded against freeing the stack.
When a RedisArray uses a custom hash algorithm, ra_find_node allocated
the hash context and digest buffers on the heap for every key lookup.
Both are small (the largest common context is SHA-512 at ~208 bytes),
so use stack buffers for the common case and fall back to emalloc only
when an algorithm's context or digest exceeds them.
The context buffer is a union with a double member to guarantee the
alignment the context structs require for their uint64_t state.
Several multibulk reply builders called array_init (8-bucket default)
right after reading the element count off the wire, forcing one or more
HashTable resizes as elements were appended. Switch these sites to
array_init_size using the known count.
The count is clamped to >= 0 because a null multibulk header yields -1,
and array_init_size takes a uint32_t; a negative value would otherwise
request a huge table. array_init_size(_, 0) is equivalent to array_init,
so the clamp is never worse than the prior behavior.
Covers redis_sock_read_multibulk_reply_zval, the LPOS COUNT path,
CLIENT TRACKINGINFO, HELLO, and nested multibulk in the recursive
variant reader.
cluster_send_command captured msstart via mstime() (a gettimeofday call)
on entry to every command, but the elapsed-time check below already only
calls mstime() when c->waitms is non-zero. When no request timeout is
configured, the initial capture is dead work. Gate it on c->waitms too.
redis_key_prefix used ecalloc to allocate the prefixed-key buffer, then
immediately overwrote the entire allocation with two memcpy calls. The
zero-fill was wasted work on every keyed argument when a prefix is set.
Use emalloc and write the single trailing NUL explicitly.
Clamp range in a couple library functions to values that can fit into an
iint, since we narrow it later.
A more comprehensive change that widens all of these values to 64 bits
will come in a future commit.
1. Make sure slot ranges are in bounds and that `high >= low`
2. Make sure any returned host lens are not >= `sizeof(c->redir_host)` so
they can be stored and null terminated.
3. Make sure all returned ports are sane (0-65535).
Previously a corrupted or malicious `MOVED` response could embed a host
name that was larger than the `c->redir_host` buffer which could leave
it non null-terminated.
Worse, `c->redir_host_len` was calculated from the too-large input which
could cause subsequent use to memcpy past the end of our buffer.
This fix simply hard rejects any host that we can't store in
`c->redir_host` while including a null terminator.
In addition we swich from a statically sized buffer in
`RedisCluster::_redir` to using `zend_smart_str`
Previously we were only checking if `LZ4_decompress_safe` was returning
> 0 but then blindly returning to the user whatever length the header
specified.
This fix does two things:
* Short circuits on negative length headers
* Fails the decompression if the decompressed length does not match.
Three independent hardening fixes against malicious cluster replies,
shipped together because they share the same threat model and live
within a few lines of each other in cluster_library.c.
Reject nil-bulk hosts in CLUSTER SLOTS rows. VALIDATE_SLOTS_INNER
checked type == TYPE_BULK but not str != NULL || len > 0, so a
hostile seed could drive redis_sock_create(NULL, (size_t)-1) into
zend_string_init's memmove from NULL. Apply the same str/len gate to
the slaves loop, which previously skipped only on len == 0.
Store ASK-redirect nodes in c->nodes. cluster_get_asking_node built
a fresh redisClusterNode on every ASK to an unknown host but never
inserted it, leaking the node plus its RedisSock, AUTH zend_strings,
slaves HashTable, and persistent connection for the cluster object's
lifetime.
Bound the redirect port and slot. atoi-into-(unsigned short) let a
hostile MOVED / ASK target reach port mod 65536; replace with strtol
plus explicit range checks and reject out-of-range values.
php_stream_getc returns int; storing into a char truncates the EOF
sentinel. On unsigned-char platforms (ARM Linux, AIX, PPC) EOF == -1
becomes 0xFF after promotion and res != EOF is always true; on
signed-char platforms a server byte of 0xFF is indistinguishable
from real EOF. Subscribe loops can busy-loop or stall. Return int,
matching the standard getc-style convention.
atol returns undefined behavior on overflow per C11 7.22.1.4. glibc
saturates to LONG_MAX, but musl, BSD libc, and Windows libc differ.
Replace atol / atoi at the three RESP length parse sites in library.c
with strtoll plus ERANGE rejection. The wire input is server-
controlled; an out-of-range value should drop the reply rather than
land an implementation-defined value in downstream length arithmetic.
generate_lock_secret derived the secret from hostname plus pid,
roughly 22 bits of guessable entropy and also readable from Redis
under <key>_LOCK. Replace with 16 bytes from php_random_bytes_silent,
hex-encoded.
Defense in depth: an attacker with write access to the Redis
instance can already bypass the lock by DELing the key, so this is
not a primary defense; worth fixing for the case where only the
lock key itself is exposed. The hostname|pid path stays as a fallback
when php_random_bytes_silent fails, so the caller always gets a
non-NULL secret.
Rarely CI hangs forever (or until it hits the maximum workflow execution
time) when Redis doesn't come up properly.
This just adds a configurable timeout so we can rerun the workflow when
it hangs.
Add support for Redis' `DELEX` and Valkey's `DELIFEQ` when deleting the
session lock key. Local testing shows about a 10-15% improvement over
the current `EVAL[SHA]` strategy.
This commit adds a new INI settingg:
```ini
redis.session.lock_release_cmd = delex|delifeq|eval
```
By default we continue to use the `EVAL` logic and if a user specifies
another mechanism but the command doesn't exist, we warn the user and
fall back to EVAL.
This commit also refactors a few functions to avoid UB and simplify key
construction.
The stub declares $seeds as ?array but the C code used format
specifier 'a' (non-nullable) instead of 'a!' in
zend_parse_method_parameters. This caused new RedisCluster(null, null)
to throw TypeError instead of RedisClusterException, contradicting
the declared type signature.
Also treat z_seeds == NULL the same as ZEND_NUM_ARGS() < 2 so that
explicitly passing null falls through to INI-based seed loading,
matching the behaviour when the argument is omitted entirely.
Fixes GH-2810.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Instead of currying around a `php_stream_context` object, just retain
the context array provided by the user itself like we do with other
connection information like host and port. This lets users reconnect in
a loop without leaking memory.
```php
$redis = new \Redis;
while (true) {
// Previously each reconnect call would leak the
// `php_stream_context` structure.
$redis->connect('tls://127.0.0.1', 9999, 1, null, 0, 0, [
'stream' => ['verify_peer' => false, 'verify_peer_name' => false],
]);
$redis->ping();
$redis->close();
}
```
We had a `delExType` enum which was later made redundant by
`redisEqType` as they have the same semantics.
This commit just removes `delExType` and uses the new enum.
In `array_zip_values_and_scores` we were blindly calling `Z_STR_P` on
the `zval` assuming it must be a string.
This isn't the case however if the user did something like this:
```php
$redis->setOption(Redis::OPT_SERIALIZER, Redis::SERIALIZER_PHP);
$redis->zAdd('zs', 3.14, ['pi', 'is', 'cool']);
// segfault when we try to get `Z_STR_P` from `['pi', 'is', 'cool']`
$redis->zRange('zs', 0, -1, true);
```
Potential fix for #2791
Michael Grunder