bug#30005: 27.0.50; call-interactively doesn't work correctly if the interactive specification has an embedded null byte

[Philipp Stephani <p.stephani2@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4551 bytes --]

Eli Zaretskii <eliz@gnu.org> schrieb am Sa., 20. Jan. 2018 um 13:03 Uhr:

> > From: Philipp <p.stephani2@gmail.com>
> > Date: Sat, 06 Jan 2018 12:39:43 +0100
> >
> > (call-interactively (lambda (a b) (interactive "sa\0b\ns")))
> >
> > the prompt is only "a" and a `wrong-number-of-argument' signal is
> > raised.  This is because `call-interactively' copies the interactive
> > specification to a C string, ignoring embedded nulls.
>
> No, it copies the spec in its entirety, including embedded null bytes,
> but then _processes_ the result as a C string, taking the first null
> byte as the end of the string.
>
> Does the patch below look right, and give good results?
>

Yes, thanks. Just some minor nits inline to make the code shorter.


>
> diff --git a/src/callint.c b/src/callint.c
> index 2253cdf..3d2ed00 100644
> --- a/src/callint.c
> +++ b/src/callint.c
> @@ -288,7 +288,8 @@ invoke it.  If KEYS is omitted or nil, the return
> value of
>    ptrdiff_t next_event;
>
>    Lisp_Object prefix_arg;
> -  char *string;
> +  char *string, *string_end;
> +  ptrdiff_t string_len;
>

I think these days (where we require C99) we always declare variables when
we first use them.


>    const char *tem;
>
>    /* If varies[i] > 0, the i'th argument shouldn't just have its value
> @@ -396,6 +397,8 @@ invoke it.  If KEYS is omitted or nil, the return
> value of
>    /* SPECS is set to a string; use it as an interactive prompt.
>       Copy it so that STRING will be valid even if a GC relocates SPECS.
> */
>    SAFE_ALLOCA_STRING (string, specs);
> +  string_len = SBYTES (specs);
> +  string_end = string + string_len;
>
>    /* Here if function specifies a string to control parsing the
> defaults.  */
>
> @@ -418,7 +421,7 @@ invoke it.  If KEYS is omitted or nil, the return
> value of
>               if (!NILP (record_flag))
>                 {
>                   char *p = string;
> -                 while (*p)
> +                 while (p < string_end)
>                     {
>                       if (! (*p == 'r' || *p == 'p' || *p == 'P'
>                              || *p == '\n'))
> @@ -469,7 +472,7 @@ invoke it.  If KEYS is omitted or nil, the return
> value of
>       `funcall-interactively') plus the number of arguments the
> interactive spec
>       would have us give to the function.  */
>    tem = string;
> -  for (nargs = 2; *tem; )
> +  for (nargs = 2; tem < string_end; )
>      {
>        /* 'r' specifications ("point and mark as 2 numeric args")
>          produce *two* arguments.  */
> @@ -477,7 +480,7 @@ invoke it.  If KEYS is omitted or nil, the return
> value of
>         nargs += 2;
>        else
>         nargs++;
> -      tem = strchr (tem, '\n');
> +      tem = memchr (tem, '\n', string_len - (tem - string));
>

You can write the third argument as string_end - tem.


>        if (tem)
>         ++tem;
>        else
> @@ -503,9 +506,12 @@ invoke it.  If KEYS is omitted or nil, the return
> value of
>      specbind (Qenable_recursive_minibuffers, Qt);
>
>    tem = string;
> -  for (i = 2; *tem; i++)
> +  for (i = 2; tem < string_end; i++)
>      {
> -      visargs[1] = make_string (tem + 1, strcspn (tem + 1, "\n"));
> +      char *pnl = memchr (tem + 1, '\n', string_len - (tem + 1 - string));
>

Here you can write the third argument as string_end - (tem + 1).


> +      ptrdiff_t sz = pnl ? pnl - (tem + 1) : string_end - (tem + 1);
>

You can write the RHS as (pnl ? pnl : string_end) - (tem + 1).


> +
> +      visargs[1] = make_string (tem + 1, sz);
>        callint_message = Fformat_message (i - 1, visargs + 1);
>
>        switch (*tem)
> @@ -781,7 +787,7 @@ invoke it.  If KEYS is omitted or nil, the return
> value of
>           {
>             /* How many bytes are left unprocessed in the specs string?
>                (Note that this excludes the trailing null byte.)  */
> -           ptrdiff_t bytes_left = SBYTES (specs) - (tem - string);
> +           ptrdiff_t bytes_left = string_len - (tem - string);
>

Same (string-end - tem).


>             unsigned letter;
>
>             /* If we have enough bytes left to treat the sequence as a
> @@ -803,9 +809,9 @@ invoke it.  If KEYS is omitted or nil, the return
> value of
>        if (NILP (visargs[i]) && STRINGP (args[i]))
>         visargs[i] = args[i];
>
> -      tem = strchr (tem, '\n');
> +      tem = memchr (tem, '\n', string_len - (tem - string));
>

again, string_end - tem.


>        if (tem) tem++;
> -      else tem = "";
> +      else tem = string_end;
>      }
>    unbind_to (speccount, Qnil);
>
>

[-- Attachment #2: Type: text/html, Size: 6631 bytes --]