fix(dns): resolv.conf options timeout: 0 is ignored

### Summary The `options timeout: 0` has a specific meaning in `resolv.conf`. It means that the request will be sent to all nameservers without waiting and whoever answers first, will be accepted. In Kong the `options timeout: 0` cause actually all the DNS queries themselves to timeout. This is bad as some platforms tend to follow `options timeout: 0` as a good practice when having more than one resolver. Kong should in future support parallel thread based resolving from multiple resolvers, but first we need to get this fix to stop it causing issues. Signed-off-by: Aapo Talvensaari <[email protected]>
Kong · Feb 28, 2024 · 3796a6b · 3796a6b
1 parent 6ead302
commit 3796a6b
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 3 deletions.
diff --git a/changelog/unreleased/kong/fix-dns-resolv-timeout-zero.yml b/changelog/unreleased/kong/fix-dns-resolv-timeout-zero.yml
@@ -0,0 +1,3 @@
+message: "**DNS Client**: ignore a non-positive values on resolv.conf for options timeout, and use a default value of 2 seconds instead."
+type: bugfix
+scope: Core
diff --git a/kong/resty/dns/client.lua b/kong/resty/dns/client.lua
@@ -32,6 +32,7 @@ local log = ngx.log
 local ERR = ngx.ERR
 local WARN = ngx.WARN
 local ALERT = ngx.ALERT
+local NOTICE = ngx.NOTICE
 local DEBUG = ngx.DEBUG
 --[[
   DEBUG = ngx.WARN
@@ -54,6 +55,8 @@ local req_dyn_hook_run_hooks = req_dyn_hook.run_hooks
 
 local DOT   = string_byte(".")
 local COLON = string_byte(":")
+local DEFAULT_TIMEOUT = 2000 -- 2000 is openresty default
+
 
 local EMPTY = setmetatable({},
   {__newindex = function() error("The 'EMPTY' table is read-only") end})
@@ -621,10 +624,15 @@ _M.init = function(options)
     if resolv.options.timeout then
       options.timeout = resolv.options.timeout * 1000
     else
-      options.timeout = 2000  -- 2000 is openresty default
+      options.timeout = DEFAULT_TIMEOUT
     end
   end
-  log(DEBUG, PREFIX, "timeout = ", options.timeout, " ms")
+  if options.timeout > 0 then
+    log(DEBUG, PREFIX, "timeout = ", options.timeout, " ms")
+  else
+    log(NOTICE, PREFIX, "timeout = ", DEFAULT_TIMEOUT, " ms (a non-positive timeout of ", options.timeout, " configured - using default timeout)")
+    options.timeout = DEFAULT_TIMEOUT
+  end
 
   -- setup the search order
   options.ndots = options.ndots or resolv.options.ndots or 1

diff --git a/t/03-dns-client/00-sanity.t b/t/03-dns-client/00-sanity.t
@@ -2,7 +2,7 @@ use strict;
 use warnings FATAL => 'all';
 use Test::Nginx::Socket::Lua;
 
-plan tests => 2;
+plan tests => 5;
 
 run_tests();
 
@@ -25,3 +25,20 @@ GET /t
 --- response_body
 127.0.0.1
 --- no_error_log
+
+
+
+=== TEST 2: load lua-resty-dns-client
+--- config
+    location = /t {
+        access_by_lua_block {
+            local client = require("kong.resty.dns.client")
+            assert(client.init({ timeout = 0 }))
+            ngx.exit(200)
+        }
+    }
+--- request
+GET /t
+--- error_log
+[notice]
+timeout = 2000 ms (a non-positive timeout of 0 configured - using default timeout)