[Feature] Improve url redirector module

- Use HEAD instead of GET
- Limit body length
- Check for cached results before testing them on HTTP
- Really limit number of nested queries

diff --git a/contrib/http-parser/http_parser.h b/contrib/http-parser/http_parser.h
index 7929315885f4cfbf93592a4e478948a41bcf8039..e2a0b4985f28709d3100b07b959f1f5ab344c09c 100644 (file)
--- a/contrib/http-parser/http_parser.h
+++ b/contrib/http-parser/http_parser.h
@@ -113,6 +113,7 @@ typedef int (*http_cb) (http_parser*);
   /* SPAMC compatibility */                    \
   XX(26, SYMBOLS,     SYMBOLS)      \
   XX(27, CHECK,       CHECK)        \
+  XX(28, METHOD_MAX,       METHOD_MAX)        \
   XX(-1, INVALID,     INVALID)      \
 
 enum http_method

diff --git a/src/libserver/redis_pool.c b/src/libserver/redis_pool.c
index 6aa4b6565265e5f568f584cf64673433311d42ea..5cb8986486d6b294446442d2aa3842e3e3d89a27 100644 (file)
--- a/src/libserver/redis_pool.c
+++ b/src/libserver/redis_pool.c
@@ -251,6 +251,7 @@ rspamd_redis_pool_new_connection (struct rspamd_redis_pool *pool,
        if (ctx) {
 
                if (ctx->err != REDIS_OK) {
+                       msg_err ("cannot connect to redis: %s", ctx->errstr);
                        redisAsyncFree (ctx);
 
                        return NULL;

diff --git a/src/plugins/lua/url_redirector.lua b/src/plugins/lua/url_redirector.lua
index 7c42a3fe3748218bfe8059ff3f334f2a02b5ea04..c4c6823eee3c02ee5547be12a42dc3be1f12ea8a 100644 (file)
--- a/src/plugins/lua/url_redirector.lua
+++ b/src/plugins/lua/url_redirector.lua
@@ -22,6 +22,7 @@ local settings = {
   --proxy = "http://example.com:3128", -- Send request through proxy
   key_prefix = 'rdr:', -- default hash name
   check_ssl = false, -- check ssl certificates
+  max_size = 10 * 1024, -- maximum body to process
 }
 
 local rspamd_logger = require "rspamd_logger"
@@ -49,55 +50,55 @@ local function cache_url(task, orig_url, url, key, param)
   end
 end
 
-local function resolve_url(task, orig_url, url, key, param, ntries)
-  if ntries > settings.nested_limit then
-    -- We cannot resolve more, stop
-    rspamd_logger.infox(task, 'cannot get more requests to resolve %s, stop on %s after %s attempts',
+local function resolve_cached(task, orig_url, url, key, param, ntries)
+  local function resolve_url(task, orig_url, url, key, param, ntries)
+    if ntries > settings.nested_limit then
+      -- We cannot resolve more, stop
+      rspamd_logger.infox(task, 'cannot get more requests to resolve %s, stop on %s after %s attempts',
         orig_url, url, ntries)
-    cache_url(task, orig_url, url, key, param)
-  end
-
-  local function http_callback(err, code, body, headers)
-    if err then
-      rspamd_logger.infox(task, 'found redirect error from %s to %s, err message: %s',
-            orig_url, url, err)
       cache_url(task, orig_url, url, key, param)
-    else
-      if code == 200 then
-        rspamd_logger.infox(task, 'found redirect from %s to %s, err code 200',
-          orig_url, url)
+
+      return
+    end
+
+    local function http_callback(err, code, body, headers)
+      if err then
+        rspamd_logger.infox(task, 'found redirect error from %s to %s, err message: %s',
+          orig_url, url, err)
         cache_url(task, orig_url, url, key, param)
-      elseif code == 301 or code == 302 then
-        local loc = headers['Location']
-        rspamd_logger.infox(task, 'found redirect from %s to %s, err code 200',
-          orig_url, loc)
-        if loc then
-          resolve_url(task, orig_url, loc, key, param, ntries + 1)
+      else
+        if code == 200 then
+          rspamd_logger.infox(task, 'found redirect from %s to %s, err code 200',
+            orig_url, url)
+          cache_url(task, orig_url, url, key, param)
+        elseif code == 301 or code == 302 then
+          local loc = headers['Location']
+          rspamd_logger.infox(task, 'found redirect from %s to %s, err code 200',
+            orig_url, loc)
+          if loc then
+            resolve_cached(task, orig_url, loc, key, param, ntries + 1)
+          else
+            cache_url(task, orig_url, url, key, param)
+          end
         else
+          rspamd_logger.infox(task, 'found redirect error from %s to %s, err code: %s',
+            orig_url, url, code)
           cache_url(task, orig_url, url, key, param)
         end
-      else
-        rspamd_logger.infox(task, 'found redirect error from %s to %s, err code: %s',
-            orig_url, url, code)
-        cache_url(task, orig_url, url, key, param)
       end
     end
-  end
-
-  rspamd_http.request{
-    url = url,
-    task = task,
-    timeout = settings.timeout,
-    opaque_body = true,
-    no_ssl_verify = not settings.check_ssl,
-    callback = http_callback
-  }
-end
-
-local function url_redirector_handler(task, url, param)
-  local url_str = tostring(url)
-  local key = settings.key_prefix .. hash.create(url_str):base32()
 
+    rspamd_http.request{
+      url = url,
+      task = task,
+      method = 'head',
+      max_size = settings.max_size,
+      timeout = settings.timeout,
+      opaque_body = true,
+      no_ssl_verify = not settings.check_ssl,
+      callback = http_callback
+    }
+  end
   local function redis_get_cb(err, data)
     if not err then
       if type(data) == 'string' then
@@ -114,21 +115,26 @@ local function url_redirector_handler(task, url, param)
       if nerr then
         rspamd_logger.errx(task, 'got error while setting redirect keys: %s', nerr)
       elseif ndata == 1 then
-        resolve_url(task, url_str, url_str, key, param, 1)
+        resolve_url(task, url, url, key, param, ntries)
       end
     end
 
-    local ret = rspamd_redis_make_request(task,
-      redis_params, -- connect params
-      key, -- hash key
-      true, -- is write
-      redis_reserve_cb, --callback
-      'SETNX', -- command
-      {key, 'processing'} -- arguments
-    )
-    if not ret then
-      rspamd_logger.errx(task, 'Couldnt schedule SETNX')
+    if orig_url == url then
+      local ret = rspamd_redis_make_request(task,
+        redis_params, -- connect params
+        key, -- hash key
+        true, -- is write
+        redis_reserve_cb, --callback
+        'SETNX', -- command
+        {key, 'processing'} -- arguments
+      )
+      if not ret then
+        rspamd_logger.errx(task, 'Couldn\'t schedule SETNX')
+      end
+    else
+      resolve_url(task, orig_url, url, key, param, ntries)
     end
+
   end
   local ret = rspamd_redis_make_request(task,
     redis_params, -- connect params
@@ -143,6 +149,12 @@ local function url_redirector_handler(task, url, param)
   end
 end
 
+local function url_redirector_handler(task, url, param)
+  local url_str = tostring(url)
+  local key = settings.key_prefix .. hash.create(url_str):base32()
+  resolve_cached(task, url_str, url_str, key, param, 1)
+end
+
 local opts =  rspamd_config:get_all_opt('url_redirector')
 if opts then
   for k,v in pairs(opts) do