diff options
-rw-r--r-- | .github/release-drafter-config.yml | 49 | ||||
-rw-r--r-- | .github/workflows/release-drafter.yml | 19 | ||||
-rw-r--r-- | CMakeLists.txt | 2 | ||||
-rw-r--r-- | README.md | 20 | ||||
-rw-r--r-- | async.c | 64 | ||||
-rw-r--r-- | async.h | 2 | ||||
-rw-r--r-- | hiredis_ssl.h | 33 | ||||
-rw-r--r-- | ssl.c | 21 | ||||
-rw-r--r-- | test.c | 54 |
9 files changed, 235 insertions, 29 deletions
diff --git a/.github/release-drafter-config.yml b/.github/release-drafter-config.yml new file mode 100644 index 0000000..de4fea7 --- /dev/null +++ b/.github/release-drafter-config.yml @@ -0,0 +1,49 @@ +name-template: '$NEXT_MAJOR_VERSION' +tag-template: 'v$NEXT_MAJOR_VERSION' +autolabeler: + - label: 'maintenance' + files: + - '*.md' + - '.github/*' + - label: 'bug' + branch: + - '/bug-.+' + - label: 'maintenance' + branch: + - '/maintenance-.+' + - label: 'feature' + branch: + - '/feature-.+' +categories: + - title: 'Breaking Changes' + labels: + - 'breakingchange' + + - title: '๐งช Experimental Features' + labels: + - 'experimental' + - title: '๐ New Features' + labels: + - 'feature' + - 'enhancement' + - title: '๐ Bug Fixes' + labels: + - 'fix' + - 'bugfix' + - 'bug' + - 'BUG' + - title: '๐งฐ Maintenance' + label: 'maintenance' +change-template: '- $TITLE (#$NUMBER)' +exclude-labels: + - 'skip-changelog' +template: | + ## Changes + + $CHANGES + + ## Contributors + We'd like to thank all the contributors who worked on this release! + + $CONTRIBUTORS + diff --git a/.github/workflows/release-drafter.yml b/.github/workflows/release-drafter.yml new file mode 100644 index 0000000..ec2d88b --- /dev/null +++ b/.github/workflows/release-drafter.yml @@ -0,0 +1,19 @@ +name: Release Drafter + +on: + push: + # branches to consider in the event; optional, defaults to all + branches: + - master + +jobs: + update_release_draft: + runs-on: ubuntu-latest + steps: + # Drafts your next Release notes as Pull Requests are merged into "master" + - uses: release-drafter/release-drafter@v5 + with: + # (Optional) specify config name to use, relative to .github/. Default: release-drafter.yml + config-name: release-drafter-config.yml + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/CMakeLists.txt b/CMakeLists.txt index ee149e6..f77dfd5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,4 +1,4 @@ -CMAKE_MINIMUM_REQUIRED(VERSION 3.4.0) +CMAKE_MINIMUM_REQUIRED(VERSION 3.0.0) OPTION(ENABLE_SSL "Build hiredis_ssl for SSL support" OFF) OPTION(DISABLE_TESTS "If tests should be compiled or not" OFF) @@ -250,7 +250,7 @@ following two execution paths: * Read from the socket until a single reply could be parsed The function `redisGetReply` is exported as part of the Hiredis API and can be used when a reply -is expected on the socket. To pipeline commands, the only things that needs to be done is +is expected on the socket. To pipeline commands, the only thing that needs to be done is filling up the output buffer. For this cause, two commands can be used that are identical to the `redisCommand` family, apart from not returning a reply: ```c @@ -357,9 +357,9 @@ void(const redisAsyncContext *c, int status); ``` On a *connect*, the `status` argument is set to `REDIS_OK` if the connection attempt succeeded. In this -case, the context is ready to accept commands. If it is called with `REDIS_ERR` then the +case, the context is ready to accept commands. If it is called with `REDIS_ERR` then the connection attempt failed. The `err` field in the context can be accessed to find out the cause of the error. -After a failed connection attempt, the context object is automatically freed by the libary after calling +After a failed connection attempt, the context object is automatically freed by the library after calling the connect callback. This may be a good point to create a new context and retry the connection. On a disconnect, the `status` argument is set to `REDIS_OK` when disconnection was initiated by the @@ -375,7 +375,7 @@ api will return `REDIS_ERR`. The function to set the callbacks have the followin int redisAsyncSetConnectCallback(redisAsyncContext *ac, redisConnectCallback *fn); int redisAsyncSetDisconnectCallback(redisAsyncContext *ac, redisDisconnectCallback *fn); ``` -`ac->data` may be used to pass user data to both of thes callbacks. An typical implementation +`ac->data` may be used to pass user data to both callbacks. A typical implementation might look something like this: ```c void appOnConnect(redisAsyncContext *c, int status) @@ -444,9 +444,9 @@ For every command issued, with the exception of **SUBSCRIBE** and **PSUBSCRIBE** called exactly once. Even if the context object id disconnected or deleted, every pending callback will be called with a `NULL` reply. -For **SUBSCRIBE** and **PSUBSCRIBE**, the callbacks may be called repeatedly until a `unsubscribe` +For **SUBSCRIBE** and **PSUBSCRIBE**, the callbacks may be called repeatedly until an `unsubscribe` message arrives. This will be the last invocation of the callback. In case of error, the callbacks -may reive a final `NULL` reply instead. +may receive a final `NULL` reply instead. ### Disconnecting @@ -604,7 +604,7 @@ redisSSLContext *ssl_context; /* An error variable to indicate what went wrong, if the context fails to * initialize. */ -redisSSLContextError ssl_error; +redisSSLContextError ssl_error = REDIS_SSL_CTX_NONE; /* Initialize global OpenSSL state. * @@ -622,11 +622,11 @@ ssl_context = redisCreateSSLContext( "redis.mydomain.com", /* Server name to request (SNI), optional */ &ssl_error); -if(ssl_context == NULL || ssl_error != 0) { +if(ssl_context == NULL || ssl_error != REDIS_SSL_CTX_NONE) { /* Handle error and abort... */ /* e.g. printf("SSL error: %s\n", - (ssl_error != 0) ? + (ssl_error != REDIS_SSL_CTX_NONE) ? redisSSLContextGetError(ssl_error) : "Unknown error"); // Abort */ @@ -708,7 +708,7 @@ If you have a unique use-case where you don't want hiredis to automatically inte redisSetPushCallback(context, NULL); ``` - _Note: With no handler configured, calls to `redisCommand` may generate more than one reply, so this strategy is only applicable when there's some kind of blocking`redisGetReply()` loop (e.g. `MONITOR` or `SUBSCRIBE` workloads)._ + _Note: With no handler configured, calls to `redisCommand` may generate more than one reply, so this strategy is only applicable when there's some kind of blocking `redisGetReply()` loop (e.g. `MONITOR` or `SUBSCRIBE` workloads)._ ## Allocator injection @@ -303,6 +303,34 @@ static void __redisRunPushCallback(redisAsyncContext *ac, redisReply *reply) { } } +static void __redisRunConnectCallback(redisAsyncContext *ac, int status) +{ + if (ac->onConnect) { + if (!(ac->c.flags & REDIS_IN_CALLBACK)) { + ac->c.flags |= REDIS_IN_CALLBACK; + ac->onConnect(ac, status); + ac->c.flags &= ~REDIS_IN_CALLBACK; + } else { + /* already in callback */ + ac->onConnect(ac, status); + } + } +} + +static void __redisRunDisconnectCallback(redisAsyncContext *ac, int status) +{ + if (ac->onDisconnect) { + if (!(ac->c.flags & REDIS_IN_CALLBACK)) { + ac->c.flags |= REDIS_IN_CALLBACK; + ac->onDisconnect(ac, status); + ac->c.flags &= ~REDIS_IN_CALLBACK; + } else { + /* already in callback */ + ac->onDisconnect(ac, status); + } + } +} + /* Helper function to free the context. */ static void __redisAsyncFree(redisAsyncContext *ac) { redisContext *c = &(ac->c); @@ -338,12 +366,11 @@ static void __redisAsyncFree(redisAsyncContext *ac) { /* Execute disconnect callback. When redisAsyncFree() initiated destroying * this context, the status will always be REDIS_OK. */ - if (ac->onDisconnect && (c->flags & REDIS_CONNECTED)) { - if (c->flags & REDIS_FREEING) { - ac->onDisconnect(ac,REDIS_OK); - } else { - ac->onDisconnect(ac,(ac->err == 0) ? REDIS_OK : REDIS_ERR); - } + if (c->flags & REDIS_CONNECTED) { + int status = ac->err == 0 ? REDIS_OK : REDIS_ERR; + if (c->flags & REDIS_FREEING) + status = REDIS_OK; + __redisRunDisconnectCallback(ac, status); } if (ac->dataCleanup) { @@ -603,7 +630,7 @@ void redisProcessCallbacks(redisAsyncContext *ac) { } static void __redisAsyncHandleConnectFailure(redisAsyncContext *ac) { - if (ac->onConnect) ac->onConnect(ac, REDIS_ERR); + __redisRunConnectCallback(ac, REDIS_ERR); __redisAsyncDisconnect(ac); } @@ -628,8 +655,19 @@ static int __redisAsyncHandleConnect(redisAsyncContext *ac) { return REDIS_ERR; } - if (ac->onConnect) ac->onConnect(ac, REDIS_OK); + /* flag us as fully connect, but allow the callback + * to disconnect. For that reason, permit the function + * to delete the context here after callback return. + */ c->flags |= REDIS_CONNECTED; + __redisRunConnectCallback(ac, REDIS_OK); + if ((ac->c.flags & REDIS_DISCONNECTING)) { + redisAsyncDisconnect(ac); + return REDIS_ERR; + } else if ((ac->c.flags & REDIS_FREEING)) { + redisAsyncFree(ac); + return REDIS_ERR; + } return REDIS_OK; } else { return REDIS_OK; @@ -653,6 +691,8 @@ void redisAsyncRead(redisAsyncContext *ac) { */ void redisAsyncHandleRead(redisAsyncContext *ac) { redisContext *c = &(ac->c); + /* must not be called from a callback */ + assert(!(c->flags & REDIS_IN_CALLBACK)); if (!(c->flags & REDIS_CONNECTED)) { /* Abort connect was not successful. */ @@ -686,6 +726,8 @@ void redisAsyncWrite(redisAsyncContext *ac) { void redisAsyncHandleWrite(redisAsyncContext *ac) { redisContext *c = &(ac->c); + /* must not be called from a callback */ + assert(!(c->flags & REDIS_IN_CALLBACK)); if (!(c->flags & REDIS_CONNECTED)) { /* Abort connect was not successful. */ @@ -702,6 +744,8 @@ void redisAsyncHandleWrite(redisAsyncContext *ac) { void redisAsyncHandleTimeout(redisAsyncContext *ac) { redisContext *c = &(ac->c); redisCallback cb; + /* must not be called from a callback */ + assert(!(c->flags & REDIS_IN_CALLBACK)); if ((c->flags & REDIS_CONNECTED)) { if (ac->replies.head == NULL && ac->sub.replies.head == NULL) { @@ -721,8 +765,8 @@ void redisAsyncHandleTimeout(redisAsyncContext *ac) { __redisAsyncCopyError(ac); } - if (!(c->flags & REDIS_CONNECTED) && ac->onConnect) { - ac->onConnect(ac, REDIS_ERR); + if (!(c->flags & REDIS_CONNECTED)) { + __redisRunConnectCallback(ac, REDIS_ERR); } while (__redisShiftCallback(&ac->replies, &cb) == REDIS_OK) { @@ -57,7 +57,7 @@ typedef struct redisCallbackList { /* Connection callback prototypes */ typedef void (redisDisconnectCallback)(const struct redisAsyncContext*, int status); -typedef void (redisConnectCallback)(const struct redisAsyncContext*, int status); +typedef void (redisConnectCallback)(struct redisAsyncContext*, int status); typedef void(redisTimerCallback)(void *timer, void *privdata); /* Context for an async connection to Redis */ diff --git a/hiredis_ssl.h b/hiredis_ssl.h index e3d3e1c..26bc9e9 100644 --- a/hiredis_ssl.h +++ b/hiredis_ssl.h @@ -61,6 +61,27 @@ typedef enum { REDIS_SSL_CTX_OS_CERT_ADD_FAILED /* Failed to add CA certificates obtained from system to the SSL context */ } redisSSLContextError; +/* Constants that mirror OpenSSL's verify modes. By default, + * REDIS_SSL_VERIFY_PEER is used with redisCreateSSLContext(). + * Some Redis clients disable peer verification if there are no + * certificates specified. + */ +#define REDIS_SSL_VERIFY_NONE 0x00 +#define REDIS_SSL_VERIFY_PEER 0x01 +#define REDIS_SSL_VERIFY_FAIL_IF_NO_PEER_CERT 0x02 +#define REDIS_SSL_VERIFY_CLIENT_ONCE 0x04 +#define REDIS_SSL_VERIFY_POST_HANDSHAKE 0x08 + +/* Options to create an OpenSSL context. */ +typedef struct { + const char *cacert_filename; + const char *capath; + const char *cert_filename; + const char *private_key_filename; + const char *server_name; + int verify_mode; +} redisSSLOptions; + /** * Return the error message corresponding with the specified error code. */ @@ -102,6 +123,18 @@ redisSSLContext *redisCreateSSLContext(const char *cacert_filename, const char * const char *server_name, redisSSLContextError *error); /** + * Helper function to initialize an OpenSSL context that can be used + * to initiate SSL connections. This is a more extensible version of redisCreateSSLContext(). + * + * options contains a structure of SSL options to use. + * + * If error is non-null, it will be populated in case the context creation fails + * (returning a NULL). +*/ +redisSSLContext *redisCreateSSLContextWithOptions(redisSSLOptions *options, + redisSSLContextError *error); + +/** * Free a previously created OpenSSL context. */ void redisFreeSSLContext(redisSSLContext *redis_ssl_ctx); @@ -219,6 +219,25 @@ redisSSLContext *redisCreateSSLContext(const char *cacert_filename, const char * const char *cert_filename, const char *private_key_filename, const char *server_name, redisSSLContextError *error) { + redisSSLOptions options = { + .cacert_filename = cacert_filename, + .capath = capath, + .cert_filename = cert_filename, + .private_key_filename = private_key_filename, + .server_name = server_name, + .verify_mode = REDIS_SSL_VERIFY_PEER, + }; + + return redisCreateSSLContextWithOptions(&options, error); +} + +redisSSLContext *redisCreateSSLContextWithOptions(redisSSLOptions *options, redisSSLContextError *error) { + const char *cacert_filename = options->cacert_filename; + const char *capath = options->capath; + const char *cert_filename = options->cert_filename; + const char *private_key_filename = options->private_key_filename; + const char *server_name = options->server_name; + #ifdef _WIN32 HCERTSTORE win_store = NULL; PCCERT_CONTEXT win_ctx = NULL; @@ -235,7 +254,7 @@ redisSSLContext *redisCreateSSLContext(const char *cacert_filename, const char * } SSL_CTX_set_options(ctx->ssl_ctx, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3); - SSL_CTX_set_verify(ctx->ssl_ctx, SSL_VERIFY_PEER, NULL); + SSL_CTX_set_verify(ctx->ssl_ctx, options->verify_mode, NULL); if ((cert_filename != NULL && private_key_filename == NULL) || (private_key_filename != NULL && cert_filename == NULL)) { @@ -1907,7 +1907,9 @@ typedef enum astest_no ASTEST_CONNECT=0, ASTEST_CONN_TIMEOUT, ASTEST_PINGPONG, - ASTEST_PINGPONG_TIMEOUT + ASTEST_PINGPONG_TIMEOUT, + ASTEST_ISSUE_931, + ASTEST_ISSUE_931_PING }astest_no; /* a static context for the async tests */ @@ -1918,6 +1920,7 @@ struct _astest { int connects; int connect_status; int disconnects; + int pongs; int disconnect_status; int connected; int err; @@ -1941,7 +1944,9 @@ static void asCleanup(void* data) t->ac = NULL; } -static void connectCallback(const redisAsyncContext *c, int status) { +static void commandCallback(struct redisAsyncContext *ac, void* _reply, void* _privdata); + +static void connectCallback(redisAsyncContext *c, int status) { struct _astest *t = (struct _astest *)c->data; assert(t == &astest); assert(t->connects == 0); @@ -1950,6 +1955,15 @@ static void connectCallback(const redisAsyncContext *c, int status) { t->connects++; t->connect_status = status; t->connected = status == REDIS_OK ? 1 : -1; + + if (t->testno == ASTEST_ISSUE_931) { + /* disconnect again */ + redisAsyncDisconnect(c); + } + else if (t->testno == ASTEST_ISSUE_931_PING) + { + status = redisAsyncCommand(c, commandCallback, NULL, "PING"); + } } static void disconnectCallback(const redisAsyncContext *c, int status) { assert(c->data == (void*)&astest); @@ -1969,20 +1983,22 @@ static void commandCallback(struct redisAsyncContext *ac, void* _reply, void* _p (void)_privdata; t->err = ac->err; strcpy(t->errstr, ac->errstr); - if (t->testno == ASTEST_PINGPONG) + t->counter++; + if (t->testno == ASTEST_PINGPONG ||t->testno == ASTEST_ISSUE_931_PING) { - test_cond(reply != NULL && reply->type == REDIS_REPLY_STATUS && strcmp(reply->str, "PONG") == 0); + assert(reply != NULL && reply->type == REDIS_REPLY_STATUS && strcmp(reply->str, "PONG") == 0); + t->pongs++; redisAsyncFree(ac); } if (t->testno == ASTEST_PINGPONG_TIMEOUT) { /* two ping pongs */ assert(reply != NULL && reply->type == REDIS_REPLY_STATUS && strcmp(reply->str, "PONG") == 0); - if (++t->counter == 1) { + t->pongs++; + if (t->counter == 1) { int status = redisAsyncCommand(ac, commandCallback, NULL, "PING"); assert(status == REDIS_OK); } else { - test_cond(reply != NULL && reply->type == REDIS_REPLY_STATUS && strcmp(reply->str, "PONG") == 0); redisAsyncFree(ac); } } @@ -2089,6 +2105,7 @@ static void test_async_polling(struct config config) { assert(status == REDIS_OK); while(astest.ac) redisPollTick(c, 0.1); + test_cond(astest.pongs == 1); /* Test a ping/pong after connection that didn't time out. * see https://github.com/redis/hiredis/issues/945 @@ -2105,8 +2122,33 @@ static void test_async_polling(struct config config) { assert(status == REDIS_OK); while(astest.ac) redisPollTick(c, 0.1); + test_cond(astest.pongs == 2); config = defaultconfig; } + + /* Test disconnect from an on_connect callback + * see https://github.com/redis/hiredis/issues/931 + */ + test("Disconnect from onConnected callback (Issue #931): "); + c = do_aconnect(config, ASTEST_ISSUE_931); + while(astest.disconnects == 0) + redisPollTick(c, 0.1); + assert(astest.connected == 0); + assert(astest.connects == 1); + test_cond(astest.disconnects == 1); + + /* Test ping/pong from an on_connect callback + * see https://github.com/redis/hiredis/issues/931 + */ + test("Ping/Pong from onConnected callback (Issue #931): "); + c = do_aconnect(config, ASTEST_ISSUE_931_PING); + /* connect callback issues ping, reponse callback destroys context */ + while(astest.ac) + redisPollTick(c, 0.1); + assert(astest.connected == 0); + assert(astest.connects == 1); + assert(astest.disconnects == 1); + test_cond(astest.pongs == 1); } /* End of Async polling_adapter driven tests */ |