3587. [func] 'named -g' now checks the logging configuration but

does not use it. [RT #33473]

CHANGES

@@ -1,3 +1,6 @@
+3587.	[func]		'named -g' now checks the logging configuration but
+			does not use it. [RT #33473]
+
 3586.	[buf]		Handle errors in xmlDocDumpFormatMemoryEnc. [RT #33706]
 
 3585.	[func]		"rndc delzone -clean" option removes zone files

bin/named/logconf.c

@@ -41,10 +41,10 @@
 
 /*%
  * Set up a logging category according to the named.conf data
- * in 'ccat' and add it to 'lctx'.
+ * in 'ccat' and add it to 'logconfig'.
  */
 static isc_result_t
-category_fromconf(const cfg_obj_t *ccat, isc_logconfig_t *lctx) {
+category_fromconf(const cfg_obj_t *ccat, isc_logconfig_t *logconfig) {
 	isc_result_t result;
 	const char *catname;
 	isc_logcategory_t *category;
@@ -64,6 +64,9 @@ category_fromconf(const cfg_obj_t *ccat, isc_logconfig_t *lctx) {
 		return (ISC_R_SUCCESS);
 	}
 
+	if (logconfig == NULL)
+		return (ISC_R_SUCCESS);
+
 	module = NULL;
 
 	destinations = cfg_tuple_get(ccat, "destinations");
@@ -74,7 +77,7 @@ category_fromconf(const cfg_obj_t *ccat, isc_logconfig_t *lctx) {
 		const cfg_obj_t *channel = cfg_listelt_value(element);
 		const char *channelname = cfg_obj_asstring(channel);
 
-		result = isc_log_usechannel(lctx, channelname, category,
+		result = isc_log_usechannel(logconfig, channelname, category,
 					    module);
 		if (result != ISC_R_SUCCESS) {
 			isc_log_write(ns_g_lctx, CFG_LOGCATEGORY_CONFIG,
@@ -89,10 +92,11 @@ category_fromconf(const cfg_obj_t *ccat, isc_logconfig_t *lctx) {
 
 /*%
  * Set up a logging channel according to the named.conf data
- * in 'cchan' and add it to 'lctx'.
+ * in 'cchan' and add it to 'logconfig'.
  */
 static isc_result_t
-channel_fromconf(const cfg_obj_t *channel, isc_logconfig_t *lctx) {
+channel_fromconf(const cfg_obj_t *channel, isc_logconfig_t *logconfig)
+{
 	isc_result_t result;
 	isc_logdestination_t dest;
 	unsigned int type;
@@ -215,7 +219,10 @@ channel_fromconf(const cfg_obj_t *channel, isc_logconfig_t *lctx) {
 			level = cfg_obj_asuint32(severity);
 	}
 
-	result = isc_log_createchannel(lctx, channelname,
+	if (logconfig == NULL)
+		result = ISC_R_SUCCESS;
+	else
+		result = isc_log_createchannel(logconfig, channelname,
 					       type, level, &dest, flags);
 
 	if (result == ISC_R_SUCCESS && type == ISC_LOG_TOFILE) {
@@ -226,32 +233,31 @@ channel_fromconf(const cfg_obj_t *channel, isc_logconfig_t *lctx) {
 		 * Fix defect #22771
 		*/
 		result = isc_file_isplainfile(dest.file.name);
-		if (result == ISC_R_SUCCESS ||
-		    result == ISC_R_FILENOTFOUND) {
+		if (result == ISC_R_SUCCESS || result == ISC_R_FILENOTFOUND) {
 			/*
 			 * Test that the file can be opened, since
 			 * isc_log_open() can't effectively report
-			 * failures when called in
-			 * isc_log_doit().
+			 * failures when called in isc_log_doit().
 			 */
 			result = isc_stdio_open(dest.file.name, "a", &fp);
 			if (result != ISC_R_SUCCESS) {
+				if (logconfig != NULL && !ns_g_nosyslog)
 					syslog(LOG_ERR,
-					"isc_stdio_open '%s' failed: %s",
-					dest.file.name,
+						"isc_stdio_open '%s' failed: "
+						"%s", dest.file.name,
 						isc_result_totext(result));
 				fprintf(stderr,
-					"isc_stdio_open '%s' failed: %s",
+					"isc_stdio_open '%s' failed: %s\n",
 					dest.file.name,
 				        isc_result_totext(result));
 			} else
 				(void)isc_stdio_close(fp);
 			goto done;
 		}
-		if (!ns_g_nosyslog)
+		if (logconfig != NULL && !ns_g_nosyslog)
 			syslog(LOG_ERR, "isc_file_isplainfile '%s' failed: %s",
 			       dest.file.name, isc_result_totext(result));
-		fprintf(stderr, "isc_file_isplainfile '%s' failed: %s",
+		fprintf(stderr, "isc_file_isplainfile '%s' failed: %s\n",
 			dest.file.name, isc_result_totext(result));
 	}
 
@@ -260,7 +266,7 @@ channel_fromconf(const cfg_obj_t *channel, isc_logconfig_t *lctx) {
 }
 
 isc_result_t
-ns_log_configure(isc_logconfig_t *logconf, const cfg_obj_t *logstmt) {
+ns_log_configure(isc_logconfig_t *logconfig, const cfg_obj_t *logstmt) {
 	isc_result_t result;
 	const cfg_obj_t *channels = NULL;
 	const cfg_obj_t *categories = NULL;
@@ -269,7 +275,8 @@ ns_log_configure(isc_logconfig_t *logconf, const cfg_obj_t *logstmt) {
 	isc_boolean_t unmatched_set = ISC_FALSE;
 	const cfg_obj_t *catname;
 
-	CHECK(ns_log_setdefaultchannels(logconf));
+	if (logconfig != NULL)
+		CHECK(ns_log_setdefaultchannels(logconfig));
 
 	(void)cfg_map_get(logstmt, "channel", &channels);
 	for (element = cfg_list_first(channels);
@@ -277,7 +284,7 @@ ns_log_configure(isc_logconfig_t *logconf, const cfg_obj_t *logstmt) {
 	     element = cfg_list_next(element))
 	{
 		const cfg_obj_t *channel = cfg_listelt_value(element);
-		CHECK(channel_fromconf(channel, logconf));
+		CHECK(channel_fromconf(channel, logconfig));
 	}
 
 	(void)cfg_map_get(logstmt, "category", &categories);
@@ -286,7 +293,7 @@ ns_log_configure(isc_logconfig_t *logconf, const cfg_obj_t *logstmt) {
 	     element = cfg_list_next(element))
 	{
 		const cfg_obj_t *category = cfg_listelt_value(element);
-		CHECK(category_fromconf(category, logconf));
+		CHECK(category_fromconf(category, logconfig));
 		if (!default_set) {
 			catname = cfg_tuple_get(category, "name");
 			if (strcmp(cfg_obj_asstring(catname), "default") == 0)
@@ -299,16 +306,14 @@ ns_log_configure(isc_logconfig_t *logconf, const cfg_obj_t *logstmt) {
 		}
 	}
 
-	if (!default_set)
-		CHECK(ns_log_setdefaultcategory(logconf));
+	if (logconfig != NULL && !default_set)
+		CHECK(ns_log_setdefaultcategory(logconfig));
 
-	if (!unmatched_set)
-		CHECK(ns_log_setunmatchedcategory(logconf));
+	if (logconfig != NULL && !unmatched_set)
+		CHECK(ns_log_setunmatchedcategory(logconfig));
 
 	return (ISC_R_SUCCESS);
 
  cleanup:
-	if (logconf != NULL)
-		isc_logconfig_destroy(&logconf);
 	return (result);
 }

bin/named/server.c

@@ -4946,7 +4946,10 @@ load_configuration(const char *filename, ns_server_t *server,
 	dns_viewlist_t viewlist, builtin_viewlist;
 	in_port_t listen_port, udpport_low, udpport_high;
 	int i;
+	int num_zones = 0;
+	isc_boolean_t exclusive = ISC_FALSE;
 	isc_interval_t interval;
+	isc_logconfig_t *logc = NULL;
 	isc_portset_t *v4portset = NULL;
 	isc_portset_t *v6portset = NULL;
 	isc_resourcevalue_t nfiles;
@@ -4955,12 +4958,10 @@ load_configuration(const char *filename, ns_server_t *server,
 	isc_uint32_t interface_interval;
 	isc_uint32_t reserved;
 	isc_uint32_t udpsize;
-	ns_cachelist_t cachelist, tmpcachelist;
-	unsigned int maxsocks;
 	ns_cache_t *nsc;
+	ns_cachelist_t cachelist, tmpcachelist;
 	struct cfg_context *nzctx;
-	int num_zones = 0;
-	isc_boolean_t exclusive = ISC_FALSE;
+	unsigned int maxsocks;
 
 	ISC_LIST_INIT(viewlist);
 	ISC_LIST_INIT(builtin_viewlist);
@@ -5642,13 +5643,30 @@ load_configuration(const char *filename, ns_server_t *server,
 	 * unprivileged user, not root.
 	 */
 	if (ns_g_logstderr) {
+		const cfg_obj_t *logobj = NULL;
+
 		isc_log_write(ns_g_lctx, NS_LOGCATEGORY_GENERAL,
 			      NS_LOGMODULE_SERVER, ISC_LOG_INFO,
-			      "ignoring config file logging "
-			      "statement due to -g option");
+			      "not using config file logging "
+			      "statement for logging due to "
+			      "-g option");
+		
+		(void)cfg_map_get(config, "logging", &logobj);
+		if (logobj != NULL) {
+			result = ns_log_configure(NULL, logobj);
+			if (result != ISC_R_SUCCESS) {
+				isc_log_write(ns_g_lctx,
+					      NS_LOGCATEGORY_GENERAL,
+					      NS_LOGMODULE_SERVER,
+					      ISC_LOG_ERROR,
+					      "checking logging configuration "
+					      "failed: %s",
+					      isc_result_totext(result));
+				goto cleanup;
+			}
+		}
 	} else {
 		const cfg_obj_t *logobj = NULL;
-		isc_logconfig_t *logc = NULL;
 
 		CHECKM(isc_logconfig_create(ns_g_lctx, &logc),
 		       "creating new logging configuration");
@@ -5667,11 +5685,9 @@ load_configuration(const char *filename, ns_server_t *server,
 			       "setting up default 'category default'");
 		}
 
-		result = isc_logconfig_use(ns_g_lctx, logc);
-		if (result != ISC_R_SUCCESS) {
-			isc_logconfig_destroy(&logc);
-			CHECKM(result, "installing logging configuration");
-		}
+		CHECKM(isc_logconfig_use(ns_g_lctx, logc),
+		       "installing logging configuration");
+		logc = NULL;
 
 		isc_log_write(ns_g_lctx, NS_LOGCATEGORY_GENERAL,
 			      NS_LOGMODULE_SERVER, ISC_LOG_DEBUG(1),
@@ -5803,6 +5819,9 @@ load_configuration(const char *filename, ns_server_t *server,
 	result = ISC_R_SUCCESS;
 
  cleanup:
+	if (logc != NULL)
+		isc_logconfig_destroy(&logc);
+		
 	if (v4portset != NULL)
 		isc_portset_destroy(ns_g_mctx, &v4portset);
 

bin/tests/system/logfileconfig/tests.sh

@@ -32,13 +32,101 @@ PIDFILE="${THISDIR}/${CONFDIR}/named.pid"
 myRNDC="$RNDC -c ${THISDIR}/${CONFDIR}/rndc.conf"
 myNAMED="$NAMED -c ${THISDIR}/${CONFDIR}/named.conf -m record,size,mctx -T clienttest -T nosyslog -d 99 -U 4"
 
+status=0
+
+cd $CONFDIR
+
+echo "I:testing log file validity (named -g + only plain files allowed)"
+
+# First run with a known good config.
+echo > $PLAINFILE
+cp $PLAINCONF named.conf
+$myRNDC reconfig
+grep "reloading configuration failed" named.run > /dev/null 2>&1
+if [ $? -ne 0 ]
+then
+	echo "I: testing plain file succeeded"
+else
+	echo "I: testing plain file failed (unexpected)"
+	echo "I:exit status: 1"
+	exit 1 
+fi
+
+# Now try directory, expect failure
+echo "I: testing directory as log file (named -g)"
+echo > named.run
+rm -rf $DIRFILE
+mkdir -p $DIRFILE >/dev/null 2>&1
+if [ $? -eq 0 ]
+then
+	cp $DIRCONF named.conf
+	echo > named.run
+	$myRNDC reconfig
+	grep "checking logging configuration failed: invalid file" named.run > /dev/null 2>&1
+	if [ $? -ne 0 ]
+	then
+		echo "I: testing directory as file succeeded (UNEXPECTED)"
+		echo "I:exit status: 1"
+		exit 1
+	else
+		echo "I: testing directory as log file failed (expected)"
+	fi
+else
+	echo "I: skipping directory test (unable to create directory)"
+fi
+
+# Now try pipe file, expect failure
+echo "I: testing pipe file as log file (named -g)"
+echo > named.run
+rm -f $PIPEFILE
+mkfifo $PIPEFILE >/dev/null 2>&1
+if [ $? -eq 0 ]
+then
+	cp $PIPECONF named.conf
+	echo > named.run
+	$myRNDC reconfig
+	grep "checking logging configuration failed: invalid file" named.run  > /dev/null 2>&1
+	if [ $? -ne 0 ]
+	then
+		echo "I: testing pipe file as log file succeeded (UNEXPECTED)"
+		echo "I:exit status: 1"
+		exit 1
+	else
+		echo "I: testing pipe file as log file failed (expected)"
+	fi
+else
+	echo "I: skipping pipe test (unable to create pipe)"
+fi
+
+# Now try symlink file to plain file, expect success 
+echo "I: testing symlink to plain file as log file (named -g)"
+# Assume success
+echo > named.run
+echo > $PLAINFILE
+rm -f  $SYMFILE  $SYMFILE
+ln -s $PLAINFILE $SYMFILE >/dev/null 2>&1
+if [ $? -eq 0 ]
+then
+	cp $SYMCONF named.conf
+	$myRNDC reconfig
+	echo > named.run
+	grep "reloading configuration failed" named.run > /dev/null 2>&1
+	if [ $? -ne 0 ]
+	then
+		echo "I: testing symlink to plain file succeeded"
+	else
+		echo "I: testing symlink to plain file failed (unexpected)"
+		echo "I:exit status: 1"
+		exit 1
+	fi
+else
+	echo "I: skipping symlink test (unable to create symlink)"
+fi
 # Stop the server and run through a series of tests with various config
 # files while controlling the stop/start of the server.
 # Have to stop the stock server because it uses "-g"
 #
-$PERL ../stop.pl . ns1
-
-cd $CONFDIR
+$PERL ../../stop.pl .. ns1
 
 $myNAMED > /dev/null 2>&1
 
@@ -70,6 +158,7 @@ fi
 # Now try directory, expect failure
 echo "I: testing directory as log file"
 echo > named.run
+rm -rf $DIRFILE
 mkdir -p $DIRFILE >/dev/null 2>&1
 if [ $? -eq 0 ]
 then
@@ -92,6 +181,7 @@ fi
 # Now try pipe file, expect failure
 echo "I: testing pipe file as log file"
 echo > named.run
+rm -f $PIPEFILE
 mkfifo $PIPEFILE >/dev/null 2>&1
 if [ $? -eq 0 ]
 then
@@ -117,6 +207,7 @@ echo "I: testing symlink to plain file as log file"
 status=0
 echo > named.run
 echo > $PLAINFILE
+rm -f $SYMFILE
 ln -s $PLAINFILE $SYMFILE >/dev/null 2>&1
 if [ $? -eq 0 ]
 then

lib/isc/log.c

@@ -767,7 +767,7 @@ isc_log_createchannel(isc_logconfig_t *lcfg, const char *name,
 		break;
 
 	default:
-		isc_mem_put(mctx, channel->name, strlen(channel->name) + 1);
+		isc_mem_free(mctx, channel->name);
 		isc_mem_put(mctx, channel, sizeof(*channel));
 		return (ISC_R_UNEXPECTED);
 	}