fix(options): missing error check for global 'scl' and 'winhl' (#30919)

4 files changed, 63 insertions, 30 deletions

diff --git a/src/nvim/option.c b/src/nvim/option.c
index 933ee4ba75..5f0115b46c 100644
--- a/src/nvim/option.c
+++ b/src/nvim/option.c
@@ -1816,12 +1816,22 @@ void check_blending(win_T *wp)
 }
 
 /// Handle setting `winhighlight' in window "wp"
-bool parse_winhl_opt(win_T *wp)
+///
+/// @param winhl  when NULL: use "wp->w_p_winhl"
+/// @param wp     when NULL: only parse "winhl"
+///
+/// @return  whether the option value is valid.
+bool parse_winhl_opt(const char *winhl, win_T *wp)
 {
-  const char *p = wp->w_p_winhl;
+  const char *p = empty_string_option;
+  if (winhl != NULL) {
+    p = winhl;
+  } else if (wp != NULL) {
+    p = wp->w_p_winhl;
+  }
 
   if (!*p) {
-    if (wp->w_ns_hl_winhl && wp->w_ns_hl == wp->w_ns_hl_winhl) {
+    if (wp != NULL && wp->w_ns_hl_winhl && wp->w_ns_hl == wp->w_ns_hl_winhl) {
       wp->w_ns_hl = 0;
       wp->w_hl_needs_update = true;
     }
@@ -1829,24 +1839,27 @@ bool parse_winhl_opt(win_T *wp)
     return true;
   }
 
-  if (wp->w_ns_hl_winhl == 0) {
-    wp->w_ns_hl_winhl = (int)nvim_create_namespace(NULL_STRING);
-  } else {
-    // namespace already exist. invalidate existing items
-    DecorProvider *dp = get_decor_provider(wp->w_ns_hl_winhl, true);
-    dp->hl_valid++;
+  int ns_hl = 0;
+  if (wp != NULL) {
+    if (wp->w_ns_hl_winhl == 0) {
+      wp->w_ns_hl_winhl = (int)nvim_create_namespace(NULL_STRING);
+    } else {
+      // Namespace already exists. Invalidate existing items.
+      DecorProvider *dp = get_decor_provider(wp->w_ns_hl_winhl, true);
+      dp->hl_valid++;
+    }
+    wp->w_ns_hl = wp->w_ns_hl_winhl;
+    ns_hl = wp->w_ns_hl;
   }
-  wp->w_ns_hl = wp->w_ns_hl_winhl;
-  int ns_hl = wp->w_ns_hl;
 
   while (*p) {
-    char *colon = strchr(p, ':');
+    const char *colon = strchr(p, ':');
     if (!colon) {
       return false;
     }
     size_t nlen = (size_t)(colon - p);
-    char *hi = colon + 1;
-    char *commap = xstrchrnul(hi, ',');
+    const char *hi = colon + 1;
+    const char *commap = xstrchrnul(hi, ',');
     size_t len = (size_t)(commap - hi);
     int hl_id = len ? syn_check_group(hi, len) : -1;
     if (hl_id == 0) {
@@ -1857,14 +1870,18 @@ bool parse_winhl_opt(win_T *wp)
       return false;
     }
 
-    HlAttrs attrs = HLATTRS_INIT;
-    attrs.rgb_ae_attr |= HL_GLOBAL;
-    ns_hl_def(ns_hl, hl_id_link, attrs, hl_id, NULL);
+    if (wp != NULL) {
+      HlAttrs attrs = HLATTRS_INIT;
+      attrs.rgb_ae_attr |= HL_GLOBAL;
+      ns_hl_def(ns_hl, hl_id_link, attrs, hl_id, NULL);
+    }
 
     p = *commap ? commap + 1 : "";
   }
 
-  wp->w_hl_needs_update = true;
+  if (wp != NULL) {
+    wp->w_hl_needs_update = true;
+  }
   return true;
 }
 
@@ -2281,7 +2298,7 @@ static const char *did_set_number_relativenumber(optset_T *args)
     // When 'relativenumber'/'number' is changed and 'statuscolumn' is set, reset width.
     win->w_nrwidth_line_count = 0;
   }
-  check_signcolumn(win);
+  check_signcolumn(NULL, win);
   return NULL;
 }
 
@@ -5116,10 +5133,10 @@ void didset_window_options(win_T *wp, bool valid_cursor)
   fill_culopt_flags(NULL, wp);
   set_chars_option(wp, wp->w_p_fcs, kFillchars, true, NULL, 0);
   set_chars_option(wp, wp->w_p_lcs, kListchars, true, NULL, 0);
-  parse_winhl_opt(wp);  // sets w_hl_needs_update also for w_p_winbl
+  parse_winhl_opt(NULL, wp);  // sets w_hl_needs_update also for w_p_winbl
   check_blending(wp);
   set_winbar_win(wp, false, valid_cursor);
-  check_signcolumn(wp);
+  check_signcolumn(NULL, wp);
   wp->w_grid_alloc.blending = wp->w_p_winbl > 0;
 }
 
