Skip to content

Conversation

@jasonbahl
Copy link
Collaborator

@jasonbahl jasonbahl commented Nov 17, 2025

What bug does this fix? Explain your changes.

This PR fixes a fatal error that occurs when updating WPGraphQL options via /wp-admin/options.php with empty string values.

What the bug was:
When a user navigates to /wp-admin/options.php and updates a WPGraphQL option (such as graphql_experiments_settings or graphql_general_settings) with an empty string value, WordPress triggers a fatal TypeError.

How it was manifesting:
The error message was:

PHP Fatal error: Uncaught TypeError: WPGraphQL\Admin\Settings\SettingsRegistry::sanitize_options(): Argument #1 ($options) must be of type array, string given

This occurred in the WordPress admin when submitting the options form, causing the entire admin dashboard to crash.

What the root cause was:
The sanitize_options() method in SettingsRegistry had a strict array type hint for its parameter. However, when WordPress calls the sanitization callback via the sanitize_option_{$option_name} filter (which happens when updating options through /wp-admin/options.php), it can pass the raw value directly - which may be a string (like an empty string '') rather than an array. This type mismatch caused the fatal error.

How your fix addresses it:
The fix modifies sanitize_options() to:

  1. Accept mixed type instead of strict array type
  2. Check if the input is an array before processing
  3. Return non-array values unchanged (since they don't need array-based sanitization)
  4. Only process array values through the existing sanitization logic

This ensures compatibility with WordPress's sanitization filter behavior while maintaining the existing functionality for array-based settings.

Does this close any currently open issues?

Closes #3439

Testing Strategy

This PR follows the preferred commit structure for bugfixes:

  1. First commit: Added a failing functional test (SettingsRegistryOptionsPageCept.php) that reproduces the bug by submitting the options form with empty string values
  2. Second commit: Implemented the fix in SettingsRegistry::sanitize_options() to handle non-array values gracefully

This approach demonstrates that:

  • The bug exists (test fails before the fix)
  • The fix resolves the issue (test passes after the fix)
  • The fix doesn't break existing functionality (all other tests continue to pass)

Test Results

Before/After Examples

Before (Buggy Behavior):

  1. Navigate to /wp-admin/options.php in WordPress admin
  2. Update graphql_experiments_settings or graphql_general_settings with an empty string value
  3. Submit the form
  4. Result: Fatal error page displayed, admin dashboard crashes
CleanShot 2025-11-17 at 11 17 07

After (No error):

  1. Navigate to /wp-admin/options.php in WordPress admin
  2. Update graphql_experiments_settings or graphql_general_settings with an empty string value
  3. Submit the form
  4. Result: Form Saves as expected
CleanShot 2025-11-17 at 11 16 43

…error (wp-graphql#3439)

Adds a test that reproduces the fatal error when updating WPGraphQL
options via /wp-admin/options.php with empty string values.

The test verifies that submitting the options form with empty string
values for registered WPGraphQL settings does not cause a TypeError
in SettingsRegistry::sanitize_options().

This test is expected to fail before the fix is applied, demonstrating
the issue where sanitize_options() receives a string instead of the
expected array when WordPress calls the sanitization callback.

Refs wp-graphql#3439
@jasonbahl jasonbahl changed the title fix: Add functional test for SettingsRegistry sanitize_options type error fix: Prevent fatal error when updating options via /wp-admin/options.php Nov 17, 2025
…p-graphql#3439)

WordPress may pass non-array values (like empty strings) to sanitization
callbacks when updating options via /wp-admin/options.php. This change
removes the strict array type hint and adds a type check to gracefully
handle non-array values, preventing fatal TypeError exceptions.

Refs wp-graphql#3439
@coveralls
Copy link

coveralls commented Nov 17, 2025

Coverage Status

coverage: 83.612% (+0.4%) from 83.228%
when pulling 7cccfc1 on jasonbahl:fix/3439-setting-registry-error
into 8c2b2b0 on wp-graphql:develop.

@jasonbahl jasonbahl merged commit 49ebc0f into wp-graphql:develop Nov 18, 2025
37 checks passed
pull bot pushed a commit to Zezo-Ai/wp-graphql that referenced this pull request Nov 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Updating an option via /wp-admin/options.php generates a FATAL ERROR

2 participants