bug#54263: 29.0.50; color-values-from-color-spec accepts whitespace in rgbi: specifications

[Philip Kaludercic <philipk@posteo.net>]

Mattias Engdegård <mattiase@acm.org> writes:

>> I haven't seen the function being used anywhere up until now (which
> doesn't have to mean anything), but considering that this is a
> relatively niche edge-case I would say that the bug should either be
> solved now or never.
>
> Let's solve it now then. I'm to blame, patch attached.

A related question is if something like

  (should (equal (color-values-from-color-spec "rgbi:0/0/0x0") nil))

should be accepted or not.  From looking at xfaces-tests.el I was
surprised to see that the exponential notation was intentional, but
there was no comment or test on the un-lispy 0x... notation that strtod
allows.

> From 0433c23ce12ea24bcb4de93b92b92070cf257310 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
> Date: Sun, 6 Mar 2022 10:50:27 +0100
> Subject: [PATCH] Don't accept leading whitespace in rgbi: colour specs
>
> `color-values-from-color-spec` (new in Emacs 28) erroneously accepted
> leading whitespace in rgbi: components.
>
> Reported by Philip Kaludercic.
>
> * src/xfaces.c (parse_float_color_comp): Disallow leading whitespace.
> * test/src/xfaces-tests.el
> (xfaces-internal-color-values-from-color-spec): Add test case.
> ---
>  src/xfaces.c             | 4 ++++
>  test/src/xfaces-tests.el | 3 ++-
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/src/xfaces.c b/src/xfaces.c
> index 8100bdb157..7fbc667dfd 100644
> --- a/src/xfaces.c
> +++ b/src/xfaces.c
> @@ -888,6 +888,10 @@ parse_hex_color_comp (const char *s, const char *e, unsigned short *dst)
>  static double
>  parse_float_color_comp (const char *s, const char *e)
>  {
> +  if (s >= e ||
> +      !(*s == '0' || *s == '1' || *s == '.' || *s == '+' || *s == '-'))
> +    /* No leading whitespace permitted.  */
> +    return -1;
>    char *end;
>    double x = strtod (s, &end);
>    return (end == e && x >= 0 && x <= 1) ? x : -1;
> diff --git a/test/src/xfaces-tests.el b/test/src/xfaces-tests.el
> index 31c0f021b2..1175874144 100644
> --- a/test/src/xfaces-tests.el
> +++ b/test/src/xfaces-tests.el
> @@ -47,7 +47,8 @@ xfaces-internal-color-values-from-color-spec
>                   '(0 32768 6554)))
>    (should (equal (color-values-from-color-spec "rgbi:1e-3/1.0e-2/1e0")
>                   '(66 655 65535)))
> -  (should (equal (color-values-from-color-spec "rgbi:0/0.5/10") nil)))
> +  (should (equal (color-values-from-color-spec "rgbi:0/0.5/10") nil))
> +  (should (equal (color-values-from-color-spec "rgbi:0/0/ 0") nil)))
>  
>  (provide 'xfaces-tests)

-- 
	Philip Kaludercic