Merge pull request #57 from yast/whitespace-named-conf-sp1

Various fixes in named.conf: SP1

package/yast2-dns-server.changes

@@ -1,3 +1,17 @@
+-------------------------------------------------------------------
+Wed May 25 11:11:30 UTC 2016 - mvidner@suse.com
+
+- Keep non-ACL items in allow-transfer AKA Enable Zone Transport
+  (bsc#976643#c23)
+- Fix parsing 'keyword{value;};' (no spaces) in named.conf
+  (bsc#976643#c16).
+- 3.1.17.1
+
+-------------------------------------------------------------------
+Wed May  4 08:45:09 UTC 2016 - cwh@suse.com
+
+- Fix handling of trailing spaces in named.conf (bsc#976643)
+
 -------------------------------------------------------------------
 Fri Oct  2 11:09:19 UTC 2015 - ancor@suse.com
 

package/yast2-dns-server.spec

@@ -17,7 +17,7 @@
 
 
 Name:           yast2-dns-server
-Version:        3.1.17
+Version:        3.1.17.1
 Release:        0
 Url:            https://github.com/yast/yast-dns-server
 

src/include/dns-server/dialog-masterzone.rb

@@ -58,31 +58,29 @@ def initialize_dns_server_dialog_masterzone(include_target)
       @current_zone_forwarders = []
     end
 
+    BUILTIN_ACLS = ["any", "none", "localhost", "localnets"]
+
+    # ACL names to present in a multiselection box
+    def acl_names
+      acls = DnsServer.GetAcl
+      names = acls.map do |a|
+        a.strip.split(/[ \t]/).fetch(0, "")
+      end
+      # bsc#976643#c23
+      names = (names + current_zone_allow_transfer + BUILTIN_ACLS).sort.uniq
+      # bug #203910
+      # hide "none" from listed ACLs
+      # "none" means, not allowed and thus multiselectbox of ACLs is disabled
+      names.find_all {|a| a != "none"}
+    end
+
     # Dialog Tab - Zone Editor - Basics
     # @return [Yast::Term] for Get_ZoneEditorTab()
     def GetMasterZoneEditorTabBasics
       updater_keys_m = DnsTsigKeys.ListTSIGKeys
       updater_keys = Builtins.maplist(updater_keys_m) do |m|
         Ops.get_string(m, "key", "")
       end
-      acl = DnsServer.GetAcl
-      acl = Builtins.maplist(acl) do |a|
-        while Builtins.substring(a, 0, 1) == " " ||
-            Builtins.substring(a, 0, 1) == "\t"
-          a = Builtins.substring(a, 1)
-        end
-        s = Builtins.splitstring(a, " \t")
-        type = Ops.get(s, 0, "")
-        type
-      end
-      acl = Builtins.filter(acl) { |a| a != "" }
-      acl = Convert.convert(
-        Builtins.sort(
-          Builtins.merge(acl, ["any", "none", "localhost", "localnets"])
-        ),
-        :from => "list",
-        :to   => "list <string>"
-      )
 
       expert_settings = Empty()
       if DnsServer.ExpertUI
@@ -114,11 +112,6 @@ def GetMasterZoneEditorTabBasics
         )
       end
 
-      # bug #203910
-      # hide "none" from listed ACLs
-      # "none" means, not allowed and thus multiselectbox of ACLs is disabled
-      acl = Builtins.filter(acl) { |one_acl| one_acl != "none" }
-
       @available_zones_to_connect = []
       zone_name = ""
       zones_to_connect = Builtins.maplist(@zones) do |z|
@@ -160,7 +153,7 @@ def GetMasterZoneEditorTabBasics
             # multi selection box
             VSquash(
               HSquash(
-                MinWidth(30, MultiSelectionBox(Id("acls_list"), _("ACLs"), acl))
+                MinWidth(30, MultiSelectionBox(Id("acls_list"), _("ACLs"), acl_names))
               )
             )
           )
@@ -207,37 +200,35 @@ def GetMasterZoneEditorTabBasics
       deep_copy(contents)
     end
 
-    def ZoneAclInit
-      allowed = false
-      keys = []
-      Builtins.foreach(Ops.get_list(@current_zone, "options", [])) do |m|
-        if Ops.get_string(m, "key", "") == "allow-transfer" && !allowed
-          key = Builtins.regexpsub(
-            Ops.get_string(m, "value", ""),
-            "^.*\\{[ \t]*(.*)[ \t]*\\}.*$",
-            "\\1"
-          )
-          if key != nil
-            keys = Builtins.splitstring(key, " ;")
-            keys = Builtins.filter(keys) { |k| k != "" }
-            allowed = true
-          end
-        end
+    # @return [Array<String>]
+    def current_zone_allow_transfer
+      target_pair = @current_zone.fetch("options", []).find do |m|
+        m["key"] == "allow-transfer"
       end
+      return [] unless target_pair
+
+      value = target_pair["value"] || ""
+      value = value[/\A.*\{[ \t]*(.*)[ \t]*\}.*\z/, 1]
+      return [] unless value
+
+      value.split(/[ \t;]/).reject(&:empty?)
+    end
+
+    def ZoneAclInit
+      keys = current_zone_allow_transfer
 
       # bug #203910
       # no keys in allow-transfer means that transfer is allowed for all
       # explicitly say that
-      if Builtins.size(keys) == 0
-        allowed = true
+      if keys.empty?
         keys = ["any"] 
         # the only way how to disable the transfer is to set "allow-transfer { none; };"
         # "none" must be alone, remove it from the list, it is not present in the multi-sel box
-      elsif Builtins.size(keys) == 1 && keys == ["none"]
-        allowed = false
+      elsif keys == ["none"]
         keys = []
       end
 
+      allowed = !keys.empty?
       UI.ChangeWidget(Id("enable_zone_transport"), :Value, allowed)
       UI.ChangeWidget(Id("acls_list"), :Enabled, allowed)
       UI.ChangeWidget(Id("acls_list"), :SelectedItems, keys) if allowed
@@ -2679,23 +2670,6 @@ def runMasterZoneTabDialog
     # Dialog Zone Editor - Slave
     # @return [Object] dialog result for wizard
     def runSlaveZoneTabDialog
-      acl = Builtins.maplist(DnsServer.GetAcl) do |acl_record|
-        acl_splitted = Builtins.splitstring(acl_record, " \t")
-        Ops.get(acl_splitted, 0, "")
-      end
-      acl = Convert.convert(
-        Builtins.sort(
-          Builtins.merge(acl, ["any", "none", "localhost", "localnets"])
-        ),
-        :from => "list",
-        :to   => "list <string>"
-      )
-
-      # bug #203910
-      # hide "none" from listed ACLs
-      # "none" means, not allowed and thus multiselectbox of ACLs is disabled
-      acl = Builtins.filter(acl) { |one_acl| one_acl != "none" }
-
       zone_name = Ops.get_string(@current_zone, "zone", "")
       contents = VBox(
         HBox(
@@ -2726,7 +2700,7 @@ def runSlaveZoneTabDialog
           )
         ),
         # multi selection box
-        VSquash(MultiSelectionBox(Id("acls_list"), _("ACLs"), acl)),
+        VSquash(MultiSelectionBox(Id("acls_list"), _("ACLs"), acl_names)),
         VStretch()
       )
 

