view: change :addr to a more natural semantics

Continue executing :addr rules until a non-chain action is executed.
Before this, the only the first match in view:addr rules got a chance,
even though the inner policy rule might not trigger in that case
or be a chain action.

diff --git a/NEWS b/NEWS
index 08cc2969dff6f562c6450e653343b83a252c9e7b..1c2c42b6974d79ac016e9bded669d11370d1440b 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -1,6 +1,11 @@
 Knot Resolver 3.x.y (201y-mm-dd)
 ================================
 
+Incompatible changes
+--------------------
+- view module: keep trying rules until a "non-chain" action is executed.
+  This improves consistency with the policy module.
+
 New features
 ------------
 - module edns_keepalive to implement server side of RFC 7828 (#408)

diff --git a/modules/view/README.rst b/modules/view/README.rst
index b29c52d63bb7f7fd617afc8bd6bc9a2c3660c395..c960daae2c5d9b1088434e6c247f7aeefacd31e0 100644 (file)
--- a/modules/view/README.rst
+++ b/modules/view/README.rst
@@ -15,6 +15,8 @@ There are two identification mechanisms:
   - identifies the client based on a TSIG key
 
 You can combine this information with :ref:`policy <mod-policy>` rules.
+Just as with policies, the rules for a request get tried until one "non-chain"
+action is executed.
 
 .. code-block:: lua
 

diff --git a/modules/view/view.lua b/modules/view/view.lua
index deb4f3b96b48c10aa47bcd9da4a7bf244f6e1ab2..9dfa91902edea9f24bc0d60c370ced3d11701287 100644 (file)
--- a/modules/view/view.lua
+++ b/modules/view/view.lua
@@ -29,31 +29,46 @@ local function match_subnet(family, subnet, bitlen, addr)
        return (family == addr:family()) and (C.kr_bitcmp(subnet, addr:ip(), bitlen) == 0)
 end
 
--- @function Find view for given request
-local function evaluate(_, req)
+-- @function Execute a policy callback (may be nil);
+-- return boolean: whether to continue trying further rules.
+local function execute(state, req, match_cb)
+       if match_cb == nil then return false end
+       local action = match_cb(req, req:current())
+       if action == nil then return false end
+       local next_state = action(state, req)
+       if next_state then    -- Not a chain rule,
+               req.state = next_state
+               return true
+       else
+               return false
+       end
+end
+
+-- @function Try all the rules in order, until a non-chain rule gets executed.
+local function evaluate(state, req)
+       -- Try :tsig first.
        local client_key = req.qsource.packet.tsig_rr
        local match_cb = (client_key ~= nil) and view.key[client_key:owner()] or nil
-       -- Search subnets otherwise
-       if match_cb == nil then
-               if req.qsource.addr ~= nil then
-                       for i = 1, #view.src do
-                               local pair = view.src[i]
-                               if match_subnet(pair[1], pair[2], pair[3], req.qsource.addr) then
-                                       match_cb = pair[4]
-                                       break
-                               end
+       if execute(state, req, match_cb) then return end
+       -- Then try :addr by the source.
+       if req.qsource.addr ~= nil then
+               for i = 1, #view.src do
+                       local pair = view.src[i]
+                       if match_subnet(pair[1], pair[2], pair[3], req.qsource.addr) then
+                               match_cb = pair[4]
+                               if execute(state, req, match_cb) then return end
                        end
-               elseif req.qsource.dst_addr ~= nil then
-                       for i = 1, #view.dst do
-                               local pair = view.dst[i]
-                               if match_subnet(pair[1], pair[2], pair[3], req.qsource.dst_addr) then
-                                       match_cb = pair[4]
-                                       break
-                               end
+               end
+       -- Finally try :addr by the destination.
+       elseif req.qsource.dst_addr ~= nil then
+               for i = 1, #view.dst do
+                       local pair = view.dst[i]
+                       if match_subnet(pair[1], pair[2], pair[3], req.qsource.dst_addr) then
+                               match_cb = pair[4]
+                               if execute(state, req, match_cb) then return end
                        end
                end
        end
-       return match_cb
 end
 
 -- @function Return policy based on source address
@@ -87,17 +102,8 @@ view.layer = {
        begin = function(state, req)
                if state == kres.FAIL then return state end
                req = kres.request_t(req)
-               local match_cb = evaluate(view, req)
-               if match_cb ~= nil then
-                       local action = match_cb(req, req:current())
-                       if action then
-                               local next_state = action(state, req)
-                               if next_state then    -- Not a chain rule,
-                                       return next_state -- stop on first match
-                               end
-                       end
-               end
-               return state
+               evaluate(state, req)
+               return req.state
        end
 }