Skip to content

Array settings are stored with their internal whitespace collapsed #1096

Description

@donnchawp

Summary

wp_cache_setting()'s array branch flattens var_export() output with:

$text = preg_replace( '/[\s]+/', ' ', var_export( $value, true ) );

That regex does not distinguish var_export()'s own layout whitespace from whitespace inside the exported string elements, so any run of spaces, tabs or newlines in a stored value is collapsed to a single space. The write is lossy and silent.

input:      array( 'Mozilla  Foo', "a\tb" )
written:    $x = array ( 0 => 'Mozilla Foo', 1 => 'a b', );
read back:  array( 'Mozilla Foo', 'a b' )

wp_cache_sanitize_value() (inc/settings-forms.php:208-214) uses the identical pattern and has the same behaviour.

Why it exists

The config file is rewritten by wp_cache_replace_line(), which matches ^ *\$field one physical line at a time. An entry spread across two lines loses its tail on the next write of that field, leaving an orphan that stops the file parsing — and it is included by advanced-cache.php before WordPress boots, so that is a fatal error. Everything written there must therefore be one line.

Collapsing all whitespace is one way to guarantee that. It just also destroys data.

Concrete impact

wpsc_plugins (inc/plugins-cookies.php:63,80) stores plugin file paths relative to ABSPATH. A plugin living in a directory with a space in its name — wp-content/plugins/My Plugin/loader.php — is stored as wp-content/plugins/My Plugin/loader.php with the double space intact only if there was one; a path with two consecutive spaces, or a tab, is silently altered and no longer matches the file on disk.

wpsc_cookies is lower risk: cookie names cannot contain spaces per RFC 6265.

wp_cache_mobile_groups will start using this branch if #1095 is actioned as described, and mobile group names plausibly contain spaces.

For wp_cache_sanitize_value()'s five callers the collapse is mostly masked, because that function splits its input on /[\s,]+/ first, so elements never contain whitespace by the time they are exported. Note inc/settings-forms.php:220-224 substitutes ___ for spaces around the call and reverses it afterwards — that workaround exists to defeat the split, and incidentally sidesteps the collapse too.

Suggested fix

#1092 solved the same one-line constraint for the string branch losslessly, by escaping the line breaks rather than removing them:

$text = strtr( $text, array( "\r" => '\' . "\r" . \'', "\n" => '\' . "\n" . \'' ) );

Applying that to the whole var_export() output — for arrays as well as strings — satisfies the one-line rule without touching the data, because var_export()'s own layout newlines get escaped rather than collapsed.

Two things to check before shipping it:

  • The on-disk shape of every array setting changes. Existing configs keep working (they are only ever included, never parsed as text), but the "setting unchanged" short-circuit in wp_cache_replace_line() will miss once per affected setting, causing one extra rewrite as the format settles.
  • SettingsFormUpdatersTest::test_wp_cache_sanitize_value_splits_and_escapes pins the current collapsed output exactly and will need updating.

That test lives in the integration tier, so this needs make test-integration.

Suggested labels: bug, ready-for-agent.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugready-for-agentFully specified, ready for an AFK agent

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions