PATCH: Better handling for (yet another) command substitution failure

PATCH: Better handling for (yet another) command substitution failure

[Peter Stephenson]

Entirely coincidentally (the problem isn't new and the fact that I'm
adding another error message exactly parallel to the one I added two
hours ago is just plain weird) another problem with command substitution
just turned up.  This handles the error case it's hitting better.  I
still need to look and see if it should be even getting here, and if it
shouldn't I'll fix that and add a corresponding test, but a hard error
at this point is much safer.

Also fix the capitalization convention for errors...

pws

diff --git a/Src/subst.c b/Src/subst.c
index 056b12b..176a004 100644
--- a/Src/subst.c
+++ b/Src/subst.c
@@ -264,7 +264,7 @@ stringsubst(LinkList list, LinkNode node, int pf_flags, int asssub)
 		while (*str != Outparmath && *str)
 		    str++;
 		if (*str != Outparmath) {
-		    zerr("Failed to find end of math substitution");
+		    zerr("failed to find end of math substitution");
 		    return NULL;
 		}
 		str[-1] = '\0';
@@ -278,8 +278,12 @@ stringsubst(LinkList list, LinkNode node, int pf_flags, int asssub)
 		endchar = c;
 		*str = '\0';
 
-		while (*++str != endchar)
-		    DPUTS(!*str, "BUG: parse error in command substitution");
+		while (*++str != endchar) {
+		    if (!*str) {
+			zerr("failed to find end of command substitution");
+			return NULL;
+		    }
+		}
 	    }
 	    *str++ = '\0';
 

Re: PATCH: Better handling for (yet another) command substitution failure

[Peter Stephenson]

On Thu, 19 Feb 2015 11:40:31 +0000
Peter Stephenson <p.stephenson@samsung.com> wrote:
> Entirely coincidentally (the problem isn't new and the fact that I'm
> adding another error message exactly parallel to the one I added two
> hours ago is just plain weird) another problem with command substitution
> just turned up.  This handles the error case it's hitting better.  I
> still need to look and see if it should be even getting here, and if it
> shouldn't I'll fix that and add a corresponding test...

That was certainly worth doing.

The problem is when we're looking for a string of multibyte characters
within a metafied string we don't take account of the fact that anything
that's not metafied but is marked as a metacharacter can't be part of a
real character --- it can only be a token, which is a standalone 8-bit
character that's not from the current character set.  So ensure the
multibyte character terminates before --- trivial fix.  This means the
character must be invalid or we wouldn't still be waiting for bytes;
there's already logic for handling invalid characters.

This showed up in some cleverly chosen parameter names with an invalid
UTF-8 character followed by tokenised input, as in the test I got a
private report about.

Also pendantically strip some unnecessary terminating whitespace.
(This got me into troubles with an earlier test I won't bother telling
you about...)

pws

diff --git a/Src/utils.c b/Src/utils.c
index 702829c..1bcceb0 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -4797,6 +4797,14 @@ mb_metacharlenconv_r(const char *s, wint_t *wcp, mbstate_t *mbsp)
 	    inchar = *++ptr ^ 32;
 	    DPUTS(!*ptr,
 		  "BUG: unexpected end of string in mb_metacharlen()\n");
+	} else if (imeta(*ptr)) {
+	    /*
+	     * As this is metafied input, this is a token --- this
+	     * can't be a part of the string.  It might be
+	     * something on the end of an unbracketed parameter
+	     * reference, for example.
+	     */
+	    break;
 	} else
 	    inchar = *ptr;
 	ptr++;
diff --git a/Test/D07multibyte.ztst b/Test/D07multibyte.ztst
index 2cb9953..33e76be 100644
--- a/Test/D07multibyte.ztst
+++ b/Test/D07multibyte.ztst
@@ -448,20 +448,30 @@
 0:read passes through invalid multibyte characters
 >0xC5
 
-  word=abcま          
+  word=abcま
   word[-1]=
   print $word
-  word=abcま 
+  word=abcま
   word[-2]=
   print $word
-  word=abcま 
+  word=abcま
   word[4]=d
   print $word
-  word=abcま 
+  word=abcま
   word[3]=not_c
-  print $word  
+  print $word
 0:assignment with negative indices
 >abc
 >abま
 >abcd
 >abnot_cま
+
+  # The following doesn't necessarily need UTF-8, but this gives
+  # us the full effect --- if we parse this wrongly the \xe9
+  # in combination with the tokenized input afterwards looks like a
+  # valid UTF-8 character.  But it isn't.
+  print $'$\xe9#``' >test_bad_param
+  (setopt nonomatch
+  . ./test_bad_param)
+127:Invalid parameter name with following tokenized input
+?./test_bad_param:1: command not found: $\M-i#