From 4b45b49a702a5d956cc3660f91656d48f5a2ff57 Mon Sep 17 00:00:00 2001
From: Jilles Tjoelker <jilles@FreeBSD.org>
Date: Sun, 9 Jan 2011 21:07:30 +0000
Subject: [PATCH] sh: Remove special %builtin PATH entry. All builtins are now
 always found before a PATH search.

Most ash derivatives have an undocumented feature where the presence of an
entry "%builtin" in $PATH will cause builtins to be checked at that point of
the PATH search, rather than before looking at any directories as documented
in the man page (very old versions do document this feature).

I am removing this feature from sh, as it complicates the code, may violate
expectations (for example, /usr/bin/alias is very close to a forkbomb with
PATH=/usr/bin:%builtin, only /usr/bin/builtin not being another link saves
it) and appears to be unused (all the %builtin google code search finds is
in some sort of ash source code).

Note that aliases and functions took and take precedence above builtins.
Because aliases work on a lexical level they can only ever be overridden on
a lexical level (quoting or preceding 'builtin' or 'command'). Allowing
override of functions via PATH does not really fit in the model of sh and it
would work differently from %builtin if implemented.

Note: POSIX says special builtins are found before functions. We comply to
this because we do not allow functions with the same name as a special
builtin.

Silence from:	freebsd-hackers@ (message sent 20101225)
Discussed with:	dougb
---
 bin/sh/exec.c                             | 40 ++++-------------------
 tools/regression/bin/sh/execution/path1.0 | 15 +++++++++
 2 files changed, 22 insertions(+), 33 deletions(-)
 create mode 100644 tools/regression/bin/sh/execution/path1.0

diff --git a/bin/sh/exec.c b/bin/sh/exec.c
index e916080cdd4e..5d00bcce3040 100644
--- a/bin/sh/exec.c
+++ b/bin/sh/exec.c
@@ -92,7 +92,6 @@ struct tblentry {
 
 
 static struct tblentry *cmdtable[CMDTABLESIZE];
-static int builtinloc = -1;		/* index in path of %builtin, or -1 */
 int exerrno = 0;			/* Last exec error */
 
 
@@ -244,8 +243,7 @@ hashcmd(int argc __unused, char **argv __unused)
 	}
 	while ((name = *argptr) != NULL) {
 		if ((cmdp = cmdlookup(name, 0)) != NULL
-		 && (cmdp->cmdtype == CMDNORMAL
-		     || (cmdp->cmdtype == CMDBUILTIN && builtinloc >= 0)))
+		 && cmdp->cmdtype == CMDNORMAL)
 			delete_cmd_entry();
 		find_command(name, &entry, DO_ERR, pathval());
 		if (verbose) {
@@ -336,8 +334,8 @@ find_command(const char *name, struct cmdentry *entry, int act,
 			goto success;
 	}
 
-	/* If %builtin not in path, check for builtin next */
-	if (builtinloc < 0 && (i = find_builtin(name, &spec)) >= 0) {
+	/* Check for builtin next */
+	if ((i = find_builtin(name, &spec)) >= 0) {
 		INTOFF;
 		cmdp = cmdlookup(name, 1);
 		if (cmdp->cmdtype == CMDFUNCTION)
@@ -353,7 +351,7 @@ find_command(const char *name, struct cmdentry *entry, int act,
 	prev = -1;		/* where to start */
 	if (cmdp) {		/* doing a rehash */
 		if (cmdp->cmdtype == CMDBUILTIN)
-			prev = builtinloc;
+			prev = -1;
 		else
 			prev = cmdp->param.index;
 	}
@@ -365,19 +363,7 @@ find_command(const char *name, struct cmdentry *entry, int act,
 		stunalloc(fullname);
 		idx++;
 		if (pathopt) {
-			if (prefix("builtin", pathopt)) {
-				if ((i = find_builtin(name, &spec)) < 0)
-					goto loop;
-				INTOFF;
-				cmdp = cmdlookup(name, 1);
-				if (cmdp->cmdtype == CMDFUNCTION)
-					cmdp = &loc_cmd;
-				cmdp->cmdtype = CMDBUILTIN;
-				cmdp->param.index = i;
-				cmdp->special = spec;
-				INTON;
-				goto success;
-			} else if (prefix("func", pathopt)) {
+			if (prefix("func", pathopt)) {
 				/* handled below */
 			} else {
 				goto loop;	/* ignore unimplemented options */
@@ -484,8 +470,7 @@ hashcd(void)
 
 	for (pp = cmdtable ; pp < &cmdtable[CMDTABLESIZE] ; pp++) {
 		for (cmdp = *pp ; cmdp ; cmdp = cmdp->next) {
-			if (cmdp->cmdtype == CMDNORMAL
-			 || (cmdp->cmdtype == CMDBUILTIN && builtinloc >= 0))
+			if (cmdp->cmdtype == CMDNORMAL)
 				cmdp->rehash = 1;
 		}
 	}
@@ -505,13 +490,11 @@ changepath(const char *newval)
 	const char *old, *new;
 	int idx;
 	int firstchange;
-	int bltin;
 
 	old = pathval();
 	new = newval;
 	firstchange = 9999;	/* assume no change */
 	idx = 0;
-	bltin = -1;
 	for (;;) {
 		if (*old != *new) {
 			firstchange = idx;
@@ -522,19 +505,12 @@ changepath(const char *newval)
 		}
 		if (*new == '\0')
 			break;
-		if (*new == '%' && bltin < 0 && prefix("builtin", new + 1))
-			bltin = idx;
 		if (*new == ':') {
 			idx++;
 		}
 		new++, old++;
 	}
-	if (builtinloc < 0 && bltin >= 0)
-		builtinloc = bltin;		/* zap builtins */
-	if (builtinloc >= 0 && bltin < 0)
-		firstchange = 0;
 	clearcmdentry(firstchange);
-	builtinloc = bltin;
 }
 
 
@@ -555,9 +531,7 @@ clearcmdentry(int firstchange)
 		pp = tblp;
 		while ((cmdp = *pp) != NULL) {
 			if ((cmdp->cmdtype == CMDNORMAL &&
-			     cmdp->param.index >= firstchange)
-			 || (cmdp->cmdtype == CMDBUILTIN &&
-			     builtinloc >= firstchange)) {
+			     cmdp->param.index >= firstchange)) {
 				*pp = cmdp->next;
 				ckfree(cmdp);
 			} else {
diff --git a/tools/regression/bin/sh/execution/path1.0 b/tools/regression/bin/sh/execution/path1.0
new file mode 100644
index 000000000000..50829d629a4d
--- /dev/null
+++ b/tools/regression/bin/sh/execution/path1.0
@@ -0,0 +1,15 @@
+# $FreeBSD$
+# Some builtins should not be overridable via PATH.
+
+set -e
+T=$(mktemp -d ${TMPDIR:-/tmp}/sh-test.XXXXXX)
+trap 'rm -rf ${T}' 0
+echo '#!/bin/sh
+echo bad' >"$T/cd"
+chmod 755 "$T/cd"
+cd /bin
+oPATH=$PATH
+PATH=$T:$PATH:%builtin
+cd /
+PATH=$oPATH
+[ "$(pwd)" = / ]