whence -v canonicalises paths improperly

[McDutchie]

Commands:

(cd /; whence -v usr/bin/env)

Actual output:

usr/bin/env is //usr/bin/env

Expected output:

usr/bin/env is /usr/bin/env

Multiple slashes are usually ignored, but a double initial slash is permitted by POSIX to have a special meaning, and in fact has a special meaning on Cygwin (indicating a Windows UNC network path). So it should not be adding one for that reason alone. It's also ugly.

[McDutchie]

Related oddness:

$ pwd
/usr/local/src/ksh93/ksh
$ whence -v ../../../../../bin/sh
../../../../../bin/sh is /usr/local/src/ksh93/ksh/../../../../../bin/sh

Would expect:

../../../../../bin/sh is /bin/sh

libast has a pathcanon() function to canonicalise a path properly; see man src/lib/libast/man/path.3. whence -v does not use it in its half-hearted attempt to canonicalise the path. It probably should.

[McDutchie]

This function should do path canonicalisation properly, as it uses pathcanon. But it is not called for the reproducers above.

ksh/src/cmd/ksh93/sh/path.c

Lines 571 to 589 in 01c25cb

	char *path_fullname(Shell_t *shp,const char *name)
	{
	int len=strlen(name)+1,dirlen=0;
	char *path,*pwd;
	if(*name!='/')
	{
	pwd = path_pwd(shp,1);
	dirlen = strlen(pwd)+1;
	}
	path = (char*)malloc(len+dirlen);
	if(dirlen)
	{
	memcpy((void*)path,(void*)pwd,dirlen);
	path[dirlen-1] = '/';
	}
	memcpy((void*)&path[dirlen],(void*)name,len);
	pathcanon(path,0);
	return(path);
	}

[McDutchie]

The bug occurs here:

ksh/src/cmd/ksh93/bltins/whence.c

Lines 223 to 239 in db72f41

	if(path_search(shp,name,&pp,2+(aflag>1)))
	{
	cp = name;
	if((flags&P_FLAG) && *cp!='/')
	cp = 0;
	}
	else
	{
	cp = stakptr(PATH_OFFSET);
	if(*cp==0)
	cp = 0;
	else if(*cp!='/')
	{
	cp = path_fullname(shp,cp);
	tofree=1;
	}
	}

The root of the problem is that path_search(), called on line 223, prepends the present working directory (pwd) to any relative path, and the pwd always starts with a /. The *cp!='/' on line 234 then prevents path_fullname() from being called. So the fix is to remove that faulty check.

[McDutchie]

Actually, I force-pushed this fix out again (sorry if that inconvenienced anyone) because it's still incomplete.

Paths that are already absolute aren't canonicalised, e.g.:

$ ksh -c 'whence -v /usr/lib/../bin/./env'
/usr/lib/../bin/./env is /usr/lib/../bin/./env

The /opt/ast/bin virtual path for certain builtins doesn't work properly:

$ ksh -c 'whence -v /opt/ast/bin/cat; whence -v /opt/ast/./bin/cat'
/opt/ast/bin/cat is a shell builtin
ksh: whence: /opt/ast/./bin/cat: not found

But then that doesn't work when directly invoking commands (like /opt/ast/./bin/cat foo) either, so that's a separate bug.

A complete fix might be to check whether a path contains / anywhere first, and if so, check if it physically exists, and if so, canonicalise it before doing anything else with it.