luci-app-firewall: update cbi models

 - allow multiple src/dest ips for rules (#1637)
 - restrict ICMP type list to ICMP protocol
 - add section title callbacks
 - remove size annotations
 - fix validation error with aliased zone fields (#1882)

Fixes #1637, #1882.
Signed-off-by: Jo-Philipp Wich <jo@mein.io>

4 files changed, 28 insertions, 17 deletions

diff --git a/applications/luci-app-firewall/luasrc/model/cbi/firewall/forwards.lua b/applications/luci-app-firewall/luasrc/model/cbi/firewall/forwards.lua
index 5485ebf09..5d1ffe091 100644
--- a/applications/luci-app-firewall/luasrc/model/cbi/firewall/forwards.lua
+++ b/applications/luci-app-firewall/luasrc/model/cbi/firewall/forwards.lua
@@ -63,8 +63,9 @@ function s.filter(self, sid)
 	return (self.map:get(sid, "target") ~= "SNAT")
 end
 
-
-ft.opt_name(s, DummyValue, translate("Name"))
+function s.sectiontitle(self, sid)
+	return (self.map:get(sid, "name") or translate("Unnamed forward"))
+end
 
 
 local function forward_proto_txt(self, s)
@@ -103,7 +104,6 @@ end
 
 match = s:option(DummyValue, "match", translate("Match"))
 match.rawhtml = true
-match.width   = "50%"
 function match.cfgvalue(self, s)
 	return "<small>%s<br />%s<br />%s</small>" % {
 		forward_proto_txt(self, s),
@@ -115,7 +115,6 @@ end
 
 dest = s:option(DummyValue, "dest", translate("Forward to"))
 dest.rawhtml = true
-dest.width   = "40%"
 function dest.cfgvalue(self, s)
 	local z = ft.fmt_zone(self.map:get(s, "dest"), translate("any zone"))
 	local a = ft.fmt_ip(self.map:get(s, "dest_ip"), translate("any host"))
@@ -129,6 +128,6 @@ function dest.cfgvalue(self, s)
 	end
 end
 
-ft.opt_enabled(s, Flag, translate("Enable")).width = "1%"
+ft.opt_enabled(s, Flag, translate("Enable"))
 
 return m
diff --git a/applications/luci-app-firewall/luasrc/model/cbi/firewall/rule-details.lua b/applications/luci-app-firewall/luasrc/model/cbi/firewall/rule-details.lua
index b57201041..a4763d5dc 100644
--- a/applications/luci-app-firewall/luasrc/model/cbi/firewall/rule-details.lua
+++ b/applications/luci-app-firewall/luasrc/model/cbi/firewall/rule-details.lua
@@ -224,7 +224,13 @@ else
 	end
 
 
-	o = s:option(DynamicList, "icmp_type", translate("Match ICMP type"))
+	o = s:option(DropDown, "icmp_type", translate("Match ICMP type"))
+	o.multiple = true
+	o.display = 10
+	o.dropdown = 10
+	o.custom = true
+	o.cast = "table"
+
 	o:value("", "any")
 	o:value("echo-reply")
 	o:value("destination-unreachable")
@@ -263,6 +269,8 @@ else
 	o:value("address-mask-request")
 	o:value("address-mask-reply")
 
+	o:depends("proto", "icmp")
+
 
 	o = s:option(Value, "src", translate("Source zone"))
 	o.nocreate = true
@@ -281,7 +289,7 @@ else
 
 
 	o = s:option(Value, "src_ip", translate("Source address"))
-	o.datatype = "neg(ipmask)"
+	o.datatype = "list(neg(ipmask))"
 	o.placeholder = translate("any")
 
 	luci.sys.net.ipv4_hints(function(ip, name)
@@ -301,7 +309,6 @@ else
 	o = s:option(Value, "dest_local", translate("Output zone"))
 	o.nocreate = true
 	o.allowany = true
-	o.rmempty = false
 	o.template = "cbi/firewall_zonelist"
 	o.alias = "dest"
 	o:depends("src", "")
@@ -316,7 +323,7 @@ else
 
 
 	o = s:option(Value, "dest_ip", translate("Destination address"))
-	o.datatype = "neg(ipmask)"
+	o.datatype = "list(neg(ipmask))"
 	o.placeholder = translate("any")
 
 	luci.sys.net.ipv4_hints(function(ip, name)
diff --git a/applications/luci-app-firewall/luasrc/model/cbi/firewall/rules.lua b/applications/luci-app-firewall/luasrc/model/cbi/firewall/rules.lua
index 22dec5ee6..f4b6b2a92 100644
--- a/applications/luci-app-firewall/luasrc/model/cbi/firewall/rules.lua
+++ b/applications/luci-app-firewall/luasrc/model/cbi/firewall/rules.lua
@@ -72,7 +72,9 @@ function s.parse(self, ...)
 	end
 end
 
-ft.opt_name(s, DummyValue, translate("Name"))
+function s.sectiontitle(self, sid)
+	return (self.map:get(sid, "name") or translate("Unnamed rule"))
+end
 
 local function rule_proto_txt(self, s)
 	local f = self.map:get(s, "family")
@@ -159,7 +161,6 @@ end
 
 match = s:option(DummyValue, "match", translate("Match"))
 match.rawhtml = true
-match.width   = "70%"
 function match.cfgvalue(self, s)
 	return "<small>%s<br />%s<br />%s</small>" % {
 		rule_proto_txt(self, s),
@@ -170,7 +171,6 @@ end
 
 target = s:option(DummyValue, "target", translate("Action"))
 target.rawhtml = true
-target.width   = "20%"
 function target.cfgvalue(self, s)
 	local t = ft.fmt_target(self.map:get(s, "target"), self.map:get(s, "src"), self.map:get(s, "dest"))
 	local l = ft.fmt_limit(self.map:get(s, "limit"),
@@ -183,7 +183,7 @@ function target.cfgvalue(self, s)
 	end
 end
 
-ft.opt_enabled(s, Flag, translate("Enable")).width = "1%"
+ft.opt_enabled(s, Flag, translate("Enable"))
 
 
 --
@@ -240,11 +240,12 @@ function s.filter(self, sid)
 	return (self.map:get(sid, "target") == "SNAT")
 end
 
-ft.opt_name(s, DummyValue, translate("Name"))
+function s.sectiontitle(self, sid)
+	return (self.map:get(sid, "name") or translate("Unnamed SNAT"))
+end
 
 match = s:option(DummyValue, "match", translate("Match"))
 match.rawhtml = true
-match.width   = "70%"
 function match.cfgvalue(self, s)
 	return "<small>%s<br />%s<br />%s</small>" % {
 		rule_proto_txt(self, s),
@@ -255,7 +256,6 @@ end
 
 snat = s:option(DummyValue, "via", translate("Action"))
 snat.rawhtml = true
-snat.width   = "20%"
 function snat.cfgvalue(self, s)
 	local a = ft.fmt_ip(self.map:get(s, "src_dip"))
 	local p = ft.fmt_port(self.map:get(s, "src_dport"))
@@ -267,7 +267,7 @@ function snat.cfgvalue(self, s)
 	end
 end
 
-ft.opt_enabled(s, Flag, translate("Enable")).width = "1%"
+ft.opt_enabled(s, Flag, translate("Enable"))
 
 
 return m
diff --git a/applications/luci-app-firewall/luasrc/model/cbi/firewall/zones.lua b/applications/luci-app-firewall/luasrc/model/cbi/firewall/zones.lua
index 2c5083a01..46402a8fc 100644
--- a/applications/luci-app-firewall/luasrc/model/cbi/firewall/zones.lua
+++ b/applications/luci-app-firewall/luasrc/model/cbi/firewall/zones.lua
@@ -62,6 +62,11 @@ s.anonymous = true
 s.addremove = true
 s.extedit   = ds.build_url("admin", "network", "firewall", "zones", "%s")
 
+function s.sectiontitle(self, sid)
+	local z = fw:get_zone(sid)
+	return z:name()
+end
+
 function s.create(self)
 	local z = fw:new_zone()
 	if z then