From 194052e67ed30121d7b66e8e7db559a32d001dab Mon Sep 17 00:00:00 2001
From: Einhard Leichtfuß <alguien@respiranto.de>
Date: Tue, 25 Dec 2018 16:03:04 +0100
Subject: Prevent matching illegal sequences globally
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

E.g., a+b will no longer be matched by anything.  This is due to the
global usage of a modified list_all().

Also:
- Use different return types for different things (e.g. $RET_SUCCESS).
 `- Both internally and externally (exit codes, documented in ctct(1)).
- Make --search-by-data more efficient.
 `- One call to grep for all files.
 `- The conjunction of the regexes still requires several calls.
- Rename functions to more speaking names.
- Document the function parameters.
- Add vi modelines.
- Use [[:alpha:]] instead of [A-Za-z] which appears to be the same but
  more clear.  Notably, both seem to include non-ASCII letters, for
  example 'à'.
- Set nullglob option in the script to prevent errors for cases where
  glob patterns would be used.
- Add a silent option ro check_syntax() (not used).
- Add a full_path option to list_all() (not used).
- Use return instead of exit in main().
- Remove one remaining usage of eval.
- Fix small format issue in ctct(1) (I -> B).
 `- They are no longer, but it hardly harms to have this option.
---
 CHANGELOG          |   1 +
 Makefile.in        |   2 +
 TODO               |  11 +-
 bash_completion.in |   2 +
 configure          |  15 +++
 configure.ac       |   9 ++
 ctct.1.in          |  22 +++-
 ctct.in            | 287 +++++++++++++++++++++++++++++++++--------------------
 ctct_config.5.in   |   1 +
 9 files changed, 230 insertions(+), 120 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 1223dfb..ba0ca85 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -33,6 +33,7 @@ Configuration Files:
 
 Compatibility:
 --------------
+
 - Fixed incompatibility with OpenBSD.
  `- Simplify configure[.ac] (most notably get rid of `sed').
  `- Use `/usr/bin/env bash' in the shebang.
diff --git a/Makefile.in b/Makefile.in
index ed1cdeb..d3f205f 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -85,3 +85,5 @@ uninstall:
 	$(RM) $(DESTDIR)$(man5dir)/$(CONFIG)$(man5ext)
 
 .PHONY: install installdirs uninstall
+
+# vi: ts=2 sw=2
diff --git a/TODO b/TODO
index 1c732c5..01f78b8 100644
--- a/TODO
+++ b/TODO
@@ -1,16 +1,10 @@
 TODO file for ctct
 
 [GENERAL]
-* consider using an array for *_program to specify arguments
- `- to circumvent the necessity for eval
 * Honor $VISUAL.
+* Honor XDG user dirs.
 * Use git tags for future versions.
-* On `ctct -s <pattern>', do not match illegal sequences (e.g. a+b).
-* On `ctct -l', do only list legal names (e.g. not a+b).
-* Use the EXIT_* variables.
-* [consider] On `ctct -d', check format of string.
 * [consider] Remove character restriction except '.', '/'.
-* [consider] Use `grep -F' for --search-by-name.
 * [consider] Require at least one argument for --search-by-*.
 
 [CONFIGURE SCRIPT]
@@ -26,12 +20,11 @@ TODO file for ctct
  `- should be made customizable
 * [optional] Do not create a new entry if nothing is entered in the editor.
 * Support templates.
-* Different return types for errors and non-matches.
 * Quiet option for --search-by-*.
 * [consider] Use another default editor.
  `- nano, easily understandable.
  `- ed, the editor.
-* Honor XDG user dirs.
+* [consider] Use regexes for --search-by-name.
 
 [MANUAL]
 * Add an EXAMPLE section to ctct(1).
diff --git a/bash_completion.in b/bash_completion.in
index 54aa137..a8fa219 100644
--- a/bash_completion.in
+++ b/bash_completion.in
@@ -92,3 +92,5 @@ _ctct()
 }
 
 complete -F _ctct ctct
