* Fix a double free when zipping keys and scores.
* Instead of aborting with an assertion if elements != 2 just warn and
return failure
* Instead of crashing on `xclaim` reply shape issues, just return false
Follow-up to the initial array_init_size pass. Converts the remaining
reply-array builders in library.c that have an element count available
at construction time, so multi-element replies don't pay HashTable
resizes as they fill: stream replies (XRANGE/XREAD/XINFO/XCLAIM/
XAUTOCLAIM), ACL LOG/GETUSER/CAT, GEOSEARCH, FUNCTION LIST, COMMAND
INFO, vlinks, the recursive variant reader, and pub/sub unsubscribe.
INFO, CLIENT INFO and CLIENT LIST are sized from a cheap delimiter
pre-count before tokenizing. Two known-empty results use
ZVAL_EMPTY_ARRAY (no HashTable allocation). redis_xrange_reply now
reads its message count before initializing the result so it can be
sized too.
Counts are clamped to >= 0 (a null multibulk yields -1); array_init_size
with a count <= 8 is identical to array_init, so the few fixed-small
sites are converted only for consistency.
Verified on PHP 8.4: streams, ACL, geo, function/command, INFO/CLIENT
parsing, ZMPOP/LMPOP and unsubscribe round-trip correctly; no leaks
under report_memleaks; testInfo/testClient/testZMPop/testLMPop/
testXAutoClaim/testSubscribe pass.
redis_mbulk_reply_loop read each element with redis_sock_read, which
emalloc's the bulk body into a raw char*, then copied it into a
zend_string via ZVAL_STRINGL and freed the char*. With no serializer or
compression active, the unwrap path through redis_unpack also resolves
to a plain copy, so every element of MGET/LRANGE/SMEMBERS/HGETALL paid
two allocations and a full-payload memcpy.
Add redis_sock_read_zstr, a zend_string-returning counterpart to
redis_sock_read, and redis_sock_read_bulk_zstr, which materializes the
bulk body straight into a zend_string. The loop moves that string into
the result array with ZVAL_STR (no copy) and only calls redis_unpack
when a serializer or compression is actually configured, in which case
it passes the string buffer and releases it afterward.
Verified on PHP 8.4: zero-copy and serializer (repack) paths, bulk edge
cases (empty, binary with NUL/CRLF, 1MB, null/missing), per-element
key/value parity for HGETALL, no leaks under report_memleaks, and the
testKeys/testHashes/testLSet/testSortAsc/testZRange suite methods.
* 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.
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.
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.
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.
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.
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.
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();
}
```
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
* Implement the new `MSETEX` Redis command.
* Refactor parsing of extended `SET` arguments into unified helper
functions, deduplicating logic.
* Add new `SET` arguments `IFNE`, `IFDEQ`, and `IFDNE`. We already
support Valkey's existing `IFEQ` argument, which Redis has added.
Fixes#2742
When a persistent ID is not provided, we were generating a "unique" one
by constructing a string with the current seconds and microseconds.
If two connections are createed in rapid succession, they can both
generate the same `persistent_id` and therefore both use the same
underlying `php_stream`.
```php
$r1 = new Redis;
$r2 = new Redis;
// If the two of these execute on the same second and usec, they will both use
// the same persistent_id and therefore the same underlying socket.
$r1->pconnect('localhost', 6379);
$r2->pconnect('localhost', 6379);
// Close and free the first connection
$r1->close();
// Segfault. Closing an already closed stream
$r2->close();
```
This commit simply adds a function scoped static monotonically
incremented counter which is used to make the persistent id's truly unique.
Fixes#2762
* PHP < 8.0 took a `char*` as `php_json_decode` input, whereas newer
versions take a const char * so ifdef around this.
* Fix compilation errors due to `false` not being defined. So as to make
a minimal change we can just use 0 and 1
Unfortunately `VEMB` has a unique `RESP2` reply as far as I can tell,
where it sends the embedding mode (int8, bin, fp32) as a simple string.
This would cause any of PhpRedis' generic reply handlers to turn that
into `true` which isn't useful. For that reason we need a custom reply
handler.
Additionally slightly rework `VINFO` to short circuit and return failure
if we read anything other than a bulk string or an integer reply type.
Otherwise we may get out of sync on the socket.
See #2543
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
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.
The current echo liveness check was doing one big complex conditional
trying to incorporate both sentinel's expected ERR no such command
response and non-sentinel's actual bulk reply to ECHO.
This commit refactors the logic to check the echo response into a little
helper with different logic depending on whether or not we're connected
to a sentinel.
Additionally, we add a test to verify that we are in fact reusing
persistent connections when the user requests a persistent connection
with `RedisSentinel`.
Fixes#2148
* 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
DragonflyDB will report to be Redis but also include `dragonfly_version`
in the hello response, which we can use to identify the fork.
Also fix parsing of the `HELLO` response for `serverName()` and
`serverVersion()`. Starting in Redis 8.0 there seem to always be modules
running, which the previous function was not expecting or parsing.
* New option 'database' for Redis class constructor
Selecting database is very common action after connecting to Redis. This simplifies lazy connecting to Redis, when requested database will be selected after first command.
* More specific exception message when invalid auth or database number is provided
Before it was just 'Redis server went away'
* Rename reselect_db method to redis_select_db and slightly optimise it
Right now we can't implement `HELLO` command to switch protocol
because we don't support new reply types that come with RESP3.
But we can use `HELLO` reply to expose some server information.
For an error reply we're starting at `buf + 1` so we want `len - 1`. As
a sanity check we now return early if `len < 1`.
Also, make certain that len > 2 for our special detection of `*-1` since
we're doing `memcmp(buf + 1, "-1", 2);`
Adds an option that instructs PhpRedis to not serialize or compress
numeric values. Specifically where `Z_TYPE_P(z) == IS_LONG` or
`Z_TYPE_P(z) == IS_DOUBLE`.
This flag lets the user enable serialization and/or compression while
still using the various increment/decrement command (`INCR`, `INCRBY`,
`DECR`, `DECRBY`, `INCRBYFLOAT`, `HINCRBY`, and `HINCRBYFLOAT`).
Because PhpRedis can't be certain that this option was enabled when
writing keys, there is a small runtime cost on the read-side that tests
whether or not the value its reading is a pure integer or floating point
value.
See #23
michael-grunder