@henningw commented on this pull request.

Thanks for the PR. I have added a few comments related to white space changes and variable declaration. Please remove the debugging comment regarding the password, its not a good security practice to log it.
For merging this to our code base, you also need to add documentation to the modules. This is done by extending the XML file in the doc directory. They are are used to create the README and web site documentation.


In src/modules/db_redis/redis_connection.c:

>      LM_DBG("connecting to redis cluster at %.*s\n", con->id->url.len, con->id->url.s);
     host_begin = strstr(con->id->url.s, "redis://");
     if (host_begin) {
         host_begin += 8;
-    } else {
-        LM_ERR("invalid url scheme\n");
+    } 
+    
+    if (db_redis_opt_ssl != 0) {
+        /* Create SSL context*/
+	redisInitOpenSSL();
+	ssl = redisCreateSSLContext(NULL, NULL, NULL, NULL, NULL, NULL);
+	if (ssl == NULL) {
+	    LM_ERR("Unable to create Redis SSL Context.\n");

It seems indention here is wrong, please adapt it for the goto and the one bracket.


In src/modules/db_redis/redis_connection.c:

> @@ -137,14 +140,23 @@ int db_redis_connect(km_redis_con_t *con) {
     char hosts[MAX_URL_LENGTH];
     char* host_begin;
     char* host_end;
+    redisSSLContext *ssl = NULL;

consider moving this parameter definition to the beginning of the function, as its needed in #if and #else


In src/modules/db_redis/redis_connection.c:

>      status = redisClusterConnect2(con->con);
     if (status != REDIS_OK) {
         LM_ERR("cannot open connection to cluster with hosts: %s, error: %s\n", hosts, con->con->errstr);
         goto err;
     }
 #else
+    redisSSLContext *ssl = NULL;

move to beginning, as before


In src/modules/db_redis/redis_connection.c:

>  #endif
 
-    if (con->id->password) {
-        reply = redisCommand(con->con, "AUTH %s", con->id->password);
+    password = con->id->password;
+    if (!password) {
+        password = db_pass;
+    }
+    if (password) {
+        LM_DBG("Using password %s\n", password);

Please not log the password to the system log, this should be removed.


In src/modules/ndb_redis/redis_client.h:

> @@ -72,6 +74,7 @@ typedef struct redisc_server {
 	param_t *attrs;
 	char *spec;
 	redisContext *ctxRedis;
+    redisSSLContext *sslCtxRedis;

Fix the indention


Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <kamailio/kamailio/pull/3345/review/1272258860@github.com>