+
+# vi: ts=2 sw=2
diff --git a/configure b/configure
index b3b2ca9..35c1039 100755
--- a/configure
+++ b/configure
@@ -595,6 +595,10 @@ default_fallback_editor
 default_datadir
 sysconfdir_expanded
 bash_completion_dir
+ret_error
+ret_badsyntax
+ret_failure
+ret_success
 target_alias
 host_alias
 build_alias
@@ -1710,6 +1714,17 @@ ac_config_files="$ac_config_files ctct"
 
 
 
+# ret_success must be 0, all others nonzero.
+ret_success=0
+
+ret_failure=1
+
+ret_badsyntax=2
+
+ret_error=4
+
+
+
 
 test -z "$bash_completion_dir" \
 	&& bash_completion_dir='${datarootdir}/bash-completion/completions'
diff --git a/configure.ac b/configure.ac
index 70cf150..dd45df4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -29,6 +29,13 @@ AC_CONFIG_FILES(
 AC_CONFIG_FILES([ctct], [chmod +x ctct])
 
 
+# ret_success must be 0, all others nonzero.
+AC_SUBST([ret_success], 0)
+AC_SUBST([ret_failure], 1)
+AC_SUBST([ret_badsyntax], 2)
+AC_SUBST([ret_error], 4)
+
+
 AC_ARG_VAR([bash_completion_dir],
 					 ['directory to store bash-completion script in'])
 test -z "$bash_completion_dir" \
@@ -95,3 +102,5 @@ test -z "$default_confirm_default_yes" \
 
 
 AC_OUTPUT
+
+# vi: ts=2 sw=2
diff --git a/ctct.1.in b/ctct.1.in
index 5a9ba61..3ac3cb8 100644
--- a/ctct.1.in
+++ b/ctct.1.in
@@ -164,7 +164,7 @@ Show a simple help text basically depicting the above options\.
 .\"
 .SH EXIT STATUS
 .TP
-.B 0
+.B @ret_success@
 Successfull operation\.
 In case of
 .BR \-\-search\-by\-name
@@ -173,22 +173,34 @@ or
 at least one matching entry was found\.
 .\"
 .TP
-.B 1
-An error occurred or, in case of
-.B \-\-search\-by\-name
+.B @ret_failure@
+In case of
+.BR \-\-search\-by\-name
 or
 .BR \-\-search\-by\-data ,
 no matching entry was found\.
+Elsewise, the action did not succeed, for example due to the requested name
+not referring to an existing contact entry\.
+.\"
+.TP
+.B @ret_badsyntax@
+The used command contains bad syntax which will fail under any conditions\.
+.\"
+.TP
+.B @ret_error@
+An unexpected error occurred, for example due to missing permissions in
+.BR datadir \.
 .\"
 .\"
 .SH ENVIROMENT
 .TP
 .B EDITOR
-The editor to use when \fBctct\fP is called with the \fI\-\-edit\fP
+The editor to use when \fBctct\fP is called with the \fB\-\-edit\fP
 option and \fBdefault_editor\fP is not set in \fBctct_config\fP(5)\.
 .\"
 .\"
 .SH SEE ALSO
 .BR ctct_config (5),
 .BR grep (1)
+.\"
 .\" vi: tw=75
diff --git a/ctct.in b/ctct.in
index a705e59..74a40f8 100644
--- a/ctct.in
+++ b/ctct.in
@@ -37,11 +37,14 @@ test -f "$user_config_file" \
 
 ## CONSTANTS:
 exec_name="${0##*/}"
-EXIT_SUCCESS=0
-EXIT_FAILURE=1
-EXIT_ERROR=2
-TRUE=0
-FALSE=1
+RET_SUCCESS=@ret_success@
+RET_FAILURE=@ret_failure@
+RET_BADSYNTAX=@ret_badsyntax@
+RET_ERROR=@ret_error@
+
+
+# Expand non matching globs to the empty string instead of themselves.
+shopt -s nullglob
 
 
 function print_help() {
@@ -59,101 +62,125 @@ usage:
 EOF
 }
 
+
 function cleanup()
 {
 	test -v tmp_file && test -f "$tmp_file" && rm -f "$tmp_file"
 }
 trap cleanup EXIT
 
+
 function main()
 {
+	local ret
+
 	if ! test -d "$datadir" && ! mkdir "$datadir"
 	then
 		print_error "$exec_name: $datadir could not be created, quitting..."
-		exit 1
+		return $RET_ERROR
 	fi
-	test $# -eq 0 && print_help && exit 1
+	test $# -eq 0 && print_help && return $RET_BADSYNTAX
 	
 	if [ "$1" = "-h" ] || ( [[ "--help" =~ ^"$1" ]] && [ ${#1} -ge 3 ] )
 	then
-		print_help; exit 0
+		print_help; return $RET_SUCCESS
 	elif [[ "--version" =~ "$1" ]] && [ ${#1} -ge 3 ]
 	then
-		print_version; exit 0
+		print_version; return $RET_SUCCESS
 	elif [ "$1" = "-l" ] || \
 		( [[ "--list-all" =~ ^"$1" ]] && [ ${#1} -ge 3 ] )
 	then
-		list_all; exit $?
+		list_all; return $?
 	elif [ "$1" = "-s" ] || \
 		( [[ "--search-by-name" =~ ^"$1" ]] && [ ${#1} -ge 13 ] )
 	then
-		shift; find_similar "Found:" "$@"; exit $?
+		shift; search_by_name "Found:" "$@"; return $?
 	elif [ "$1" = "-S" ] || \
 		( [[ "--search-by-data" =~ ^"$1" ]] && [ ${#1} -ge 13 ] )
 	then
-		shift; search_file "Found:" "$@"; exit $?
+		shift; search_by_content "Found:" "$@"; return $?
 	elif [ "$1" = "-e" ] || ( [[ "--edit" =~ ^"$1" ]] && [ ${#1} -ge 3 ] )
 	then
 		test $# -lt 2 && print_error "$exec_name: no entry specified." \
-			&& exit 1
-		edit_file "$2"; exit $?
+			&& return $RET_BADSYNTAX
+		edit_contact "$2"; return $?
 	elif [ "$1" = "-d" ] || \
 		( [[ "--delete" =~ ^"$1" ]] && [ ${#1} -ge 3 ] )
 	then
 		test $# -lt 2 && print_error "$exec_name: no entry to be deleted." \
-			&& exit 1
-		shift; delete_file "$@"; exit $?
+			&& return $RET_BADSYNTAX
+		shift; delete_file "$@"; return $?
 	elif [[ "--rename" =~ ^"$1" ]] && [ ${#1} -ge 3 ]
 	then
 		( test $# -lt 2 && print_error "$exec_name: no entry specified." ) || \
 		 	( test $# -lt 3 && print_error "$exec_name: no new name specified." ) \
-			&& exit 1
-		rename_file "$2" "$3"; exit $?
+			&& return $RET_BADSYNTAX
+		rename_contact "$2" "$3"; return $?
 	elif [[ "$1" == '--' ]]
 	then
 		shift 1
-		test $# -eq 0 && print_help && exit 1
+		test $# -eq 0 && print_help && return $RET_BADSYNTAX
 	elif [[ "$1" =~ ^- ]]
 	then
-		print_error "$exec_name: unknown option '$1'"; exit 1
+		print_error "$exec_name: unknown option '$1'"; return $RET_BADSYNTAX
 	fi
 	
-	if ! find_exact "$1"
+	display_exact "$1"
+	ret=$?
+	if [ $ret -ne $RET_SUCCESS ]
 	then
-		if ! find_similar "Did you mean:" "$@"
+		if [ $ret -eq $RET_BADSYNTAX ]
+		then
+			return $RET_BADSYNTAX
+		fi
+
+		if ! search_by_name "Did you mean:" "$@"
 		then
 			print_msg "No match found."
 		fi
-		return 1
+		return $RET_FAILURE
 	fi
+
+	return $RET_SUCCESS
 }
 
+
+# $1: message
 function print_msg()
 {
 	printf "%s\n" "$@"
 }
 
+
+# $1: error message
 function print_error()
 {
 	printf "%s\n" "$@" >&2
 }
 
-function find_exact()
+
+# $1: contact name
+function display_exact()
 {
 	local file
-	file="$datadir/$(get_filename "$1")" || return $FALSE
+
+	check_syntax "$1" || return $RET_BADSYNTAX
+	file="$datadir/$(get_filename "$1")" || return $RET_FAILURE
 	
 	if test "$visual_program" = "cat"
 	then
-		$output_program < "$file"
+		$output_program < "$file" || return $RET_ERROR
 	else
-		$output_program < "$file" | $visual_program
+		$output_program < "$file" | $visual_program || return $RET_ERROR
 	fi
-	return $TRUE
+
+	return $RET_SUCCESS
 }
 
-# $1: initial success message;  $2-${$#}: search-patterns
-function find_similar()
+
+# $1: initial success message
+# ${@:2}: search-patterns
+function search_by_name()
 {
 	local found msg name bool file pattern
 
@@ -167,15 +194,11 @@ function find_similar()
 	#   else the reverse order (last.first) would have to be checked as well
 	if [[ "$*" =~ \. ]]
 	then
-		return $FALSE
+		return $RET_FAILURE
 	fi
-	
-	for file in "$datadir"/*
-	do
-		# Ignore non-regular files.
-		test -f "$file" || continue
 
-		name="${file##*/}"
+	for name in $(list_all || return $RET_ERROR)
+	do
 		bool=true
 		for pattern in "${@,,}"
 		do
@@ -191,60 +214,71 @@ function find_similar()
 			print_msg " $name"
 		fi
 	done
-	$found && return $TRUE || return $FALSE
+
+	$found && return $RET_SUCCESS || return $RET_FAILURE
 }
 
-function search_file()
+
+# $1: initial success message
+# ${@:2}: regular expressions
+function search_by_content()
 {
-	local found=false msg="$1" valid file pattern
-	shift	1					# skip $1
-	
-	for file in "$datadir"/*
-	do
-		# Ignore non-regular files.
-		test -f "$file" || continue
+	local msg files regexp ret
 
-		valid=true
-		if test "$output_program" = "cat"
-		then
-			for pattern in "$@"
-			do
-				! grep -qEi "$pattern" "$file" && valid=false && break
-			done
-		else
-			for pattern in "$@"
-			do
-			! $output_program < "$file" | \
-				grep -qEi "$pattern" "$file" && valid=false && break
-			done
-		fi
-		if $valid
-		then
-			! $found && print_msg "$msg" && found=true
-			print_msg " ${file##*/}"
-		fi
+	msg="$1"
+	shift	1
+
+	# Change to datadir to be able to treat file names and contact names
+	# equally.
+	cd "$datadir"
+	files=( $(list_all || return $RET_ERROR) )
+	cd -
+
+	for regexp in "${@,,}"
+	do
+		files=( $(grep -Eil "$regexp" ${files[@]}) )
+		ret=$?
+		[ $ret -eq 1 ] && return $RET_FAILURE
+		[ $ret -gt 1 ] && return $RET_ERROR
 	done
-	$found && return $TRUE || return $FALSE
+
+	print_msg "$msg"
+	printf " %s\n" ${files[@]}
+	return $RET_SUCCESS
 }
 
+
+# $1: [full_path]
 function list_all()
 {
-	# Use find instead of ls to avoid listing non-regular files.
-	find "$datadir" -maxdepth 1 -type f | sed 's|.*/||'
+	local fmt
+	[[ "$1" == 'full_path' ]] && fmt='%p\n' || fmt='%f\n'
+
+	# Only list regular files with valid names.
+	find "$datadir" -maxdepth 1 -type f -regextype posix-extended -regex \
+		'.*/[[:alpha:]]+([-_][[:alpha:]]+)*(\.[[:alpha:]]+([-_][[:alpha:]]+)*)?' \
+		-printf "$fmt"
+
+	[ $? -eq 0 ] && return $RET_SUCCESS || return $RET_ERROR
 }
 
-function edit_file()
+
+# $1: contact name
+function edit_contact()
 {
-	local file editor new=false
+	local file editor new
 	
+	check_syntax "$1" || return $RET_BADSYNTAX
+
+	new=false
 	if ! file="$datadir/$(get_filename "$1")"
 	then
-		if check_syntax "$1" && check_non_existance "$1"
+		if check_non_existance "$1"
 		then
 			file="$datadir/$1"
 			new=true
 		else
-			return $FALSE
+			return $RET_FAILURE
 		fi
 	fi
 	
@@ -260,41 +294,60 @@ function edit_file()
 	
 	if test "$input_program" = "cat" -a "$output_program" = "cat"
 	then
-		$new && touch "$file"		# vim does not save an empty file
-		$editor "$file"
+		if $new
+		then
+			# vim does not save an empty file.
+			touch "$file" || return $RET_ERROR
+		fi
+
+		$editor "$file" || return $RET_ERROR
 	else
-		tmp_file="$(mktemp)"
-		chmod 600 "$tmp_file"
-		! $new && $output_program < "$file" > "$tmp_file"
-		$editor "$tmp_file"
-		eval $input_program < "$tmp_file" > "$file"
-		rm -f "$tmp_file" && unset tmp_file
+		tmp_file="$(mktemp || return $RET_ERROR)"
+		chmod 600 "$tmp_file" || return $RET_ERROR
+
+		if ! $new
+		then
+			$output_program < "$file" > "$tmp_file" || return $RET_ERROR
+		fi
+
+		$editor "$tmp_file" || return $RET_ERROR
+		$input_program < "$tmp_file" > "$file" || return $RET_ERROR
+		rm -f "$tmp_file" && unset tmp_file || return $RET_ERROR
 	fi
+
+	return $RET_SUCCESS
 }
 
-function rename_file()
-{
-	check_syntax "$2" || return $FALSE
 
+# $1: old name
+# $2: new name
+function rename_contact()
+{
 	local file
+
+	check_syntax "$1" || return $RET_BADSYNTAX
+	check_syntax "$2" || return $RET_BADSYNTAX
+
 	if file="$datadir/$(get_filename "$1")"
 	then
 		if get_filename "$2" > /dev/null
 		then
 			print_error "$exec_name: Entry \"$2\" does already exist."
-			return $FALSE
+			return $RET_FAILURE
 		elif ! check_non_existance "$2"
 		then
-			return $FALSE
+			return $RET_FAILURE
 		else
-			mv "$file" "$datadir/$2"
+			mv "$file" "$datadir/$2" || return $RET_ERROR
 		fi
 	else
 		print_error "$exec_name: Entry \"$1\" does not exist."
-		return $FALSE
+		return $RET_FAILURE
 	fi
 }
 
+
+# $@: names of contacts to be deleted
 function delete_file()
 {
 	local name file str
@@ -304,17 +357,20 @@ function delete_file()
 	# check for existance of all to be deleted files
 	for name in "$@"
 	do
+		check_syntax "$@" || return $RET_BADSYNTAX
+
 		if file="$(get_filename "$name")"
 		then
 			files[i++]="$file"
 		else
 			print_error "$exec_name: Contact \"$1\" does not exist, aborting..."
-			return $FALSE
+			return $RET_FAILURE
 		fi
 	done
 	
-	if "$confirm_deletion"; then
-		# prepare confirmation
+	if $confirm_deletion
+	then
+		# Prepare confirmation.
 		if test $# -eq 1; then
 			str="Do you really want to delete the entry ${files[0]}"
 		else
@@ -325,19 +381,25 @@ function delete_file()
 			str="Are you sure to delete all these entries"
 		fi
 		
-		# confirm and delete
+		# Confirm and delete.
 		if confirm "$str"; then
 			for file in "${files[@]}"; do
-				rm "$datadir/$file"
+				rm "$datadir/$file" || return $RET_ERROR
 			done
 		fi
 	else # simply delete
 		for file in "${files[@]}"; do
-			rm "$datadir/$file"
+			rm "$datadir/$file" || return $RET_ERROR
 		done
 	fi
+
+	return $RET_SUCCESS
 }
 
+
+# For dot separated names (both parts interchangeable), get the permutation
+# on disk (e.g. last.first -> first.last).
+# $1: contact name
 function get_filename()
 {
 	local rev
@@ -351,13 +413,16 @@ function get_filename()
 		if test -f "$datadir/$rev"; then
 			printf "%s\n" "$rev"
 		else
-			return $FALSE
+			return $RET_FAILURE
 		fi
 	else
-		return $FALSE
+		return $RET_FAILURE
 	fi
+
+	return $RET_SUCCESS
 }
 
+
 # $1: confirmation string
 function confirm()
 {
@@ -371,40 +436,50 @@ function confirm()
 	
 	read var
 	if test -z "$var"; then
-		$confirm_default_yes && return $TRUE || return $FALSE
+		$confirm_default_yes && return $RET_SUCCESS || return $RET_FAILURE
 	fi
 	
 	var="${var,,}"
 	if test "$var" = "y" -o "$var" = "yes"; then
-		return $TRUE
+		return $RET_SUCCESS
 	else
-		return $FALSE
+		return $RET_FAILURE
 	fi
 }
 
+
+# $1: filename to be checked
+# $2: [silent]
 function check_syntax()
 {
-	if [[ "$1" =~ ^[A-Za-z]+([-_][A-Za-z]+)*(\.[A-Za-z]+([-_][A-Za-z]+)*)?$ ]]
+	if [[ "$1" =~ \
+		^[[:alpha:]]+([-_][[:alpha:]]+)*(\.[[:alpha:]]+([-_][[:alpha:]]+)*)?$ ]]
 	then
-		return $TRUE
+		return $RET_SUCCESS
 	else
-		print_error "$exec_name: invalid name \"$1\"" \
-			" An entry name may only contain letters," \
-			" '-' and '_' as separators and exactly one dot ('.')"
-		return $FALSE
+		if [[ "$2" != 'silent' ]]
+		then
+			print_error "$exec_name: invalid name \"$1\"" \
+				" An entry name may only contain letters," \
+				" '-' and '_' as separators and exactly one dot ('.')"
+		fi
+
+		return $RET_FAILURE
 	fi
 }
 
+
 # Only use this when $1 is assured not to be a regular file.
+# $1: filename (relative to datadir)
 function check_non_existance()
 {
 	if test -e "$datadir/$1"
 	then
 		print_error "$exec_name: file \"$1\" exists in $datadir," \
 			"however is not a regular file."
-		return $FALSE
+		return $RET_FAILURE
 	else
-		return $TRUE
+		return $RET_SUCCESS
 	fi
 }
 
diff --git a/ctct_config.5.in b/ctct_config.5.in
index 4dcf063..ccf146e 100644
--- a/ctct_config.5.in
+++ b/ctct_config.5.in
@@ -193,4 +193,5 @@ Default
 .BR vi (1),
 .BR less (1),
 .BR more (1)
+.\"
 .\" vi: tw=75
-- 
cgit v1.2.3