Background
PR #44 added defensive normalisation for theme_set / theme_set_allowed in the System theme-switch block (b_system_themes_show) via two helpers in htdocs/modules/system/blocks/system_blocks.php:
systemNormalizeThemeName(string $name): string
systemThemesResolveConfig(array $xoopsConfig): array
These guard the block against missing, malformed, or non-scalar xoops_config entries (string overrides, corrupted rows, mid-upgrade state).
Problem
The same config values are read raw in several other places that bypass the normalisation:
| File |
Line |
Pattern |
htdocs/include/common.php |
266 |
in_array($user_theme, $xoopsConfig['theme_set_allowed']) |
htdocs/include/checklogin.php |
69 |
in_array($user_theme, $xoopsConfig['theme_set_allowed']) |
htdocs/include/functions.php |
808 |
$xoopsThemeFactory->allowedThemes = $xoopsConfig['theme_set_allowed']; |
htdocs/include/site-closed.php |
42 |
same as above |
htdocs/class/xoopsform/formselecttheme.php |
45 |
$theme_arr = $GLOBALS['xoopsConfig']['theme_set_allowed']; |
htdocs/class/xoopskernel.php |
192, 201 |
in_array(..., xoops_getConfigOption('theme_set_allowed')) |
Result: the block-level dropdown can render a usable list while the runtime path may still reject (or accept!) themes inconsistently when the config row is corrupted.
Proposed work
Consolidate the normalisation so the block selector and the runtime checks share a single source of truth.
Approach (sketch)
- Move
systemNormalizeThemeName + systemThemesResolveConfig to a shared, eagerly-loaded location — most likely htdocs/include/functions.php (the stdlib already loaded by common.php and friends).
- Update each of the six call sites above to use the normalized allowed list instead of the raw config value.
- Decide on the contract: should
xoops_config be normalized once at hydration time (cleanest, but requires touching the config loader) or at each read site (more local, less risky)?
Out of scope for this issue
- Validating/rejecting themes that pass the directory-name regex but don't actually exist on disk (separate concern).
- Updating administrator-side theme-list editors (the saved value path).
References
Background
PR #44 added defensive normalisation for
theme_set/theme_set_allowedin the System theme-switch block (b_system_themes_show) via two helpers inhtdocs/modules/system/blocks/system_blocks.php:systemNormalizeThemeName(string $name): stringsystemThemesResolveConfig(array $xoopsConfig): arrayThese guard the block against missing, malformed, or non-scalar
xoops_configentries (string overrides, corrupted rows, mid-upgrade state).Problem
The same config values are read raw in several other places that bypass the normalisation:
htdocs/include/common.phpin_array($user_theme, $xoopsConfig['theme_set_allowed'])htdocs/include/checklogin.phpin_array($user_theme, $xoopsConfig['theme_set_allowed'])htdocs/include/functions.php$xoopsThemeFactory->allowedThemes = $xoopsConfig['theme_set_allowed'];htdocs/include/site-closed.phphtdocs/class/xoopsform/formselecttheme.php$theme_arr = $GLOBALS['xoopsConfig']['theme_set_allowed'];htdocs/class/xoopskernel.phpin_array(..., xoops_getConfigOption('theme_set_allowed'))Result: the block-level dropdown can render a usable list while the runtime path may still reject (or accept!) themes inconsistently when the config row is corrupted.
Proposed work
Consolidate the normalisation so the block selector and the runtime checks share a single source of truth.
Approach (sketch)
systemNormalizeThemeName+systemThemesResolveConfigto a shared, eagerly-loaded location — most likelyhtdocs/include/functions.php(the stdlib already loaded bycommon.phpand friends).xoops_configbe normalized once at hydration time (cleanest, but requires touching the config loader) or at each read site (more local, less risky)?Out of scope for this issue
References