chg: usr: plugin extension in plugin path is now optional

Plugin configuration no longer requires the library file extension, so it is now possible to invoke a plugin using the syntax `plugin query "library"` instead of `plugin query "libary.so"`.

Closes #5377

Merge branch '5377-implicit-plugin-ext' into 'main'

See merge request isc-projects/bind9!10753

bin/tests/system/hooks/ns1/named.conf.j2

@@ -10,6 +10,7 @@
  * See the COPYRIGHT file distributed with this work for additional
  * information regarding copyright ownership.
  */
+{% set noextension = noextension | default(False) %}
 
 options {
 	query-source address 10.53.0.1;
@@ -24,8 +25,11 @@ options {
 	minimal-responses no;
 };
 
-
+{% if noextension %}
+plugin query "@TOP_BUILDDIR@/testlib-driver-async";
+{% else %}
 plugin query "@TOP_BUILDDIR@/testlib-driver-async.@DYLIB@";
+{% endif %}
 
 key rndc_key {
 	secret "1234abcd8765";

bin/tests/system/hooks/tests_hooks.py

@@ -16,8 +16,16 @@ pytest.importorskip("dns")
 import dns.message
 
 
-def test_async_hook():
+def test_hooks():
     msg = dns.message.make_query("example.com.", "A")
     res = isctest.query.udp(msg, "10.53.0.1")
     # the test-async plugin changes the status of any positive answer to NOTIMP
     isctest.check.notimp(res)
+
+
+def test_hooks_noextension(ns1, templates):
+    templates.render("ns1/named.conf", {"noextension": True})
+    with ns1.watch_log_from_here() as watcher:
+        ns1.rndc("reload")
+        watcher.wait_for_line("all zones loaded")
+    test_hooks()

doc/arm/plugins.inc.rst

@@ -47,8 +47,10 @@ A plugin is configured with the :any:`plugin` statement in :iscman:`named.conf`:
        };
 
 
-In this example, file ``library.so`` is the plugin library. ``query``
-indicates that this is a query plugin.
+In this example, ``query`` indicates that this is a query plugin,
+and ``library.so`` is the name of the plugin library.  Note that the
+library file extension (in this case, ``.so``) is optional, and can
+be omitted.
 
 Multiple :any:`plugin` statements can be specified, to load different
 plugins or multiple instances of the same plugin.

lib/ns/hooks.c

@@ -18,6 +18,7 @@
 #include <string.h>
 
 #include <isc/errno.h>
+#include <isc/file.h>
 #include <isc/list.h>
 #include <isc/log.h>
 #include <isc/mem.h>
@@ -54,9 +55,10 @@ struct ns_plugin {
 static ns_hooklist_t default_hooktable[NS_HOOKPOINTS_COUNT];
 ns_hooktable_t *ns__hook_table = &default_hooktable;
 
-isc_result_t
-ns_plugin_expandpath(const char *src, char *dst, size_t dstsize) {
+static isc_result_t
+plugin_expandpath(const char *src, char *dst, size_t dstsize, bool appendext) {
 	int result;
+	const char *ext = appendext ? NAMED_PLUGINEXT : "";
 
 	/*
 	 * On Unix systems, differentiate between paths and filenames.
@@ -65,12 +67,13 @@ ns_plugin_expandpath(const char *src, char *dst, size_t dstsize) {
 		/*
 		 * 'src' is an absolute or relative path.  Copy it verbatim.
 		 */
-		result = snprintf(dst, dstsize, "%s", src);
+		result = snprintf(dst, dstsize, "%s%s", src, ext);
 	} else {
 		/*
 		 * 'src' is a filename.  Prepend default plugin directory path.
 		 */
-		result = snprintf(dst, dstsize, "%s/%s", NAMED_PLUGINDIR, src);
+		result = snprintf(dst, dstsize, "%s/%s%s", NAMED_PLUGINDIR, src,
+				  ext);
 	}
 
 	if (result < 0) {
@@ -82,6 +85,22 @@ ns_plugin_expandpath(const char *src, char *dst, size_t dstsize) {
 	}
 }
 
+isc_result_t
+ns_plugin_expandpath(const char *src, char *dst, size_t dstsize) {
+	isc_result_t result;
+
+	result = plugin_expandpath(src, dst, dstsize, false);
+	if (result != ISC_R_SUCCESS) {
+		return result;
+	}
+
+	if (isc_file_exists(dst) == false) {
+		result = plugin_expandpath(src, dst, dstsize, true);
+	}
+
+	return result;
+}
+
 static isc_result_t
 load_symbol(uv_lib_t *handle, const char *modpath, const char *symbol_name,
 	    void **symbolp) {

meson.build

@@ -215,7 +215,8 @@ if host_machine.cpu_family() == 'x86'
     )
 endif
 
-if host_machine.system() == 'darwin'
+isdarwin = host_machine.system() == 'darwin'
+if isdarwin
     add_project_arguments(
         cc.get_supported_arguments(
             '-Wno-deprecated-declarations', # For GSS.Framework
@@ -279,6 +280,13 @@ config.set_quoted('RNDC_CONFFILE', sysconfdir / 'rndc.conf')
 config.set_quoted('RNDC_KEYFILE', sysconfdir / 'rndc.key')
 
 config.set_quoted('NAMED_PLUGINDIR', libdir / 'bind')
+if isdarwin
+    # Plugin extensions - macOS is the only specific case
+    config.set_quoted('NAMED_PLUGINEXT', '.dylib')
+else
+    config.set_quoted('NAMED_PLUGINEXT', '.so')
+endif
+
 config.set_quoted('NAMED_LOCALSTATEDIR', localstatedir)
 config.set_quoted('NAMED_SYSCONFDIR', sysconfdir)
 config.set_quoted('NAMED_CONFFILE', sysconfdir / 'named.conf')

tests/ns/meson.build

@@ -14,6 +14,12 @@ foreach unit : [
     'plugin',
     'query',
 ]
+    linkargs = ''
+    if unit == 'plugin'
+        linkargs = [
+            '-Wl,--wrap=isc_file_exists',
+        ]
+    endif
     test_bin = executable(
         unit,
         files(f'@unit@_test.c', 'netmgr_wrap.c'),
@@ -31,6 +37,7 @@ foreach unit : [
             cmocka_dep,
             nghttp2_dep,
         ],
+        link_args: linkargs,
     )
 
     test(

tests/ns/plugin_test.c

@@ -34,7 +34,16 @@
 
 #include <dns/lib.h>
 
-#include <ns/hooks.h>
+#include "../ns/hooks.c"
+
+bool
+__wrap_isc_file_exists(const char *pathname);
+
+bool
+__wrap_isc_file_exists(const char *pathname) {
+	UNUSED(pathname);
+	return mock();
+}
 
 #include <tests/ns.h>
 
@@ -43,8 +52,8 @@
  */
 typedef struct {
 	const ns_test_id_t id; /* libns test identifier */
-	const char *input;     /* source string - plugin name or path
-				* */
+	const char *input;     /* source string - plugin name or path */
+	bool exists;	       /* return of mocked isc_file_exists() */
 	size_t output_size;    /* size of target char array to
 				* allocate */
 	isc_result_t result;   /* expected return value */
@@ -65,6 +74,10 @@ run_full_path_test(const ns_plugin_expandpath_test_params_t *test,
 	REQUIRE(test->input != NULL);
 	REQUIRE(test->result != ISC_R_SUCCESS || test->output != NULL);
 
+	if (test->result == ISC_R_SUCCESS) {
+		will_return(__wrap_isc_file_exists, test->exists);
+	}
+
 	/*
 	 * Prepare a target buffer of given size.  Store it in 'state' so that
 	 * it can get cleaned up by _teardown() if the test fails.
@@ -108,6 +121,7 @@ ISC_RUN_TEST_IMPL(ns_plugin_expandpath) {
 		{
 			NS_TEST_ID("correct use with an absolute path"),
 			.input = "/usr/lib/named/foo.so",
+			.exists = true,
 			.output_size = PATH_MAX,
 			.result = ISC_R_SUCCESS,
 			.output = "/usr/lib/named/foo.so",
@@ -115,6 +129,7 @@ ISC_RUN_TEST_IMPL(ns_plugin_expandpath) {
 		{
 			NS_TEST_ID("correct use with a relative path"),
 			.input = "../../foo.so",
+			.exists = true,
 			.output_size = PATH_MAX,
 			.result = ISC_R_SUCCESS,
 			.output = "../../foo.so",
@@ -122,31 +137,72 @@ ISC_RUN_TEST_IMPL(ns_plugin_expandpath) {
 		{
 			NS_TEST_ID("correct use with a filename"),
 			.input = "foo.so",
+			.exists = true,
 			.output_size = PATH_MAX,
 			.result = ISC_R_SUCCESS,
 			.output = NAMED_PLUGINDIR "/foo.so",
 		},
+		{
+			NS_TEST_ID("correct use with an absolute path and no "
+				   "extension"),
+			.input = "/usr/lib/named/foo",
+			.exists = false,
+			.output_size = PATH_MAX,
+			.result = ISC_R_SUCCESS,
+			.output = "/usr/lib/named/foo.so",
+		},
+		{
+			NS_TEST_ID("correct use with a relative path and no "
+				   "extension"),
+			.input = "../../foo",
+			.exists = false,
+			.output_size = PATH_MAX,
+			.result = ISC_R_SUCCESS,
+			.output = "../../foo.so",
+		},
+		{
+			NS_TEST_ID("correct use with a filename and no "
+				   "extension"),
+			.input = "foo",
+			.exists = false,
+			.output_size = PATH_MAX,
+			.result = ISC_R_SUCCESS,
+			.output = NAMED_PLUGINDIR "/foo.so",
+		},
+		{
+			NS_TEST_ID("correct use with a filename and no "
+				   "extension but a name with dots"),
+			.input = "foo.bar",
+			.exists = false,
+			.output_size = PATH_MAX,
+			.result = ISC_R_SUCCESS,
+			.output = NAMED_PLUGINDIR "/foo.bar.so",
+		},
 		{
 			NS_TEST_ID("no space at all in target buffer"),
 			.input = "/usr/lib/named/foo.so",
+			.exists = true,
 			.output_size = 0,
 			.result = ISC_R_NOSPACE,
 		},
 		{
 			NS_TEST_ID("target buffer too small to fit input"),
 			.input = "/usr/lib/named/foo.so",
+			.exists = true,
 			.output_size = 1,
 			.result = ISC_R_NOSPACE,
 		},
 		{
 			NS_TEST_ID("target buffer too small to fit NULL byte"),
 			.input = "/foo.so",
+			.exists = true,
 			.output_size = 7,
 			.result = ISC_R_NOSPACE,
 		},
 		{
 			NS_TEST_ID("target buffer too small to fit full path"),
 			.input = "foo.so",
+			.exists = true,
 			.output_size = 7,
 			.result = ISC_R_NOSPACE,
 		},