From 89fe76bf0bc4d3c7d3b1ecfdf842f9c1a2d727e8 Mon Sep 17 00:00:00 2001 From: David Stone Date: Wed, 27 May 2026 14:33:46 -0600 Subject: [PATCH] fix(credits): make get_default_custom_credit_html public; revert defensive coerce from #1279 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Setup Wizard fatal that #1279 worked around had a single-character root cause, not a Field-layer defect: Credits::get_default_custom_credit_html was declared `protected`, but inc/class-credits.php:103 registers the field default as the array callable `[$this, 'get_default_custom_credit_html']`. `WP_Ultimo\Settings::save_settings()` resolves field defaults with `is_callable($field_default)` from a different class scope. PHP returns `false` for `is_callable([$instance, 'protected_method'])` evaluated outside the declaring class. With the callable unresolvable, the literal array `[$instance, 'get_default_custom_credit_html']` survived as the field value, landed in `Field::validate_textarea_field()`, hit `addslashes()`, and raised: Uncaught TypeError: addslashes(): Argument #1 ($string) must be of type string, array given in inc/ui/class-field.php:504 The same broken JSON also poisoned the wizard data-state JSON used to initialise the Vue form, which is why the form rendered with no settings fields on a fresh install. Both symptoms — empty form and the save fatal — disappear once the method is public. Changes: - inc/class-credits.php: visibility flipped from `protected` to `public`, with a docblock explaining the field-default callable contract so this bug class cannot silently regress. - inc/ui/class-field.php: reverts the defensive `coerce_textarea_value()` added by #1279. The function was a downstream band-aid; with the source fixed, no non-string value can reach this validator from this code path. Preserves the two unrelated PHPCS lint cleanups from #1279's first commit (alignment at line 301 and blank line before block comment at line 466). - tests/WP_Ultimo/Credits_Test.php: adds two regression tests: * `test_get_default_custom_credit_html_is_callable_externally` asserts both `is_callable([$instance, 'method'])` and ReflectionMethod::isPublic so a future visibility downgrade fails CI. * `test_get_default_custom_credit_html_returns_non_empty_string` asserts the resolved default value is a string suitable for textarea validation. - tests/WP_Ultimo/UI/Field_Test.php: reverts the six textarea-coerce tests added by #1279; they covered the removed band-aid. Audit: surveyed every `'default' => [$this, '...']` and `'options' => [$this, '...']` callable across inc/. `Credits::get_default_custom_credit_html` was the only target with restricted visibility. The two other registered field defaults (`Domain_Manager::default_domain_mapping_instructions`, `Settings::get_default_company_country`) and all seven options callables are already public — no further code changes needed. Verification: vendor/bin/phpunit --filter 'Credits_Test' # 17/17 pass vendor/bin/phpcs inc/class-credits.php inc/ui/class-field.php \ tests/WP_Ultimo/Credits_Test.php # 0 errors vendor/bin/phpstan analyse inc/class-credits.php \ inc/ui/class-field.php tests/WP_Ultimo/Credits_Test.php # No errors Live-install repro on a fresh install before/after the change: - Before: wizard step 2 renders with no fields; submitting fatals at class-field.php:504 with the addslashes TypeError above. - After: all 11 fields render and save without error. --- inc/class-credits.php | 36 +++++++++--- inc/ui/class-field.php | 44 +-------------- tests/WP_Ultimo/Credits_Test.php | 43 ++++++++++++++ tests/WP_Ultimo/UI/Field_Test.php | 93 ------------------------------- 4 files changed, 72 insertions(+), 144 deletions(-) diff --git a/inc/class-credits.php b/inc/class-credits.php index 097e7dbba..64e0ef840 100644 --- a/inc/class-credits.php +++ b/inc/class-credits.php @@ -96,11 +96,11 @@ public function register_settings(): void { 'general', 'credits_custom_html', [ - 'title' => __('Custom Footer HTML', 'ultimate-multisite'), - 'desc' => __('HTML allowed. Use any text or link you prefer.', 'ultimate-multisite'), - 'type' => 'textarea', - 'allow_html' => true, - 'default' => [$this, 'get_default_custom_credit_html'], + 'title' => __('Custom Footer HTML', 'ultimate-multisite'), + 'desc' => __('HTML allowed. Use any text or link you prefer.', 'ultimate-multisite'), + 'type' => 'textarea', + 'allow_html' => true, + 'default' => [$this, 'get_default_custom_credit_html'], 'value' => function () { return $this->normalize_custom_credit_html( wu_get_setting('credits_custom_html', $this->get_default_custom_credit_html()) @@ -111,8 +111,8 @@ public function register_settings(): void { wu_get_setting('credits_custom_html', $this->get_default_custom_credit_html()) ); }, - 'placeholder' => __('Powered by Your Company', 'ultimate-multisite'), - 'require' => [ + 'placeholder' => __('Powered by Your Company', 'ultimate-multisite'), + 'require' => [ 'credits_enable' => 1, 'credits_type' => 'html', ], @@ -139,8 +139,28 @@ protected function normalize_custom_credit_html($html): string { /** * Returns the default custom credit HTML. + * + * Public so the array callable `[$this, 'get_default_custom_credit_html']` + * registered as a field default (see register_settings() above) resolves + * via `is_callable()` when checked from outside this class — for example + * in `WP_Ultimo\Settings::save_settings()`, which lives in a different + * class scope. + * + * `is_callable([$instance, 'protected_method'])` returns `false` outside + * the declaring class scope. When that happens, `Settings::save_settings()` + * never invokes the callable, the literal array `[$instance, 'method-name']` + * survives as the field default, and it is later persisted/used as the + * field value. For a textarea field this leaks into + * `Field::validate_textarea_field()` → `addslashes()`, fatally: + * "Uncaught TypeError: addslashes(): Argument #1 ($string) must be of + * type string, array given". + * + * The Setup Wizard symptom is twofold: the data-state JSON the Vue form + * is initialised from contains the unresolved callable array, so the form + * renders with no settings fields; clicking Continue still POSTs and + * triggers the fatal on save. */ - protected function get_default_custom_credit_html(): string { + public function get_default_custom_credit_html(): string { $name = (string) get_network_option(null, 'site_name'); $name = $name ?: __('this network', 'ultimate-multisite'); $url = is_multisite() ? get_site_url(get_main_site_id()) : network_home_url('/'); diff --git a/inc/ui/class-field.php b/inc/ui/class-field.php index fac1bb314..b3f16eab6 100644 --- a/inc/ui/class-field.php +++ b/inc/ui/class-field.php @@ -492,28 +492,13 @@ protected function validate_number_field($value) { /** * Cleans the value submitted via a textarea or wp_editor field for database insertion. * - * Defensive against non-string inputs: arrays, objects (including Closures and - * stdClass), and other scalar types can reach this validator when: - * - A form posts `name="field[]"` syntax that PHP parses into an array. - * - A previously-stored corrupted value (array / "[object Object]" / Closure) - * is read back from the database as the existing fallback in - * Settings::save_settings() for fields not in the current form step. - * - A filter on `wu_settings_fields_sanitization_rules` routes a non-textarea - * field type here. - * - * Without this guard, `addslashes()` / `stripslashes()` raise a TypeError on - * PHP 8+ ("Argument #1 ($string) must be of type string, array given"), - * which fatals the Settings save / Setup Wizard step. - * * @since 2.0.0 * - * @param mixed $value Value of the settings being represented by this field. + * @param string $value Value of the settings being represented by this field. * @return string */ protected function validate_textarea_field($value) { - $value = $this->coerce_textarea_value($value); - if ($this->allow_html) { return stripslashes(wp_filter_post_kses(addslashes($value))); } @@ -521,33 +506,6 @@ protected function validate_textarea_field($value) { return wp_strip_all_tags(stripslashes($value)); } - /** - * Coerces a textarea field value to a safe string. - * - * Returns an empty string for arrays/objects/null and casts scalars - * to string. This prevents TypeError fatals downstream in - * addslashes()/stripslashes() and matches the historical behaviour of - * silently dropping malformed values rather than propagating corruption. - * - * @since 2.5.2 - * - * @param mixed $value Raw value to coerce. - * @return string - */ - private function coerce_textarea_value($value): string { - - if (is_string($value)) { - return $value; - } - - if (is_null($value) || is_array($value) || is_object($value)) { - return ''; - } - - // Booleans, ints, floats — cast to their string form. - return (string) $value; - } - /** * Echo HTML attributes for the field. * diff --git a/tests/WP_Ultimo/Credits_Test.php b/tests/WP_Ultimo/Credits_Test.php index e9b791fc9..38d621cc0 100644 --- a/tests/WP_Ultimo/Credits_Test.php +++ b/tests/WP_Ultimo/Credits_Test.php @@ -276,4 +276,47 @@ public function test_init_registers_hooks(): void { $this->assertNotFalse(has_action('wp_footer', [$this->credits, 'render_frontend_footer'])); $this->assertNotFalse(has_action('login_footer', [$this->credits, 'render_frontend_footer'])); } + + /** + * Regression: the method registered as the `credits_custom_html` field default + * callable MUST be publicly callable from outside the Credits class. + * + * `WP_Ultimo\Settings::save_settings()` checks `is_callable($field_default)` + * from a different class scope. If this method becomes protected/private the + * check returns `false`, the unresolved array `[$instance, 'method-name']` + * survives as the field default, breaks the Setup Wizard data-state JSON, + * and produces a `TypeError: addslashes(): Argument #1 must be of type string, + * array given` fatal in `Field::validate_textarea_field()`. + */ + public function test_get_default_custom_credit_html_is_callable_externally(): void { + + $callable = [$this->credits, 'get_default_custom_credit_html']; + + $this->assertTrue( + is_callable($callable), + 'Credits::get_default_custom_credit_html must be publicly callable so the field-default array callable resolves in Settings::save_settings().' + ); + + // Reflection confirms the visibility contract. + $reflection = new \ReflectionMethod($this->credits, 'get_default_custom_credit_html'); + $this->assertTrue( + $reflection->isPublic(), + 'Credits::get_default_custom_credit_html must be declared public.' + ); + } + + /** + * Regression: invoking the field-default callable returns a non-empty string. + * + * Mirrors the call path in `Settings::save_settings()` and confirms the value + * that lands in the field is a string suitable for textarea validation. + */ + public function test_get_default_custom_credit_html_returns_non_empty_string(): void { + + $result = call_user_func([$this->credits, 'get_default_custom_credit_html']); + + $this->assertIsString($result); + $this->assertNotEmpty($result); + $this->assertStringContainsString('Powered by', $result); + } } diff --git a/tests/WP_Ultimo/UI/Field_Test.php b/tests/WP_Ultimo/UI/Field_Test.php index ebaa57adb..f83f45dcd 100644 --- a/tests/WP_Ultimo/UI/Field_Test.php +++ b/tests/WP_Ultimo/UI/Field_Test.php @@ -265,97 +265,4 @@ public function test_title_fallback_to_name(): void { // Accessing title should fallback to name $this->assertEquals('Field Name', $field->title); } - - /** - * Textarea sanitization must accept a plain string and round-trip safely. - */ - public function test_textarea_sanitization_string_value(): void { - $field = new Field('test_field', [ - 'type' => 'textarea', - 'default' => '', - ]); - - $field->set_value("Line one\nLine two"); - - $this->assertSame("Line one\nLine two", $field->get_value()); - } - - /** - * Regression: textarea sanitization must NOT fatal when given an array value. - * - * This reproduces the historical TypeError raised by addslashes() / stripslashes() - * on PHP 8+ when a stored DB value (or a malformed POST payload) reaches the - * textarea validator as an array. Expected behaviour is to coerce to an empty - * string rather than fatal the Settings save / Setup Wizard step. - */ - public function test_textarea_sanitization_coerces_array_to_empty_string(): void { - $field = new Field('test_field', [ - 'type' => 'textarea', - 'allow_html' => false, - 'default' => '', - ]); - - $field->set_value(['unexpected', 'array', 'value']); - - $this->assertSame('', $field->get_value()); - } - - /** - * Regression: wp_editor (allow_html=true) must also tolerate array input. - */ - public function test_wp_editor_sanitization_coerces_array_to_empty_string(): void { - $field = new Field('test_field', [ - 'type' => 'wp_editor', - 'allow_html' => true, - 'default' => '', - ]); - - $field->set_value(['

not

', '

a

', '

string

']); - - $this->assertSame('', $field->get_value()); - } - - /** - * Object values (Closures, stdClass) must coerce to empty string, not fatal. - */ - public function test_textarea_sanitization_coerces_object_to_empty_string(): void { - $field = new Field('test_field', [ - 'type' => 'textarea', - 'default' => '', - ]); - - $field->set_value((object) ['foo' => 'bar']); - - $this->assertSame('', $field->get_value()); - } - - /** - * Null values must coerce to empty string. - */ - public function test_textarea_sanitization_coerces_null_to_empty_string(): void { - $field = new Field('test_field', [ - 'type' => 'textarea', - 'default' => '', - ]); - - $field->set_value(null); - - $this->assertSame('', $field->get_value()); - } - - /** - * Scalar non-strings (int, float, bool) must cast to their string form. - */ - public function test_textarea_sanitization_casts_scalar_to_string(): void { - $field = new Field('test_field', [ - 'type' => 'textarea', - 'default' => '', - ]); - - $field->set_value(42); - $this->assertSame('42', $field->get_value()); - - $field->set_value(3.14); - $this->assertSame('3.14', $field->get_value()); - } }