From 02fff24b4fa27c500a964148d54f793ddc1df1cb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= <daniel@ekloef.se>
Date: Thu, 26 Oct 2023 16:22:41 +0200
Subject: [PATCH] config: improve validation of color values, default alpha to
 0xff

Reject color values that aren't in either RGB, or ARGB format. That
is, color values that aren't hexadecimal numbers with either 6 or 8
digits.

Also, if a color value is allowed to have an alpha component, and the
user left it out, default to 0xff (opaque) rather than 0x00 (fully
transparent).

Closes #1526
---
 CHANGELOG.md        | 12 ++++++++++++
 config.c            | 24 ++++++++++++++++++------
 tests/test-config.c |  4 ++++
 3 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4b68a804..0696b580 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -52,9 +52,21 @@
 ## Unreleased
 ### Added
 ### Changed
+
+* config: ARGB color values now default to opaque, rather than
+  transparent, when the alpha component has been left out
+  ([#1526][1526]).
+
+[1526]: https://codeberg.org/dnkl/foot/issues/1526
+
+
 ### Deprecated
 ### Removed
 ### Fixed
+
+* config: improved validation of color values.
+
+
 ### Security
 ### Contributors
 
diff --git a/config.c b/config.c
index 35c847e9..98d2918b 100644
--- a/config.c
+++ b/config.c
@@ -618,18 +618,30 @@ value_to_enum(struct context *ctx, const char **value_map, int *res)
 }
 
 static bool NOINLINE
-value_to_color(struct context *ctx, uint32_t *color, bool allow_alpha)
+value_to_color(struct context *ctx, uint32_t *result, bool allow_alpha)
 {
-    if (!str_to_uint32(ctx->value, 16, color)) {
-        LOG_CONTEXTUAL_ERR("not a valid color value");
+    uint32_t color;
+    const size_t len = strlen(ctx->value);
+    const size_t component_count = len / 2;
+
+    if (!(len == 6 || (allow_alpha && len == 8)) ||
+        !str_to_uint32(ctx->value, 16, &color))
+    {
+        if (allow_alpha) {
+            LOG_CONTEXTUAL_ERR("color must be in either RGB or ARGB format");
+        } else {
+            LOG_CONTEXTUAL_ERR("color must be in RGB format");
+        }
+
         return false;
     }
 
-    if (!allow_alpha && (*color & 0xff000000) != 0) {
-        LOG_CONTEXTUAL_ERR("color value must not have an alpha component");
-        return false;
+    if (allow_alpha && component_count == 3) {
+        /* If user left out the alpha component, assume non-transparency */
+        color |= 0xff000000;
     }
 
+    *result = color;
     return true;
 }
 
diff --git a/tests/test-config.c b/tests/test-config.c
index 666f4689..95abf1a4 100644
--- a/tests/test-config.c
+++ b/tests/test-config.c
@@ -426,6 +426,10 @@ test_color(struct context *ctx, bool (*parse_fun)(struct context *ctx),
         {"ffffff", 0xffffff},
         {"ffffffff", 0xffffffff, !alpha_allowed},
         {"aabbccdd", 0xaabbccdd, !alpha_allowed},
+        {"00", 0, true},
+        {"0000", 0, true},
+        {"00000", 0, true},
+        {"000000000", 0, true},
         {"unittest-invalid-color", 0, true},
     };