src/modules/DnsData.pm

@@ -32,6 +32,23 @@ our $chroot = 0;
 
 my @allowed_interfaces = ();
 
+=head3 @zones
+
+See also L</%current_zone>
+
+One zone is a hash with this content:
+
+=over
+
+=item - "modified" -> boolean
+
+=item - "options" -> list of hashes {"key" => ..., "value" => ...}
+
+=item - ...
+
+=back
+
+=cut
 my @zones = ();
 
 my @options = ();
@@ -50,8 +67,24 @@ my $save_all = 0;
 
 my @files_to_delete = ();
 
+=head3 %current_zone
+
+A copy of an item from @zones
+
+Get it with SelectZone(integer); see also FindZone(string) -> integer
+
+Put it back with StoreZone() -> true
+
+=cut
 my %current_zone = ();
 
+=head3 $current_zone_index
+
+SelectZone sets this
+
+StoreZone will append a zone if this is -1
+
+=cut
 my $current_zone_index = -1;
 
 my $adapt_firewall = 0;

src/modules/DnsServer.pm

@@ -571,6 +571,8 @@ sub StoreZone {
 }
 
 BEGIN { $TYPEINFO{FindZone} = ["function", "integer", "string"]; }
+# Find zone by name ("zone" key)
+# Return -1 if not found
 sub FindZone {
     my $self = shift;
     my $zone_name = shift;
@@ -611,6 +613,10 @@ sub RemoveZone {
 }
 
 BEGIN { $TYPEINFO{SelectZone} = ["function", "boolean", "integer"]; }
+# SelectZone(index) copies that zone to %current_zone
+# SelectZone(-1) is valid, initializes a default %current_zone,
+#   later StoreZone will append it to the @zones list
+# The index is stored to $current_zone_index
 sub SelectZone {
     my $self = shift;
     my $zone_index = shift;
@@ -773,14 +779,17 @@ sub GetAllowedInterfaces {
 
     return \@allowed_interfaces;
 }
+
 BEGIN {$TYPEINFO{FetchCurrentZone} = [ "function", ["map", "string", "any"] ]; }
+# read %current_zone
 sub FetchCurrentZone {
     my $self = shift;
 
     return \%current_zone;
 }
 
 BEGIN {$TYPEINFO{StoreCurrentZone} = [ "function", "boolean", ["map", "string", "any"] ]; }
+# write %current_zone
 sub StoreCurrentZone {
     my $self = shift;
     %current_zone = %{+shift};

src/scrconf/dns_named.scr

@@ -22,7 +22,7 @@
 	    "comments"	: [ "^[ \t]*#.*$", "^[ \t]*$" ],
 	    "params"    : [
 		$[
-		    "match" : [ "([^ \t]+)[ \t]([^ \t]+(.*[^ \t]+)+)[ \t]*;$", "%s %s;" ],
+		    "match" : [ "([^ \t{]+)[ \t]*([^ \t]+(.*[^ \t]+)+)[ \t]*;[ \t]*$", "%s %s;" ],
 		    // This enables multiline parameters
 		    "multiline" : [ "([^ \t]+)[ \t]+([^}]*)", "([^}]*\\}[ \t]*;)[ \t]*" ],
 		],

test/Makefile.am

@@ -1,5 +1,8 @@
 TESTS = \
-  dns_server_test.rb
+  dns_server_test.rb \
+  dns_server_ui_test.rb \
+  etc_named_parsing_test.rb \
+  masterzone_test.rb
 
 TEST_EXTENSIONS = .rb
 RB_LOG_COMPILER = rspec

test/data/braces-without-space/etc/named.conf

@@ -0,0 +1,12 @@
+zone "localhost" in {
+	type master;
+	file "localhost.zone";
+        allow-update {none;};
+};
+
+zone "0.0.127.in-addr.arpa" in {
+	type master;
+	file "127.0.0.zone";
+        # here a space is missing which caused trouble in bsc#976643 (c16)
+        allow-update{none;};
+};

test/data/braces-without-space/etc/named.conf.json

@@ -0,0 +1,69 @@
+{
+  "comment": "",
+  "file": -1,
+  "kind": "section",
+  "name": "",
+  "type": -1,
+  "value": [
+    {
+      "comment": "",
+      "file": -1,
+      "kind": "section",
+      "name": "zone \"localhost\" in",
+      "type": 0,
+      "value": [
+        {
+          "comment": "",
+          "kind": "value",
+          "name": "type",
+          "type": 0,
+          "value": "master"
+        },
+        {
+          "comment": "",
+          "kind": "value",
+          "name": "file",
+          "type": 0,
+          "value": "\"localhost.zone\""
+        },
+        {
+          "comment": "",
+          "kind": "value",
+          "name": "allow-update",
+          "type": 0,
+          "value": "{none;}"
+        }
+      ]
+    },
+    {
+      "comment": "\n",
+      "file": -1,
+      "kind": "section",
+      "name": "zone \"0.0.127.in-addr.arpa\" in",
+      "type": 0,
+      "value": [
+        {
+          "comment": "",
+          "kind": "value",
+          "name": "type",
+          "type": 0,
+          "value": "master"
+        },
+        {
+          "comment": "",
+          "kind": "value",
+          "name": "file",
+          "type": 0,
+          "value": "\"127.0.0.zone\""
+        },
+        {
+          "comment": "        # here a space is missing which caused trouble in bsc#976643 (c16)\n",
+          "kind": "value",
+          "name": "allow-update",
+          "type": 0,
+          "value": "{none;}"
+        }
+      ]
+    }
+  ]
+}