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

["Mattias Engdegård" <mattiase@acm.org>]

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

6 mars 2022 kl. 11.45 skrev Philip Kaludercic <philipk@posteo.net>:

>  (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.

We could disallow hex floats (such as 0x1 or 0XDEFP-16) too, but whether they are un-lispy or not should have no bearing on our decision because we are parsing an external representation that doesn't come from the Lisp world.

Finding an authoritative source for the rgbi: format proved elusive but it doesn't seem that X11 or NS allow hex floats so let's reject them. New patch attached.


[-- Attachment #2: 0001-Don-t-accept-whitespace-or-hex-floats-in-rgbi-colour.patch --]
[-- Type: application/octet-stream, Size: 2068 bytes --]

From 5356a4dd21e2d427cc3c17a0592b30f85b7af1db 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 whitespace or hex floats in rgbi: colour specs

`color-values-from-color-spec` (new in Emacs 28) erroneously accepted
leading whitespace and hex floats in rgbi: components.

Reported by Philip Kaludercic.

* src/xfaces.c (parse_float_color_comp): Disallow leading whitespace
and hex floats.
* test/src/xfaces-tests.el
(xfaces-internal-color-values-from-color-spec): Add test cases.
---
 src/xfaces.c             | 5 +++++
 test/src/xfaces-tests.el | 4 +++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/xfaces.c b/src/xfaces.c
index 8100bdb157..d43e2936a9 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -888,6 +888,11 @@ 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 == '-')
+      || (s + 1 < e && (s[1] == 'x' || s[1] == 'X')))
+    /* No leading whitespace or hex floats 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..fc0e731db4 100644
--- a/test/src/xfaces-tests.el
+++ b/test/src/xfaces-tests.el
@@ -47,7 +47,9 @@ 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))
+  (should (equal (color-values-from-color-spec "rgbi:0/0x0/0") nil)))
 
 (provide 'xfaces-tests)
 
-- 
2.32.0 (Apple Git-132)