FS-9058 [mod_hiredis] fix rate counters so the keys expire after interval completes...

diff --git a/src/mod/applications/mod_hiredis/hiredis_profile.c b/src/mod/applications/mod_hiredis/hiredis_profile.c
index 333188f893b4f25f8e5b044f112e1338cd2c16ea..f4394ec0ccfa38b71636ccad96f043eb52aa9522 100644 (file)
--- a/src/mod/applications/mod_hiredis/hiredis_profile.c
+++ b/src/mod/applications/mod_hiredis/hiredis_profile.c
@@ -202,7 +202,9 @@ static hiredis_context_t *hiredis_profile_get_context(hiredis_profile_t *profile
 
 static switch_status_t hiredis_context_execute_sync(hiredis_context_t *context, const char *data, char **resp, switch_core_session_t *session)
 {
-       redisReply *response = redisCommand(context->context, data);
+       redisReply *response;
+       switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(session), SWITCH_LOG_DEBUG, "hiredis: %s\n", data);
+       response = redisCommand(context->context, data);
        if ( !response ) {
                *resp = NULL;
                return SWITCH_STATUS_GENERR;

diff --git a/src/mod/applications/mod_hiredis/mod_hiredis.c b/src/mod/applications/mod_hiredis/mod_hiredis.c
index e753192bba86a0d6db9f06c1591bd438e40df38b..9536521899f98b33151a3aa15e38e948878e4d5c 100644 (file)
--- a/src/mod/applications/mod_hiredis/mod_hiredis.c
+++ b/src/mod/applications/mod_hiredis/mod_hiredis.c
@@ -66,10 +66,10 @@ SWITCH_STANDARD_APP(raw_app)
        }
 
        if ( hiredis_profile_execute_sync(profile, cmd, &response, session) != SWITCH_STATUS_SUCCESS) {
-               switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(session), SWITCH_LOG_ERROR, "hiredis: profile[%s] error executing [%s] because [%s]\n", profile_name, cmd, response);
+               switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(session), SWITCH_LOG_ERROR, "hiredis: profile[%s] error executing [%s] because [%s]\n", profile_name, cmd, response ? response : "");
        }
 
-       switch_channel_set_variable(channel, "hiredis_raw_response", response);
+       switch_channel_set_variable(channel, "hiredis_raw_response", response ? response : "");
 
  done:
        switch_safe_free(profile_name);
@@ -108,7 +108,9 @@ SWITCH_STANDARD_API(raw_api)
                switch_goto_status(SWITCH_STATUS_GENERR, done);
        }
 
-       stream->write_function(stream, response);
+       if (response) {
+               stream->write_function(stream, response);
+       }
  done:
        switch_safe_free(input);
        switch_safe_free(response);
@@ -135,6 +137,11 @@ SWITCH_LIMIT_INCR(hiredis_limit_incr)
                switch_goto_status(SWITCH_STATUS_GENERR, done);
        }
 
+       if ( interval < 0 ) {
+               switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(session), SWITCH_LOG_ERROR, "hiredis: interval must be >= 0\n");
+               switch_goto_status(SWITCH_STATUS_GENERR, done);
+       }
+
        profile = switch_core_hash_find(mod_hiredis_globals.profiles, realm);
 
        if ( !profile ) {
@@ -160,7 +167,16 @@ SWITCH_LIMIT_INCR(hiredis_limit_incr)
                switch_goto_status(SWITCH_STATUS_GENERR, done);
        }
 
-       switch_channel_set_variable(channel, "hiredis_raw_response", response);
+       /* set expiration for interval on first increment */
+       if ( interval && !strcmp("1", response ? response : "") ) {
+               char *expire_response = NULL;
+               char *expire_cmd = switch_mprintf("expire %s %d", limit_key, interval);
+               hiredis_profile_execute_sync(profile, expire_cmd, &expire_response, session);
+               switch_safe_free(expire_cmd);
+               switch_safe_free(expire_response);
+       }
+
+       switch_channel_set_variable(channel, "hiredis_raw_response", response ? response : "");
 
        limit_pvt = switch_core_alloc(session_pool, sizeof(hiredis_limit_pvt_t));
        limit_pvt->next = switch_channel_get_private(channel, "hiredis_limit_pvt");
@@ -171,7 +187,7 @@ SWITCH_LIMIT_INCR(hiredis_limit_incr)
        limit_pvt->interval = interval;
        switch_channel_set_private(channel, "hiredis_limit_pvt", limit_pvt);
 
