Skip to content

Fix: Don't pass null $old_value to single-arg callable sanitizers - #2358

Open
AlexGStapleton wants to merge 1 commit into
developfrom
fix-base-fields-fix-missing-second-args
Open

Fix: Don't pass null $old_value to single-arg callable sanitizers#2358
AlexGStapleton wants to merge 1 commit into
developfrom
fix-base-fields-fix-missing-second-args

Conversation

@AlexGStapleton

Copy link
Copy Markdown
Member

Deprecated: intval(): Passing null to parameter #2 ($base) of type int is deprecated in wp-content/plugins/so-widgets-bundle/base/inc/fields/base.class.php on line 405

Fix deprecated intval() call when a field uses a callable sanitizer such as 'intval'. sanitize() passed $old_value as a second argument to every callable sanitizer, so a null $old_value was forwarded to intval()'s optional $base parameter, triggering a PHP 8.1+ deprecation and, when a non-null old value was present, silently converting with the wrong base.

Only pass $old_value to callables that declare a second required parameter:

  • Add private sanitize_callback_accepts_old_value() using reflection (ReflectionFunction / ReflectionMethod, incl. 'Class::method' strings), defaulting to false on reflection failure.
  • One-argument callables like 'intval' are now invoked with the value only; callables that genuinely require $old_value keep receiving it.
  • Uses the two-argument ReflectionMethod constructor for PHP 7.0 compatibility.

Add tests/BaseFieldSanitizeTest.php covering the intval sanitizer (null and non-null old value), one-param callables, and two-required-param callables (closure, array, and static method forms). The intval test reproduces the reported deprecation against the unpatched code.

Fix deprecated intval() call when a field uses a callable sanitizer such as
'intval'. sanitize() passed $old_value as a second argument to every callable
sanitizer, so a null $old_value was forwarded to intval()'s optional $base
parameter, triggering a PHP 8.1+ deprecation and, when a non-null old value
was present, silently converting with the wrong base.

Only pass $old_value to callables that declare a second required parameter:
- Add private sanitize_callback_accepts_old_value() using reflection
  (ReflectionFunction / ReflectionMethod, incl. 'Class::method' strings),
  defaulting to false on reflection failure.
- One-argument callables like 'intval' are now invoked with the value only;
  callables that genuinely require $old_value keep receiving it.
- Uses the two-argument ReflectionMethod constructor for PHP 7.0 compatibility.

Add `tests/BaseFieldSanitizeTest.php` covering the intval sanitizer (null and
non-null old value), one-param callables, and two-required-param callables
(closure, array, and static method forms). The intval test reproduces the
reported deprecation against the unpatched code.
@AlexGStapleton
AlexGStapleton requested a review from Misplon August 20, 2026 07:43
@AlexGStapleton AlexGStapleton self-assigned this Aug 20, 2026

@Misplon Misplon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified: new tests fail on develop (5), branch green (25). Closure, 'Class::method' and invokable branches checked. Premium CPT Builder's sanitize_reserved_post_types( $post_type, $old_value ) still receives $old_value. Also fixes a latent fatal: 1-arg internal sanitizers like 'strlen' throw ArgumentCountError on develop.

Change requested: the check is too broad.

PHP ignores extra arguments to user-defined functions. The deprecation, wrong-base and ArgumentCountError problems only occur with internal functions. But required >= 2 also strips $old_value from user callables that got it on develop:

  • function ( $value, $old_value = null ) — loses $old_value
  • function ( $value, ...$args ) — loses it
  • 'sanitize_title_with_dashes' (Premium lightbox) — fallback behaviour changes

Fix:

return ! ( $reflection->isInternal() && $reflection->getNumberOfRequiredParameters() < 2 );

Internal + fewer than 2 required params → value only. Everything else → two args, as on develop. Closure::fromCallable( 'intval' ) reflects internal on 8.3, so it is covered too.

Tests:

  1. The one-param data provider never registers the provided callables — the variadic wrapper is the sanitizer in every case. The one-param ReflectionMethod branch is untested.
  2. After the fix above, assert single-arg dispatch for internal callables only. Assert user callables (one-param, optional-second, variadic) still receive both args.

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.

2 participants