Automattic / Automattic/wp-super-cache
Route the last two raw config writes through wp_cache_setting()
- Dominant language
- PHP
- Stars
- 436
- Forks
- 130
- Avg merge
- 15h 11m
- Merged PRs (30d)
- 10
Description
## Summary
`wp_cache_setting()` now builds its config line with `var_export()` (#1092), so every caller of it writes a value the config file can read back. Two writes still bypass it and interpolate raw into PHP source. Both should route through `wp_cache_setting()` rather than gain another copy of the metacharacter strip.
There are ~70 direct `wp_cache_replace_line()` calls outside `wp-cache-phase2.php`. All but these two carry numeric, `(int)`-cast, whitelisted or format-validated values, so this is a two-site job, not a sweep.
## 1. `$wp_cache_mobile_groups` — `inc/htaccess.php:48`
```php
wp_cache_replace_line( '^ *\$wp_cache_mobile_groups', "\$wp_cache_mobile_groups = '" . implode( ', ', $mobile_groups ) . "';", $wp_cache_config_file );
```
Fed from `apply_filters( 'cached_mobile_groups', array() )` (`inc/admin-ui.php:629`), so the value is whatever a third-party filter returns.
This one is more than an escaping gap — **the writer and the reader disagree about the type.** `wp_cache_mobile_group()` (`wp-cache-phase2.php`) expects a nested array:
```php
foreach ( (array) $wp_cache_mobile_groups as $name => $group ) {
foreach ( (array) $group as $browser ) {
```
and the documented shape at `inc/admin-ui.php:630` agrees:
```php
// mobile_groups = array( 'apple' => array( 'ipod', 'iphone' ), 'nokia' => array( 'nokia5800', 'symbianos' ) );
```
But the writer `implode()`s it into a string. For the documented nested shape that yields the literal `'Array'` plus an "Array to string conversion" notice — so the grouping feature does not work as documented today, independently of any escaping concern.
The fix is the same as the escaping fix: pass the array to `wp_cache_setting()`, which takes the array branch and stores a real array literal via `var_export()`. Note the two lines above it in the same function already do this:
```php
wp_cache_setting( 'wp_cache_mobile_browsers', $mobile_browsers );
wp_cache_setting( 'wp_cache_mobile_prefixes', $mobile_prefixes );
```
Worth checking whether anything else reads `$wp_cache_mobile_groups` as a string before changing the stored type, and whether an existing config holding the string `'Array'` needs handling on upgrade.
## 2. `$wptouch_exclude_ua` — `plugins/wptouch.php:97`
```php
$browsers = implode( ',', bnc_wptouch_get_exclude_user_agents() );
wp_cache_replace_line( '^ *\$wptouch_exclude_ua', "\$wptouch_exclude_ua = '$browsers';", $wp_cache_config_file );
```
An implode of a third-party plugin's user-agent list, interpolated raw. Reachable only with WPtouch installed and an apostrophe or backslash somewhere in its exclude list — low, but it is a straight `wp_cache_setting( 'wptouch_exclude_ua', $browsers )` swap.
`plugins/wptouch.php:15` writes `$wptouch_browsers` the same way; that value comes from `bnc_wptouch_get_user_agents()` and deserves the same treatment while the file is open.
## Why route rather than strip
The three existing metacharacter strips (`inc/admin-ui.php:433`, `inc/settings-forms.php:358,368`) are what this codebase reached for last time. That approach is why the REST `cache_path` handler and six other string settings went unprotected for years — a strip has to be remembered at every new call site, and it was not. Routing through `wp_cache_setting()` makes the guarantee structural, and deletes a copy of the strip pattern rather than adding a fourth.
## Not in scope here
With the sink fixed, the strips at `inc/settings-forms.php:358` and `:368` are no longer load-bearing for `$wp_cache_debug_ip` and `$wp_super_cache_front_page_text`, and they silently mangle legitimate input — an apostrophe or parentheses in the front-page notification text still disappear. Removing them is a user-visible behaviour change with its own risk and belongs in its own issue.
Suggested labels: `bug`, `ready-for-agent`.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with inc/htaccess.php:48 and plugins/wptouch.php:15,97, then read wp_cache_setting() and the corresponding readers in wp-cache-phase2.php. Route the named values through wp_cache_setting(), preserve the documented nested mobile-groups shape, and check existing reads and configuration handling before changing its stored type. Done means these raw writes no longer interpolate third-party values and the relevant configuration values remain readable.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- backend, security
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100