When a Redis Cluster failover occurs, the client detects that the
redirected node was a replica of the old master and calls
cluster_map_keyspace() to remap the cluster topology.
If cluster_map_keyspace() fails (e.g., due to network issues during
the remap), it frees all node objects via zend_hash_clean(c->nodes)
and zeros the master array via memset(c->master, 0, ...).
The bug was that the return value of cluster_map_keyspace() was being
ignored in the failover detection path. This caused the code to
continue with NULL socket pointers, leading to segfaults when
dereferencing c->cmd_sock later.
This fix:
1. Checks the return value of cluster_map_keyspace() in failover
detection and returns FAILURE if it fails
2. Adds defense-in-depth NULL checks after MOVED and ASK redirections
to prevent segfaults if slots become NULL for any reason
Fixes production crashes observed during Redis Cluster failovers.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
All of these commands have the same form `<cmd> key`. `VINFO` is a bit
of an outlier however that uses simple strings as opposed to bulk
strings for the key names, meaning we had to create a custom handler.
See #2543
When the slot's->slaves was null, it was dereferencing null, causing
segfault.
This happens in weird scenario when some of the nodes in cluster are
down or having changed IP addresses without knowing about it.
Mostly null pointer derefs or use of uninitialized values. Some were
probably false positives since hte analyzer can't fully reason about how
the zend internals use `zval` structs but the fixes don't really have
any downside.
* Rework HMGET and implement HGETEX
Instead of using a bespoke NULL terminated `zval**` array for the
context array we can use a `HashTable`. This might be a tiny bit more
expensive but Zend hashtables are quite efficient and this should also
be less error prone.
* Rework our `HashTable` context array to store keys
Instead of sending an array of values we can instead add the fields as
keys to our context array. That way when we combine the keys with the
Redis provided values we can do it in-place and then just give the
HashTable to the user to then do with what they want.
* Implement HGETDEL command.
* Fix edge cases to abide by legacy behavior.
Previously we coerced integer strings into integer keys when zipping
`HMGET` responses. This commit adds logic so we continue to do this and
do not change semantics.
* Implement `HGETDEL` and `HGETEX` for `RedisCluster`.
This commit implements the new commands and reworks the `HMGET` reply
handler to use the new context `HashTable`.
* Fix an edge case where we get zero multiblk elements
* Tests for `HGETEX` and `HGETDEL`
* Minor logic improvement
We don't need to check if `c->reply_len > 0` in the last else block
since we have already determined it must be.
* Implement `HSETEX` for `Redis` and `RedisCluster`
* Use `zval_get_tmp_string` ro populating non-long keys
On some glibc implementations strncmp is a macro. This commit simply creates a
`redis_strncmp` static inline wrapper function so we can `ZEND_STRL` instead of
manually counting the length or using `sizeof(s)-1` each time.
Fixes#2565
PHP switched from `ZEND_ASSUME` to `ZEND_ASSERT` when making sure
`Z_PTR_P(zv)` was nonnull in `zend_hash_str_update_ptr`.
This commit just switches to `zend_hash_str_add_empty_element` which
is semantically more correct anyway.
Fixes#2539
When a node timeout occurs, then phpredis will try to connect to another
node, whose answer probably will be MOVED redirect. After this we need
more time to accomplish the redirection, otherwise we get "Timed out
attempting to find data in the correct node" error message.
Fixes#795#888#1142#1385#1633#1707#1811#2407
We also need to update the `RedisCluster` logic to handle very large
curosr values, in addition to handling them for the `Redis` and
`RedisArray` classes.
See #2454, #2458
* fix bug: the pipeline mode socket return an unexpected result after reconnecting
* fix typos: pipeline is right
---------
Co-authored-by: marcofu <marcofu@tencent.com>
* Use our `redis_cmd_append_sstr_key_*` and `redis_cmd_append_sstr_zval`
wrappers, which handle key prefixing and serialization transparently.
* Rework ZADD so it can handle the bulk double response from the `INCR`
options.
* Implement `ZDIFF`, `ZINTER`, `ZUNION`, `ZMSCORE`, and
`ZRANDMEMBER` for `RedisCluster`.
* Refactor `ZUNIONSTORE` command and switch to using our centralized
zset option parsing handler.
See #1894
* Create inline wrappers of the low-level php_stream_* functions that
also keep track of the number of bytes written/read.
* Change the logic to aggregate network traffic until the user
explicitly "resets" it. I think this will be a more common use-case
(running many commands and then seeing overall network IO).
See #2106
- Finish adding docblocks with examples for all of the stream commands.
- Fix XAUTOCLAIM response handler (the reply has a slightly different
structure to XCLAIM.
* Add ZRANGESTORE command.
* Add Redis 6.2's `REV`, `BYLEX`, and `BYSCORE` to ZRANGE options.
* Refactor several ZRANGE family commands into a single reply and
options handler, using PHP's new argument parsing macros.
* Extend our tests to use the new ZRANGE options.
See #1894
Implement the new Redis 7.0.0 commands to pop multiple elements from one
or more lists/zsets.
Additionally, remove INTERNAL_FUNCTION_PARAMETERS from the
redis_sock_read_multibulk_reply_zval helper function as it wasn't
actually being used.
* Suppress implicit fallthrough warnings by using an attribute if we
have it and a do { } while(0) if we don't.
* Move duplicated logic for appending a ZSET score to one utility
function.
