Fix ProtocolError
This commit attempts to fix hiredis such that a recoverable write error
will be retried rather than throwing a hard error.
Since our read/write functions are now behind function pointers, we
specify semantically that a return value of < 0 is a hard error, 0 a
recoverable error, and > 0 a success.
Our default `redisNetRead` function was already doing something similar
so this also improves code consistency.
Resolves#961
Co-authored-by: Maksim Tuleika <maksim.tuleika@appcast.io>
Currently, hiredis supports TCP/SSL/Unix, all of the connection types
use a single FD(int), close() is enough to close a connection. For the
further step, introduce .close method for redisContextFuncs, this
allows to close a complex connection context, for example RDMA.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Prevent incrementing passed a `\0` in our format string.
Co-authored-by: Michael Grunder <michael.grunder@gmail.com>
Co-authored-by: Kristján Valur Jónsson <sweskman@gmail.com>
A command with a faulty formatting string that lacks the
conversion specifier results in a ASAN heap-buffer-overflow.
This was due to that strchr() matches on null-termination,
which triggers a continuation of the string parsing.
When set hiredis will not automatically free replies in an async context, and the replies must be freed instead by the user.
Co-authored-by: Michael Grunder <michael.grunder@gmail.com>
Since redisGetReplyFromReader is exposed in a header file, we probably
shouldn't modify how it behaves in any way. For this reason, handle the
changed logic in an internal static helper method.
RESP3 invalidation messages always seemed to be sent before the response
to a given command, but it appears this is not always the case:
In Redis 6.2.0RC1 Redis sends the invalidation after the HSET in the
following sequence:
```
hget hash field
$5
value
hset hash field value
:0
>2
$10
invalidate
*1
$4
hash
```
To account for this possibility just wrap redisGetReplyFromReader in a
loop as it is called twice in redisGetReply.
When CLIENT TRACKING is enabled, Redis will send an invalidation message
with a NIL payload to all tracking clients after a FLUSHDB is executed.
We didn't account for REDIS_REPLY_PUSH being a valid parent object to a
NIL payload, and were failing an assertion.
Additionally this commit adds a regression test for the logic.
We need to allow our users to use redisContext->privdata as context
for any RESP3 PUSH messages, which means we can't use it for managing
SSL connections.
Bulletpoints:
* Create a secondary redisContext member for internal use only called
privctx and rename the redisContextFuncs->free_privdata accordingly.
* Adds a `free_privdata` function pointer so the user can tie allocated
memory to the lifetime of a redisContext (like they can already do
with redisAsyncContext)
* Enables SSL tests in .travis.yml
Proper support for RESP3 PUSH messages.
By default, PUSH messages are now intercepted and the reply memory freed.
This means existing code should work unchanged when connecting to Redis
>= 6.0.0 even if `CLIENT TRACKING` were then enabled.
Additionally, we define two callbacks users can configure if they wish to handle
these messages in a custom way:
void redisPushFn(void *privdata, void *reply);
void redisAsyncPushFn(redisAsyncContext *ac, void *reply);
See #825
* Adds an indirection to every allocation/deallocation to allow users to
plug in ones of their choosing (use custom functions, jemalloc, etc).
* Gracefully handle OOM everywhere in hiredis. This should make it possible
for users of the library to have more flexibility in how they handle such situations.
* Changes `redisReaderTask->elements` from an `int` to a `long long` to prevent
a possible overflow when transferring the task elements into a `redisReply`.
* Adds a configurable `max elements` member to `redisReader` that defaults to
2^32 - 1. This can be set to "unlimited" by setting the value to zero.
We currently perform a NULL check in redisGetReply and don't push the
reply back to the caller, but we don't free any reply meaning that this
will leak memory:
redisGetReply(context, NULL);
This change simply frees the reply if we were passed NULL.
Addresses #740
We should not attempt to keep the context and re-establish the
TLS connection for several reasons:
1. Maintain symmetry between redisConnect() and redisReconnect(), so in
both cases an extra step is required to initiate SSL.
2. The caller may also wish to reconfigure the SSL session and needs a
chance to do that.
3. It is not a practical thing to do on an async non-blocking connection
context.
The redisFD type should be equal to the system native socket file
desciptor type (for POSIX, this is a plain int).
We also introduce the REDIS_INVALID_FD value, which maps to -1 on POSIX
systems.
This makes hiredis.c free from system calls related to socket I/O. This
is also makes the treatment of raw socket connections more similar to
the SSL backend.
Referred to redisContextReadReply which I cannot find in this codebase
nor the old redis-tools one. Presumably this meant to say
redisGetReplyFromReader which is how redisBufferRead is used in this
file. Could've also meant the interface function redisReaderGetReply.