diff --git a/src/nvim/optionstr.c b/src/nvim/optionstr.c
index 23b70ab5ad..21a3a3877b 100644
--- a/src/nvim/optionstr.c
+++ b/src/nvim/optionstr.c
@@ -283,15 +283,27 @@ static bool valid_filetype(const char *val)
 
 /// Handle setting 'signcolumn' for value 'val'. Store minimum and maximum width.
 ///
+/// @param wcl  when NULL: use "wp->w_p_scl"
+/// @param wp   when NULL: only parse "scl"
+///
 /// @return OK when the value is valid, FAIL otherwise
-int check_signcolumn(win_T *wp)
+int check_signcolumn(char *scl, win_T *wp)
 {
-  char *val = wp->w_p_scl;
+  char *val = empty_string_option;
+  if (scl != NULL) {
+    val = scl;
+  } else if (wp != NULL) {
+    val = wp->w_p_scl;
+  }
+
   if (*val == NUL) {
     return FAIL;
   }
 
   if (check_opt_strings(val, p_scl_values, false) == OK) {
+    if (wp == NULL) {
+      return OK;
+    }
     if (!strncmp(val, "no", 2)) {  // no
       wp->w_minscwidth = wp->w_maxscwidth = SCL_NO;
     } else if (!strncmp(val, "nu", 2) && (wp->w_p_nu || wp->w_p_rnu)) {  // number
@@ -321,6 +333,9 @@ int check_signcolumn(win_T *wp)
     if (min < 1 || max < 2 || min > 8 || min >= max) {
       return FAIL;
     }
+    if (wp == NULL) {
+      return OK;
+    }
     wp->w_minscwidth = min;
     wp->w_maxscwidth = max;
   }
@@ -2099,8 +2114,9 @@ int expand_set_showcmdloc(optexpand_T *args, int *numMatches, char ***matches)
 const char *did_set_signcolumn(optset_T *args)
 {
   win_T *win = (win_T *)args->os_win;
+  char **varp = (char **)args->os_varp;
   const char *oldval = args->os_oldval.string.data;
-  if (check_signcolumn(win) != OK) {
+  if (check_signcolumn(*varp, varp == &win->w_p_scl ? win : NULL) != OK) {
     return e_invarg;
   }
   // When changing the 'signcolumn' to or from 'number', recompute the
@@ -2568,7 +2584,8 @@ const char *did_set_winbar(optset_T *args)
 const char *did_set_winhighlight(optset_T *args)
 {
   win_T *win = (win_T *)args->os_win;
-  if (!parse_winhl_opt(win)) {
+  char **varp = (char **)args->os_varp;
+  if (!parse_winhl_opt(*varp, varp == &win->w_p_winhl ? win : NULL)) {
     return e_invarg;
   }
   return NULL;
diff --git a/src/nvim/winfloat.c b/src/nvim/winfloat.c
index 054ef07fc5..4698487708 100644
--- a/src/nvim/winfloat.c
+++ b/src/nvim/winfloat.c
@@ -137,7 +137,7 @@ void win_set_minimal_style(win_T *wp)
                    ? xstrdup("EndOfBuffer:")
                    : concat_str(old, ",EndOfBuffer:"));
   free_string_option(old);
-  parse_winhl_opt(wp);
+  parse_winhl_opt(NULL, wp);
 
   // signcolumn: use 'auto'
   if (wp->w_p_scl[0] != 'a' || strlen(wp->w_p_scl) >= 8) {
diff --git a/test/old/testdir/gen_opt_test.vim b/test/old/testdir/gen_opt_test.vim
index 2cc9b3142a..51f260cc5e 100644
--- a/test/old/testdir/gen_opt_test.vim
+++ b/test/old/testdir/gen_opt_test.vim
@@ -45,8 +45,6 @@ endwhile
 let skip_setglobal_reasons = #{
       \ iminsert: 'The global value is always overwritten by the local value',
       \ imsearch: 'The global value is always overwritten by the local value',
-      \ signcolumn:	'TODO(nvim): fix missing error handling for setglobal',
-      \ winhighlight:	'TODO(nvim): fix missing error handling for setglobal',
       \}
 
 " Script header.
@@ -92,12 +90,13 @@ let test_values = {
       \		'noinsert', 'noselect', 'fuzzy', 'menu,longest'],
       \		['xxx', 'menu,,,longest,']],
       \ 'encoding': [['utf8'], []],
-      \ 'foldcolumn': [[0, 1, 4], [-1, 13, 999]],
+      \ 'foldcolumn': [[0, 1, 4, 'auto', 'auto:1', 'auto:9'], [-1, 13, 999]],
       \ 'foldlevel': [[0, 100], [-1, '']],
       \ 'highlight': [[&highlight], []],
       \ 'iminsert': [[0, 1], [-1, 2, 3, 999]],
       \ 'imsearch': [[-1, 0, 1], [-2, 2, 3, 999]],
-      \ 'signcolumn': [['auto', 'no', 'yes', 'number'], ['', 'xxx', 'no,yes']],
+      \ 'signcolumn': [['auto', 'no', 'yes', 'number', 'yes:1', 'auto:1-9'],
+      \		['', 'xxx', 'no,yes', 'auto:0-9', 'auto:9-1', 'auto:1-@']],
       \ 'writedelay': [[0, 100], [-1, '']],
       \
       "\ boolean options