From 79ae5ffc693b57688b4c76141fd2c94868ebdbff Mon Sep 17 00:00:00 2001 From: Michael Grunder Date: Mon, 5 Sep 2022 11:59:21 -0700 Subject: Fix protocol error (#1106) 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 --- hiredis.c | 4 ++-- hiredis.h | 6 ++++++ net.c | 12 ++++++++---- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/hiredis.c b/hiredis.c index 8b9ceca..5e8b606 100644 --- a/hiredis.c +++ b/hiredis.c @@ -986,8 +986,8 @@ int redisBufferRead(redisContext *c) { * successfully written to the socket. When the buffer is empty after the * write operation, "done" is set to 1 (if given). * - * Returns REDIS_ERR if an error occurred trying to write and sets - * c->errstr to hold the appropriate error string. + * Returns REDIS_ERR if an unrecoverable error occurred in the underlying + * c->funcs->write function. */ int redisBufferWrite(redisContext *c, int *done) { diff --git a/hiredis.h b/hiredis.h index 5969368..0a17495 100644 --- a/hiredis.h +++ b/hiredis.h @@ -249,10 +249,16 @@ typedef struct redisContextFuncs { void (*free_privctx)(void *); void (*async_read)(struct redisAsyncContext *); void (*async_write)(struct redisAsyncContext *); + + /* Read/Write data to the underlying communication stream, returning the + * number of bytes read/written. In the event of an unrecoverable error + * these functions shall return a value < 0. In the event of a + * recoverable error, they should return 0. */ ssize_t (*read)(struct redisContext *, char *, size_t); ssize_t (*write)(struct redisContext *); } redisContextFuncs; + /* Context for a connection to Redis */ typedef struct redisContext { const redisContextFuncs *funcs; /* Function table */ diff --git a/net.c b/net.c index 2b591b9..0f36da2 100644 --- a/net.c +++ b/net.c @@ -70,7 +70,7 @@ ssize_t redisNetRead(redisContext *c, char *buf, size_t bufcap) { __redisSetError(c, REDIS_ERR_TIMEOUT, "recv timeout"); return -1; } else { - __redisSetError(c, REDIS_ERR_IO, NULL); + __redisSetError(c, REDIS_ERR_IO, strerror(errno)); return -1; } } else if (nread == 0) { @@ -82,15 +82,19 @@ ssize_t redisNetRead(redisContext *c, char *buf, size_t bufcap) { } ssize_t redisNetWrite(redisContext *c) { - ssize_t nwritten = send(c->fd, c->obuf, sdslen(c->obuf), 0); + ssize_t nwritten; + + nwritten = send(c->fd, c->obuf, sdslen(c->obuf), 0); if (nwritten < 0) { if ((errno == EWOULDBLOCK && !(c->flags & REDIS_BLOCK)) || (errno == EINTR)) { - /* Try again later */ + /* Try again */ + return 0; } else { - __redisSetError(c, REDIS_ERR_IO, NULL); + __redisSetError(c, REDIS_ERR_IO, strerror(errno)); return -1; } } + return nwritten; } -- cgit v1.2.3