Automattic / Automattic/wp-super-cache

Route the last two raw config writes through wp_cache_setting()

Open
#1,095 0 comments 0 reactions 0 assignees View on GitHub
bug ready-for-agent
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.