resolved notifications: follow-up fixes

Further review comments from: https://github.com/systemd/systemd/pull/22845

diff --git a/src/resolve/resolved-bus.c b/src/resolve/resolved-bus.c
index 908d5aeb312e30a6d94f9b4f45ff87ca6adefe34..51bb5b1ac7627c50942263da3ae6c117d8ca1a91 100644 (file)
--- a/src/resolve/resolved-bus.c
+++ b/src/resolve/resolved-bus.c
@@ -515,9 +515,6 @@ static int bus_method_resolve_hostname(sd_bus_message *message, void *userdata,
 
         q->bus_request = sd_bus_message_ref(message);
         q->request_family = family;
-        q->request_name = strdup(hostname);
-        if (!q->request_name)
-                return log_oom();
         q->complete = bus_method_resolve_hostname_complete;
 
         r = dns_query_bus_track(q, message);
@@ -838,9 +835,6 @@ static int bus_method_resolve_record(sd_bus_message *message, void *userdata, sd
 
         q->bus_request = sd_bus_message_ref(message);
         q->complete = bus_method_resolve_record_complete;
-        q->request_name = strdup(name);
-        if (!q->request_name)
-                return log_oom();
 
         r = dns_query_bus_track(q, message);
         if (r < 0)
@@ -1198,9 +1192,6 @@ static int resolve_service_hostname(DnsQuery *q, DnsResourceRecord *rr, int ifin
                 return r;
 
         aux->request_family = q->request_family;
-        aux->request_name = strdup(rr->srv.name);
-        if (!aux->request_name)
-                return log_oom();
         aux->complete = resolve_service_hostname_complete;
 
         r = dns_query_make_auxiliary(aux, q);

diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c
index 8daaacfc197f58a8a5842768fd5eb9e9e8dfe433..50c0e05ca61868a2600f65ba84ac06862f2e1816 100644 (file)
--- a/src/resolve/resolved-dns-query.c
+++ b/src/resolve/resolved-dns-query.c
@@ -427,7 +427,6 @@ DnsQuery *dns_query_free(DnsQuery *q) {
         }
 
         free(q->request_address_string);
-        free(q->request_name);
 
         if (q->manager) {
                 LIST_REMOVE(queries, q->manager->dns_queries, q);
@@ -586,12 +585,8 @@ void dns_query_complete(DnsQuery *q, DnsTransactionState state) {
 
         q->state = state;
 
-        if (state == DNS_TRANSACTION_SUCCESS && set_size(q->manager->varlink_subscription) > 0) {
-                DnsQuestion *question = q->request_packet ? q->request_packet->question : NULL;
-                const char *query_name = question ? dns_question_first_name(question) : q->request_name;
-                if (query_name)
-                        (void) send_dns_notification(q->manager, q->answer, query_name);
-        }
+        if (q->question_utf8 && state == DNS_TRANSACTION_SUCCESS && set_size(q->manager->varlink_subscription) > 0)
+                (void) send_dns_notification(q->manager, q->answer, dns_question_first_name(q->question_utf8));
 
         dns_query_stop(q);
         if (q->complete)

diff --git a/src/resolve/resolved-dns-query.h b/src/resolve/resolved-dns-query.h
index 0b00465008dd2826bca4b9a9ac84c3273b7037e6..43a833a08a2bd6ef012b7e8f1cc0e2bac57ae6a8 100644 (file)
--- a/src/resolve/resolved-dns-query.h
+++ b/src/resolve/resolved-dns-query.h
@@ -95,7 +95,6 @@ struct DnsQuery {
         union in_addr_union request_address;
         unsigned block_all_complete;
         char *request_address_string;
-        char *request_name;
 
         /* DNS stub information */
         DnsPacket *request_packet;

diff --git a/src/resolve/resolved-varlink.c b/src/resolve/resolved-varlink.c
index 67ae6ff441d0b335564088c05e5a313fc93d7c03..259a4bb7c6a97d54726f0e1d70e4577fa6639afc 100644 (file)
--- a/src/resolve/resolved-varlink.c
+++ b/src/resolve/resolved-varlink.c
@@ -350,9 +350,6 @@ static int vl_method_resolve_hostname(Varlink *link, JsonVariant *parameters, Va
         q->varlink_request = varlink_ref(link);
         varlink_set_userdata(link, q);
         q->request_family = p.family;
-        q->request_name = strdup(p.name);
-        if (!q->request_name)
-                return log_oom();
         q->complete = vl_method_resolve_hostname_complete;
 
         r = dns_query_go(q);
@@ -540,8 +537,11 @@ static int vl_method_subscribe_dns_resolves(Varlink *link, JsonVariant *paramete
 
         assert(link);
 
-        m = varlink_server_get_userdata(varlink_get_server(link));
-        assert(m);
+        m = ASSERT_PTR(varlink_server_get_userdata(varlink_get_server(link)));
+
+        /* if the client didn't set the more flag, return an empty response and close the connection */
+        if (!FLAGS_SET(flags, VARLINK_METHOD_MORE))
+                return varlink_error_errno(link, EINVAL);
 
         if (json_variant_elements(parameters) > 0)
                 return varlink_error_invalid_parameter(link, parameters);
@@ -553,13 +553,48 @@ static int vl_method_subscribe_dns_resolves(Varlink *link, JsonVariant *paramete
 
         log_debug("%u clients now attached for varlink notifications", set_size(m->varlink_subscription));
 
-        /* if the client didn't set the more flag, return an empty response and close the connection */
-        if (!FLAGS_SET(flags, VARLINK_METHOD_MORE))
-                return varlink_reply(link, NULL);
-
         return 1;
 }
 
+static int varlink_notification_server_init(Manager *m) {
+        _cleanup_(varlink_server_unrefp) VarlinkServer *server = NULL;
+        int r;
+
+        assert(m);
+
+        if (!m->enable_varlink_notifications || m->varlink_notification_server)
+                return 0;
+
+        r = varlink_server_new(&server, VARLINK_SERVER_ROOT_ONLY);
+        if (r < 0)
+                return log_error_errno(r, "Failed to allocate varlink server object: %m");
+
+        varlink_server_set_userdata(server, m);
+
+        r = varlink_server_bind_method(
+                        server,
+                        "io.systemd.Resolve.Monitor.SubscribeQueryResults",
+                        vl_method_subscribe_dns_resolves);
+        if (r < 0)
+                return log_error_errno(r, "Failed to register varlink methods: %m");
+
+        r = varlink_server_bind_disconnect(server, vl_on_notification_disconnect);
+        if (r < 0)
+                return log_error_errno(r, "Failed to register varlink disconnect handler: %m");
+
+        r = varlink_server_listen_address(server, "/run/systemd/resolve/io.systemd.Resolve.Monitor", 0660);
+        if (r < 0)
+                return log_error_errno(r, "Failed to bind to varlink socket: %m");
+
+        r = varlink_server_attach_event(server, m->event, SD_EVENT_PRIORITY_NORMAL);
+        if (r < 0)
+                return log_error_errno(r, "Failed to attach varlink connection to event loop: %m");
+
+        m->varlink_notification_server = TAKE_PTR(server);
+
+        return 0;
+}
+
 int manager_varlink_init(Manager *m) {
         _cleanup_(varlink_server_unrefp) VarlinkServer *s = NULL;
         int r;
@@ -596,37 +631,9 @@ int manager_varlink_init(Manager *m) {
 
         m->varlink_server = TAKE_PTR(s);
 
-        if (m->enable_varlink_notifications) {
-                if (m->varlink_notification_server)
-                        return 0;
-
-                r = varlink_server_new(&s, VARLINK_SERVER_ACCOUNT_UID);
-                if (r < 0)
-                        return log_error_errno(r, "Failed to allocate varlink server object: %m");
-
-                varlink_server_set_userdata(s, m);
-
-                r = varlink_server_bind_method_many(
-                                s,
-                                "io.systemd.Resolve.Monitor.SubscribeDnsResolves",
-                                vl_method_subscribe_dns_resolves);
-                if (r < 0)
-                        return log_error_errno(r, "Failed to register varlink methods: %m");
-
-                r = varlink_server_bind_disconnect(s, vl_on_notification_disconnect);
-                if (r < 0)
-                        return log_error_errno(r, "Failed to register varlink disconnect handler: %m");
-
-                r = varlink_server_listen_address(s, "/run/systemd/resolve/io.systemd.Resolve.Monitor", 0660);
-                if (r < 0)
-                        return log_error_errno(r, "Failed to bind to varlink socket: %m");
-
-                r = varlink_server_attach_event(s, m->event, SD_EVENT_PRIORITY_NORMAL);
-                if (r < 0)
-                        return log_error_errno(r, "Failed to attach varlink connection to event loop: %m");
-
-                m->varlink_notification_server = TAKE_PTR(s);
-        }
+        r = varlink_notification_server_init(m);
+        if (r < 0)
+                return r;
 
         return 0;
 }

diff --git a/test/units/testsuite-75.sh b/test/units/testsuite-75.sh
index ba91284a73efabc6e5181f262a4f72ae66b98a97..7823f465839b5da678a36748345810cb1ead11bf 100755 (executable)
--- a/test/units/testsuite-75.sh
+++ b/test/units/testsuite-75.sh
@@ -102,7 +102,7 @@ cat >$NOTIFICATION_SUBSCRIPTION_SCRIPT <<EOF
 #!/bin/sh
 printf '
 {
-  "method": "io.systemd.Resolve.Monitor.SubscribeDnsResolves",
+  "method": "io.systemd.Resolve.Monitor.SubscribeQueryResults",
   "more": true
 }\0' | nc -U /run/systemd/resolve/io.systemd.Resolve.Monitor > $NOTIFICATION_LOGS
 EOF
@@ -274,7 +274,7 @@ grep -qF "; fully validated" "$RUN_OUT"
 run resolvectl query -t A cname-chain.signed.test
 grep -qF "follow14.final.signed.test IN A 10.0.0.14" "$RUN_OUT"
 grep -qF "authenticated: yes" "$RUN_OUT"
-notification_check_host "cname-chain.signed.test" "10.0.0.14"
+notification_check_host "follow10.so.close.signed.test" "10.0.0.14"
 # Non-existing RR + CNAME chain
 run dig +dnssec AAAA cname-chain.signed.test
 grep -qF "status: NOERROR" "$RUN_OUT"