parser: Add VSBIT to ensure subtype is never zero

parser: Add VSBIT to ensure subtype is never zero

Harald van Dijk <harald@gigawatt.nl> wrote:
> On 21/11/2022 13:08, Harald van Dijk wrote:
>> On 21/11/2022 02:38, Christoph Anton Mitterer wrote:
>>> reject_filtered_cmd()
>>> {
>>> 	reject_and_die "disallowed command${restrict_path_list:+
>>> (restrict-path: \"${restrict_path_list//|/\", \"}\")}"
>>> }
>>>
>>> reject_filtered_cmd
>>[...]
>> This should either result in the ${...//...} being skipped, or the "Bad
>> substitution" error. Currently, what happens instead is it attempts, but
>> fails, to skip the ${...//...}.
>
> The reason it fails is because the word is cut off.
>
> Variable substitutions are encoded as a CTLVAR special character,
> followed by a byte indicating the type of substitution, followed by the
> rest of the substitution data. The type of substitution is the VSNORMAL,
> VSMINUS, etc. seen in parser.h. An invalid substitution is encoded as a
> value of 0.
>
> When we define a function, we clone the function body in order to
> preserve it. Cloning the function body is done by cloning each node.
> Cloning a "word" node (NARG) involves copying the characters that make
> up the word up to and including the terminating null byte.
>
> These two interact badly. The invalid substitution is seen as
> terminating the word, the rest of the word is not copied, but the
> expansion code does not have any way of seeing that anything got cut off
> and happily continues attempting to process the rest of the word.
>
> If dash decides to issue an error in this case, this is not a problem:
> the null byte is guaranteed to be copied, and if processing is
> guaranteed to stop if a null byte is encountered, everything works out.
>
> If dash decides to not issue an error in this case, the encoding of bad
> substitutions needs to change to a non-null byte. It appears that if we
> set the byte to VSNUL, the expansion logic is already able to handle it,
> but I have not tested this extensively.

Thanks for the analysis Harald!

This patch does basically what you've described except it uses a new
bit to avoid any confusion with a genuine VSNUL.

Fixes: 3df3edd13389 ("[PARSER] Report substition errors at...")
Reported-by: Christoph Anton Mitterer <calestyo@scientia.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Cheers,

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>