[PATCH] avoid multiple "domain" entries in resolv.conf

[Corey Hickey]

On 2016-08-25 08:07, David Woodhouse wrote:
> 
>> +	DOMAINS=$CISCO_DEF_DOMAIN
> 
> If $CISCO_DEF_DOMAIN contains spaces (e.g. 'foo bar'), this becomes
>  DOMAINS=foo bar
> ...resulting in:
>  bar: command not found
> 
> It needs quotes around it, surely?

My understanding is that quotes are not required when assigning one
variable to the contents of another.

$ CISCO_DEF_DOMAIN='foo bar'
$ DOMAINS=$CISCO_DEF_DOMAIN
$ echo "$DOMAINS"
foo bar

They don't hurt, though, so I added them in.

> 
>> -				#if [ -z "$CISCO_DEF_DOMAIN_ORIG" ]; then
>> +				#if [ -z "$$CISCO_DEF_DOMAIN" ]; then
> 
> Something went wrong with your search/replace there (and in a few more places).

Aha, yes. Sorry.

> Other than that, looks good. Thanks for the careful analysis.

Excellent; you're welcome. Here's one more revision:


commit 6a44a28ed357c5a9c2e5bc720145b39cb7849c97
Author: Corey Hickey <bugfood-ml at fatooh.org>
Date:   Mon 2016-08-15 10:29:50

    rewrite resolv.conf parsing
    
    This patch simplifies parsing and changes behavior in two ways:
    
    1. Domains for searching are now parsed from "search" and "domain"
    lines. Only a "search" line is outputted, since "search" supports
    multiple domains and is mutually exclusive with "domain". The motivation
    for this is to make vpnc-script behave sanely when there are existing
    resolv.conf files with only "domain" or with both "domain" and "search".
    
    2. All original "nameserver" lines are discarded and replaced rather
    than only the number of nameservers from $INTERNAL_IP4_DNS. The
    rationale here is that vpnc-script should be consistent and either
    retain all original nameservers or overwrite all of them. Retaining them
    is problematic because there is a limit of three, and overwriting is
    closer to the original behavior.
    
    The Darwin changes are untested, but are a simple search/replace and
    thus should work fine.
    
    Signed-off-by: Corey Hickey <bugfood-ml at fatooh.org>

diff --git a/vpnc-script b/vpnc-script
index c3aafa4..9c413e0 100755
--- a/vpnc-script
+++ b/vpnc-script
@@ -369,49 +369,31 @@ modify_resolvconf_generic() {
 # and will be overwritten by vpnc
 # as long as the above mark is intact"
 
-	# If multiple domains are listed, prefer the first for "domain".
-	DOMAIN="${CISCO_DEF_DOMAIN%% *}"
-	# Remember the original value of CISCO_DEF_DOMAIN we need it later
-	CISCO_DEF_DOMAIN_ORIG="$CISCO_DEF_DOMAIN"
-	# Don't step on INTERNAL_IP4_DNS value, use a temporary variable
-	INTERNAL_IP4_DNS_TEMP="$INTERNAL_IP4_DNS"
+	DOMAINS="$CISCO_DEF_DOMAIN"
+
 	exec 6< "$RESOLV_CONF_BACKUP"
 	while read LINE <&6 ; do
 		case "$LINE" in
-			nameserver*)
-				if [ -n "$INTERNAL_IP4_DNS_TEMP" ]; then
-					read ONE_NAMESERVER INTERNAL_IP4_DNS_TEMP <<-EOF
-	$INTERNAL_IP4_DNS_TEMP
-EOF
-					LINE="nameserver $ONE_NAMESERVER"
-				else
-					LINE=""
-				fi
-				;;
-			search*)
-				if [ -n "$CISCO_DEF_DOMAIN" ]; then
-					LINE="$LINE $CISCO_DEF_DOMAIN"
-					CISCO_DEF_DOMAIN=""
-				fi
-				;;
-			domain*)
-				if [ -n "$DOMAIN" ]; then
-					LINE="domain $DOMAIN"
-				fi
-				;;
+			# omit; we will overwrite these
+			nameserver*) ;;
+			# extract listed domains and prepend to list
+			domain* | search*) DOMAINS="${LINE#* } $DOMAINS" ;;
+			# retain other lines
+			*) NEW_RESOLVCONF="$NEW_RESOLVCONF
+$LINE" ;;
 		esac
-		NEW_RESOLVCONF="$NEW_RESOLVCONF
-$LINE"
 	done
 	exec 6<&-
 
-	for i in $INTERNAL_IP4_DNS_TEMP ; do
+	for i in $INTERNAL_IP4_DNS ; do
 		NEW_RESOLVCONF="$NEW_RESOLVCONF
 nameserver $i"
 	done
-	if [ -n "$CISCO_DEF_DOMAIN" ]; then
+	# note that "search" is mutually exclusive with "domain";
+	# "search" allows multiple domains to be listed, so use that
+	if [ -n "$DOMAINS" ]; then
 		NEW_RESOLVCONF="$NEW_RESOLVCONF
-search $CISCO_DEF_DOMAIN"
+search $DOMAINS"
 	fi
 	echo "$NEW_RESOLVCONF" > /etc/resolv.conf
 
@@ -453,7 +435,7 @@ search $CISCO_DEF_DOMAIN"
 				# DNS matching when available.  When multiple DNS matching
 				# is present, anything reading the /etc/resolv.conf file
 				# directly will probably not work as intended.
-				#if [ -z "$CISCO_DEF_DOMAIN_ORIG" ]; then
+				#if [ -z "$CISCO_DEF_DOMAIN" ]; then
 					# Cannot use multiple DNS matching without a domain
 					OVERRIDE_PRIMARY='d.add OverridePrimary # 1'
 				#fi
@@ -471,13 +453,13 @@ search $CISCO_DEF_DOMAIN"
 					set State:/Network/Service/$TUNDEV/IPv4
 					close
 				EOF
-				if [ -n "$CISCO_DEF_DOMAIN_ORIG" ]; then
+				if [ -n "$CISCO_DEF_DOMAIN" ]; then
 					scutil >/dev/null 2>&1 <<-EOF
 						open
 						get State:/Network/Service/$TUNDEV/DNS
-						d.add DomainName $CISCO_DEF_DOMAIN_ORIG
-						d.add SearchDomains * $CISCO_DEF_DOMAIN_ORIG
-						d.add SupplementalMatchDomains * $CISCO_DEF_DOMAIN_ORIG
+						d.add DomainName $CISCO_DEF_DOMAIN
+						d.add SearchDomains * $CISCO_DEF_DOMAIN
+						d.add SupplementalMatchDomains * $CISCO_DEF_DOMAIN
 						set State:/Network/Service/$TUNDEV/DNS
 						close
 					EOF



Thanks,
Corey