-       count = atoll(response);
+       count = atoll(response ? response : "");
 
        if ( !count || count > max ) {
                switch_goto_status(SWITCH_STATUS_GENERR, done);
@@ -200,17 +216,20 @@ SWITCH_LIMIT_RELEASE(hiredis_limit_release)
                hiredis_limit_pvt_t *tmp = limit_pvt;
 
                while (tmp) {
-                       profile = switch_core_hash_find(mod_hiredis_globals.profiles, limit_pvt->realm);
-                       hashkey = switch_mprintf("decr %s", tmp->limit_key);
-
-                       if ( hiredis_profile_execute_sync(profile, hashkey, &response, session) != SWITCH_STATUS_SUCCESS ) {
-                               switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(session), SWITCH_LOG_ERROR, "hiredis: profile[%s] error executing [%s] because [%s]\n",
-                                                                 tmp->realm, hashkey, response);
+                       /* Rate limited resources are not auto-decremented, they will expire. */
+                       if (!tmp->interval) {
+                               profile = switch_core_hash_find(mod_hiredis_globals.profiles, limit_pvt->realm);
+                               hashkey = switch_mprintf("decr %s", tmp->limit_key);
+
+                               if ( hiredis_profile_execute_sync(profile, hashkey, &response, session) != SWITCH_STATUS_SUCCESS ) {
+                                       switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(session), SWITCH_LOG_ERROR, "hiredis: profile[%s] error executing [%s] because [%s]\n",
+                                                                         tmp->realm, hashkey, response ? response : "");
+                               }
+
+                               switch_safe_free(response);
+                               switch_safe_free(hashkey);
                        }
-
                        tmp = tmp->next;
-                       switch_safe_free(response);
-                       switch_safe_free(hashkey);
                }
        } else {
                profile = switch_core_hash_find(mod_hiredis_globals.profiles, limit_pvt->realm);
@@ -222,12 +241,12 @@ SWITCH_LIMIT_RELEASE(hiredis_limit_release)
                                switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(session), SWITCH_LOG_INFO, "hiredis: ignoring profile[%s] connection error executing [%s]\n", realm, hashkey);
                                switch_goto_status(SWITCH_STATUS_SUCCESS, done);
                        }
-                       switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(session), SWITCH_LOG_ERROR, "hiredis: profile[%s] error executing [%s] because [%s]\n", realm, hashkey, response);
-                       switch_channel_set_variable(channel, "hiredis_raw_response", response);
+                       switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(session), SWITCH_LOG_ERROR, "hiredis: profile[%s] error executing [%s] because [%s]\n", realm, hashkey, response ? response : "");
+                       switch_channel_set_variable(channel, "hiredis_raw_response", response ? response : "");
                        switch_goto_status(SWITCH_STATUS_GENERR, done);
                }
 
-               switch_channel_set_variable(channel, "hiredis_raw_response", response);
+               switch_channel_set_variable(channel, "hiredis_raw_response", response ? response : "");
        }
 
  done:
@@ -258,11 +277,11 @@ SWITCH_LIMIT_USAGE(hiredis_limit_usage)
        hashkey = switch_mprintf("get %s", resource);
 
        if ( hiredis_profile_execute_sync(profile, hashkey, &response, NULL) != SWITCH_STATUS_SUCCESS) {
-               switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_ERROR, "hiredis: profile[%s] error executing [%s] because [%s]\n", realm, hashkey, response);
+               switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_ERROR, "hiredis: profile[%s] error executing [%s] because [%s]\n", realm, hashkey, response ? response : "");
                goto err;
        }
 
-       count = atoll(response);
+       count = atoll(response ? response : "");
 
        switch_safe_free(response);
        switch_safe_free(hashkey);
@@ -290,6 +309,7 @@ SWITCH_LIMIT_RESET(hiredis_limit_reset)
 */
 SWITCH_LIMIT_INTERVAL_RESET(hiredis_limit_interval_reset)
 {
+       /* TODO this doesn't work since the key has the interval in it */
        hiredis_profile_t *profile = switch_core_hash_find(mod_hiredis_globals.profiles, realm);
        switch_status_t status = SWITCH_STATUS_SUCCESS;
        char *hashkey = NULL, *response = NULL;
@@ -307,7 +327,7 @@ SWITCH_LIMIT_INTERVAL_RESET(hiredis_limit_interval_reset)
        hashkey = switch_mprintf("set %s 0", resource);
 
        if ( hiredis_profile_execute_sync(profile, hashkey, &response, NULL) != SWITCH_STATUS_SUCCESS) {
-               switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_ERROR, "hiredis: profile[%s] error executing [%s] because [%s]\n", realm, hashkey, response);
+               switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_ERROR, "hiredis: profile[%s] error executing [%s] because [%s]\n", realm, hashkey, response ? response : "");
                switch_goto_status(SWITCH_STATUS_GENERR, done);
        }