From 1bb7556868843ef2d212f7ae932fec9050ee5ab5 Mon Sep 17 00:00:00 2001
From: Franco Fichtner <franco@opnsense.org>
Date: Fri, 18 Aug 2023 08:47:13 +0200
Subject: [PATCH] dns/ddclient: fix trailing comma issue with disabled accounts

selectattr seems like a good way to fix this.  While here reformat the JSON
and fix an error with empty stats causing a number of API errors being thrown.
---
 .../DynDNS/FieldTypes/AccountField.php        | 48 +++++++-------
 .../templates/OPNsense/ddclient/ddclient.json | 62 +++++++++----------
 2 files changed, 54 insertions(+), 56 deletions(-)

diff --git a/dns/ddclient/src/opnsense/mvc/app/models/OPNsense/DynDNS/FieldTypes/AccountField.php b/dns/ddclient/src/opnsense/mvc/app/models/OPNsense/DynDNS/FieldTypes/AccountField.php
index ce279e626..32ec0c854 100644
--- a/dns/ddclient/src/opnsense/mvc/app/models/OPNsense/DynDNS/FieldTypes/AccountField.php
+++ b/dns/ddclient/src/opnsense/mvc/app/models/OPNsense/DynDNS/FieldTypes/AccountField.php
@@ -1,31 +1,29 @@
 <?php
 
-/**
- *    Copyright (C) 2022 Deciso B.V.
+/*
+ * Copyright (C) 2022 Deciso B.V.
+ * All rights reserved.
  *
- *    All rights reserved.
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
  *
- *    Redistribution and use in source and binary forms, with or without
- *    modification, are permitted provided that the following conditions are met:
+ * 1. Redistributions of source code must retain the above copyright notice,
+ *    this list of conditions and the following disclaimer.
  *
- *    1. Redistributions of source code must retain the above copyright notice,
- *       this list of conditions and the following disclaimer.
- *
- *    2. Redistributions in binary form must reproduce the above copyright
- *       notice, this list of conditions and the following disclaimer in the
- *       documentation and/or other materials provided with the distribution.
- *
- *    THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES,
- *    INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
- *    AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
- *    AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY,
- *    OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- *    SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
- *    INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
- *    CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- *    ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
- *    POSSIBILITY OF SUCH DAMAGE.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
  *
+ * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
+ * AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY,
+ * OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
  */
 
 namespace OPNsense\DynDNS\FieldTypes;
@@ -49,8 +47,10 @@ class AccountField extends ArrayField
         $current_mtime->setInternalIsVirtual();
         if (isset(self::$current_stats[$node->getAttribute('uuid')])) {
             $stats = self::$current_stats[$node->getAttribute('uuid')];
-            $current_ip->setValue($stats['ip']);
-            $current_mtime->setValue(date('c', (int)$stats['mtime']));
+            if (!empty($stats)) {
+                $current_ip->setValue($stats['ip']);
+                $current_mtime->setValue(date('c', (int)$stats['mtime']));
+            }
         } elseif (!empty((string)$node->hostnames)) {
             foreach (explode(",", (string)$node->hostnames) as $hostname) {
                 if (!empty(self::$current_stats[$hostname]) && !empty(self::$current_stats[$hostname]['ip'])) {
diff --git a/dns/ddclient/src/opnsense/service/templates/OPNsense/ddclient/ddclient.json b/dns/ddclient/src/opnsense/service/templates/OPNsense/ddclient/ddclient.json
index 97895c809..64e0ec7bc 100644
--- a/dns/ddclient/src/opnsense/service/templates/OPNsense/ddclient/ddclient.json
+++ b/dns/ddclient/src/opnsense/service/templates/OPNsense/ddclient/ddclient.json
@@ -1,35 +1,33 @@
 {% from 'OPNsense/Macros/interface.macro' import physical_interface %}
 {
-  "general": {
-      "enabled": {{ "true" if not helpers.empty('OPNsense.DynDNS.general.enabled') else "false" }},
-      "verbose": {{ "true" if not helpers.empty('OPNsense.DynDNS.general.verbose') else "false" }},
-      "allowipv6": {{ "true" if not helpers.empty('OPNsense.DynDNS.general.allowipv6') else "false" }},
-      "daemon_delay": {{OPNsense.DynDNS.general.daemon_delay|default('300')}}
-  },
-  "accounts": [
-{%  if helpers.exists('OPNsense.DynDNS.accounts.account') %}
-{%     for account in helpers.toList('OPNsense.DynDNS.accounts.account') %}
-{%         if account.enabled  == '1' %}
-      {
-          "id": "{{ account['@uuid'] }}",
-          "service": "{{ account.service }}",
-          "protocol": "{{ account.protocol }}",
-          "server": "{{ account.server }}",
-          "resourceId": "{{ account.resourceId }}",
-          "username": "{{ account.username }}",
-          "password": "{{ account.password }}",
-          "hostnames": "{{ account.hostnames }}",
-          "wildcard": {{ "true" if account.wildcard  == '1' else "false"}},
-          "zone": "{{ account.zone }}",
-          "checkip": "{{ account.checkip }}",
-          "checkip_timeout": {{ account.checkip_timeout }},
-          "force_ssl": {{ "true" if account.force_ssl  == '1' else "false"}},
-          "ttl": "{{ account.ttl }}",
-          "interface": "{%if account.interface %}{{physical_interface(account.interface)}}{% endif%}",
-          "description": "{{ account.description }}"
-      }{{ "," if not loop.last else ""}}
-{%         endif %}
-{%     endfor %}
-{%  endif %}
-  ]
+    "general": {
+        "enabled": {{ "true" if not helpers.empty('OPNsense.DynDNS.general.enabled') else "false" }},
+        "verbose": {{ "true" if not helpers.empty('OPNsense.DynDNS.general.verbose') else "false" }},
+        "allowipv6": {{ "true" if not helpers.empty('OPNsense.DynDNS.general.allowipv6') else "false" }},
+        "daemon_delay": {{OPNsense.DynDNS.general.daemon_delay|default('300')}}
+    },
+    "accounts": [
+{% if helpers.exists('OPNsense.DynDNS.accounts.account') %}
+{%   for account in helpers.toList('OPNsense.DynDNS.accounts.account') | selectattr('enabled', 'equalto', '1') %}
+        {
+            "id": "{{ account['@uuid'] }}",
+            "service": "{{ account.service }}",
+            "protocol": "{{ account.protocol }}",
+            "server": "{{ account.server }}",
+            "resourceId": "{{ account.resourceId }}",
+            "username": "{{ account.username }}",
+            "password": "{{ account.password }}",
+            "hostnames": "{{ account.hostnames }}",
+            "wildcard": {{ "true" if account.wildcard == '1' else "false"}},
+            "zone": "{{ account.zone }}",
+            "checkip": "{{ account.checkip }}",
+            "checkip_timeout": {{ account.checkip_timeout }},
+            "force_ssl": {{ "true" if account.force_ssl == '1' else "false"}},
+            "ttl": "{{ account.ttl }}",
+            "interface": "{%if account.interface %}{{physical_interface(account.interface)}}{% endif%}",
+            "description": "{{ account.description }}"
+        }{{ "," if not loop.last else ""}}
+{%   endfor %}
+{% endif %}
+    ]
 }