Suspecting bug in /bin/sh's IFS

[Jilles Tjoelker]

On Thu, Mar 19, 2009 at 03:12:39PM +0100, Oliver Fromme wrote:
> Further debugging reveals that this is not a generic
> problem with field splitting, but it affects the read
> command only.

> I tried to use "set" instead of "read":

>  ORIG_IFS="$IFS"
>  while read line; do
>          IFS="$IFS="
>          set -- $line
>          IFS="$ORIG_IFS"
>          key="$1"
>          shift
>          val="$*"
>          echo "'$key'  --  '$val'"
>  done < config

> Now i get correct output for both " \t\n=" and "= \t\n"
> as the IFS value.  So the bug is in the "read" builtin.

> The following patch fixes the problem:

> --- bin/sh/miscbltin.c.orig     2006-02-04 15:37:50.000000000 +0100
> +++ bin/sh/miscbltin.c  2009-03-19 15:01:43.000000000 +0100
> @@ -188,7 +188,7 @@
>  		}
>  		if (c == '\n')
>  			break;
> -		if (startword && *ifs == ' ' && strchr(ifs, c)) {
> +		if (startword && strchr(ifs, c)) {
>  			continue;
>  		}
>  		startword = 0;

> The bug seems to exist for at least 15 years; the bogus
> comparison was already present in the initial import in
> the FreeBSD CVS repository in 1994.

> Any opinions on this?

The code is wrong, but your patched code is also wrong. The read builtin
should use the same logic as normal field splitting, with additional
rules if there are more fields in the input than variables.

I have noticed that NetBSD has already fixed this. I have ported these
fixes over: http://www.stack.nl/~jilles/unix/sh-read-split.patch
The patch is against RELENG_7, I hope it applies to -CURRENT as well.

The NetBSD commit message also refers to
http://www.research.att.com/~gsf/public/ifs.sh
Just like their /bin/sh, our /bin/sh with the patch now passes the
'read' tests from there (there are still many other failures though).

By the way, to avoid all processing by read, one must IFS= read -r VAR.
Without the IFS specification, leading and trailing whitespace will
still be stripped.

-- 
Jilles Tjoelker