From aa4674d9cdd0da846f7900aaf566fa6951ea0e98 Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Fri, 28 Aug 2026 12:18:08 +0700 Subject: [PATCH] fix(config): strip whitespace around comma-separated config values load_limits split a config list on ',' and kept the spaces, so 'list=cpu, mem, load' became ['cpu', ' mem', ' load']. glances.conf writes lists that way in its own comments, so following the shipped documentation produced a config that did not work. The loudest symptom is show/hide: every item is used as a re.fullmatch pattern, and a leading space makes the pattern match nothing. 'hide=sda2, loop.*' hid sda2 and silently kept showing every loop device. Quicklook also answered a bad list by falling back to AVAILABLE_STATS_LIST - more than the user asked for, and its two GPU entries flip the gpu_stats polling flags, so a typo started polling the GPU. It now falls back to the documented DEFAULT_STATS_LIST and names the offending entries in the warning. --- glances/plugins/plugin/model.py | 9 ++- glances/plugins/quicklook/__init__.py | 14 +++- tests/test_plugin_quicklook.py | 109 ++++++++++++++++++++++++++ 3 files changed, 128 insertions(+), 4 deletions(-) create mode 100644 tests/test_plugin_quicklook.py diff --git a/glances/plugins/plugin/model.py b/glances/plugins/plugin/model.py index 979cbc91..257ebb48 100644 --- a/glances/plugins/plugin/model.py +++ b/glances/plugins/plugin/model.py @@ -742,7 +742,14 @@ class GlancesPluginModel: try: self._limits[limit] = config.get_float_value(self.plugin_name, level) except ValueError: - self._limits[limit] = config.get_value(self.plugin_name, level).split(",") + # Strip each item: `list=cpu, mem, load` is how a comma-separated + # value is normally written, and glances.conf writes it that way in + # its own comments. A bare split kept the spaces, so ' mem' matched + # nothing and plugins that validate their list against a set of + # known names silently rejected a config the user got right. + self._limits[limit] = [ + item.strip() for item in config.get_value(self.plugin_name, level).split(",") + ] logger.debug(f"Load limit: {limit} = {self._limits[limit]}") return True diff --git a/glances/plugins/quicklook/__init__.py b/glances/plugins/quicklook/__init__.py index ea111a1e..c4077c10 100644 --- a/glances/plugins/quicklook/__init__.py +++ b/glances/plugins/quicklook/__init__.py @@ -109,9 +109,17 @@ class QuicklookPlugin(GlancesPluginModel): # Define the stats list self.stats_list = self.get_conf_value('list', default=self.DEFAULT_STATS_LIST) - if not set(self.stats_list).issubset(self.AVAILABLE_STATS_LIST): - logger.warning(f'Quicklook plugin: Invalid stats list: {self.stats_list}') - self.stats_list = self.AVAILABLE_STATS_LIST + # A misconfigured list falls back to the DEFAULT, not to everything available. + # Falling back to AVAILABLE_STATS_LIST answered a config mistake by showing MORE + # than the user asked for, and the two GPU entries in it flip the gpu_stats + # polling flags below — so a typo silently started polling the GPU. + unknown = [stat for stat in self.stats_list if stat not in self.AVAILABLE_STATS_LIST] + if unknown: + logger.warning( + f'Quicklook plugin: unknown stats in the list: {unknown} ' + f'(available: {self.AVAILABLE_STATS_LIST}), falling back to {self.DEFAULT_STATS_LIST}' + ) + self.stats_list = self.DEFAULT_STATS_LIST if "gpu_mem" in self.stats_list: gpu_stats.get_gpu_mem = True if "gpu_proc" in self.stats_list: diff --git a/tests/test_plugin_quicklook.py b/tests/test_plugin_quicklook.py new file mode 100644 index 00000000..6bb41af8 --- /dev/null +++ b/tests/test_plugin_quicklook.py @@ -0,0 +1,109 @@ +#!/usr/bin/env python +# +# Glances - An eye on your system +# +# SPDX-FileCopyrightText: 2026 Nicolas Hennion +# +# SPDX-License-Identifier: LGPL-3.0-only +# + +"""Tests for the Quicklook plugin stats list configuration.""" + +import os + +import pytest + +from glances.config import Config +from glances.plugins.quicklook import QuicklookPlugin + + +@pytest.fixture +def plugin_for(tmp_path): + """Build a Quicklook plugin from a `[quicklook] list=...` config value.""" + + def build(list_value): + config_file = tmp_path / f'glances-{abs(hash(list_value))}.conf' + config_file.write_text(f'[quicklook]\nlist={list_value}\n', encoding='utf-8') + return QuicklookPlugin(args=None, config=Config(config_dir=os.fspath(config_file))) + + return build + + +class TestQuicklookStatsList: + def test_a_plain_list_is_honoured(self, plugin_for): + assert plugin_for('cpu,mem,load').stats_list == ['cpu', 'mem', 'load'] + + @pytest.mark.parametrize( + 'list_value', + [ + 'cpu, mem, load', + 'cpu ,mem ,load', + ' cpu , mem , load ', + 'cpu,\tmem,\tload', + ], + ) + def test_whitespace_around_items_is_ignored(self, plugin_for, list_value): + """`list=cpu, mem, load` is how anyone writes a comma-separated value. + + glances.conf writes it that way in its own `# Available stats are:` comment, so + a user copying that line got a list where every item but the first carried a + leading space, matched nothing, and was discarded whole. + """ + assert plugin_for(list_value).stats_list == ['cpu', 'mem', 'load'] + + def test_a_typo_falls_back_to_the_default_not_to_everything(self, plugin_for): + """A config mistake must not answer by displaying MORE than was asked for. + + The fallback used to be AVAILABLE_STATS_LIST, which includes both GPU entries — + and those flip the gpu_stats polling flags. A single typo therefore started + polling the GPU on a machine whose owner never asked for it. + """ + plugin = plugin_for('cpu,mem,typo') + + assert plugin.stats_list == QuicklookPlugin.DEFAULT_STATS_LIST + assert 'gpu_mem' not in plugin.stats_list + assert 'gpu_proc' not in plugin.stats_list + + def test_the_gpu_entries_are_still_selectable_on_purpose(self, plugin_for): + """The fallback must not become a filter: an explicit request still works.""" + assert plugin_for('cpu,gpu_mem,gpu_proc').stats_list == ['cpu', 'gpu_mem', 'gpu_proc'] + + def test_no_list_configured_uses_the_default(self, tmp_path): + config_file = tmp_path / 'glances-empty.conf' + config_file.write_text('[quicklook]\ndisable=False\n', encoding='utf-8') + plugin = QuicklookPlugin(args=None, config=Config(config_dir=os.fspath(config_file))) + + assert plugin.stats_list == QuicklookPlugin.DEFAULT_STATS_LIST + + +class TestConfigListParsing: + """The strip belongs to `load_limits`, so every plugin reading a list gets it.""" + + def test_limits_carry_stripped_items(self, plugin_for): + assert plugin_for('cpu, mem, load').get_limits('list') == ['cpu', 'mem', 'load'] + + def test_a_hide_pattern_written_with_spaces_still_hides(self, tmp_path): + """The show/hide filters are the loudest symptom of the same parsing bug. + + Every item in those lists is used as a `re.fullmatch` pattern, so a leading + space made the pattern match nothing at all — `hide=sda2, loop.*` hid `sda2` + and silently kept showing every loop device. + """ + from glances.plugins.diskio import DiskioPlugin + + config_file = tmp_path / 'glances-hide.conf' + config_file.write_text('[diskio]\nhide=sda2, loop.*\n', encoding='utf-8') + plugin = DiskioPlugin(args=None, config=Config(config_dir=os.fspath(config_file))) + + assert plugin.get_conf_value('hide') == ['sda2', 'loop.*'] + assert plugin.is_hide('sda2') + assert plugin.is_hide('loop0') + assert not plugin.is_hide('sda1') + + def test_internal_spaces_are_kept(self, tmp_path): + """Only the edges are stripped — a value may legitimately contain spaces.""" + config_file = tmp_path / 'glances-alias.conf' + config_file.write_text('[quicklook]\nlist=cpu\nalias=sda1:System Disk , sdb1:Data Disk\n', encoding='utf-8') + plugin = QuicklookPlugin(args=None, config=Config(config_dir=os.fspath(config_file))) + + assert plugin.get_limits('alias') == ['sda1:System Disk', 'sdb1:Data Disk']