Automattic / Automattic/wp-super-cache

The admin UI writes cache_path by hand instead of through wp_cache_setting()

Open
#1,097 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

`inc/admin-ui.php:433-434` writes `$cache_path` to the config file by hand instead of calling `wp_cache_setting()`:

```php
$cache_path = preg_replace( '/[ <>\'\"\r\n\t\(\)\$\[\];#]/', '', $new_cache_path );
wp_cache_replace_line( '^ *\$cache_path', "\$cache_path = " . var_export( $cache_path, true ) . ";", $wp_cache_config_file );
```

The REST endpoint performs the same write correctly one line at a time:

```php
// rest/class.wp-super-cache-rest-update-settings.php:167
wp_cache_setting( 'cache_path', $cache_path );
```

## Why it matters

Since #1092, `wp_cache_setting()` escapes values with `var_export()` plus newline flattening. The admin-ui path bypasses that and reimplements a strip-then-export of its own, so the same setting is now written two different ways depending on which form saved it:

- **REST** preserves the value and escapes it.
- **Admin UI** deletes ` < > ' " \r \n \t ( ) $ [ ] ; #` from the path first, then exports what is left.

A cache path containing a space — `/var/www/my site/cache/` — is stored verbatim by REST and as `/var/www/mysite/cache/` by the admin UI. The two disagree about what is on disk for the same input, and neither is documented as canonical.

It is also the site most likely to drift: it has its own inline `var_export()`, so any future change to how `wp_cache_setting()` renders values silently leaves this one behind. That already happened once — #1092 added newline flattening and this line did not get it.

## Suggested fix

```php
wp_cache_setting( 'cache_path', $new_cache_path );
```

`wp_cache_setting()` already assigns `$GLOBALS[ $field ]`, so the local `$cache_path = …` assignment goes too.

Before dropping the strip, check what else depends on it. `$cache_path` is not only written to the config file — it is used to build filesystem paths and generated `.htaccess` rules, so the character removal may be doing double duty as path hygiene rather than only as escaping. If so, the hygiene part should stay and be commented as such, with only the escaping responsibility handed to `wp_cache_setting()`.

## Related

- #1095 covers the two remaining raw `wp_cache_replace_line()` callers that carry unvalidated values. This site is a third bypass but was not in that list, because its value is stripped rather than unvalidated — it is a consistency and drift problem rather than an escaping hole.

Suggested labels: `bug`, `ready-for-agent`.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in inc/admin-ui.php:433-434 and compare that write with wp_cache_setting() at rest/class.wp-super-cache-rest-update-settings.php:167. Trace how $cache_path is used for filesystem paths and generated .htaccess rules before deciding whether the character removal must remain as path hygiene. Done means both save paths use consistent escaping without breaking those consumers.

Written by the indexing model from the issue text.

Assessment

Tech stack
php, wordpress
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.