Skip to content

Guard ExcludedEncodeContentSC filter entry points against non-string input - #856

Open
MikeWard0321 wants to merge 1 commit into
CleanTalk:masterfrom
MikeWard0321:guard-excluded-encoder-non-string-input
Open

Guard ExcludedEncodeContentSC filter entry points against non-string input#856
MikeWard0321 wants to merge 1 commit into
CleanTalk:masterfrom
MikeWard0321:guard-excluded-encoder-non-string-input

Conversation

@MikeWard0321

Copy link
Copy Markdown

Summary

ExcludedEncodeContentSC::changeContentBeforeEncoderModify() and ::changeContentAfterEncoderModify() are registered directly as WordPress filter callbacks, so they receive whatever value the filter carries — which is not always a string. Neither checks before passing that value to preg_match_all() / preg_replace_callback().

Its sibling EncodeContentSC is attached to the same filters by the same ShortCodesService methods and already guards both of its entry points with exactly this check. This PR applies the same guard to ExcludedEncodeContentSC so the two classes behave consistently on the hooks they share.

The inconsistency

ShortCodesService registers both classes together:

public function addActionsBeforeModify($hook, $priority = 1)
{
    add_filter($hook, array($this->encode, 'changeContentBeforeEncoderModify'), $priority);
    add_filter($hook, array($this->shortcode_to_exclude, 'changeContentBeforeEncoderModify'), $priority);
}
Class changeContentBeforeEncoderModify() changeContentAfterEncoderModify()
EncodeContentSC guards (line 100) guards (line 145)
ExcludedEncodeContentSC no guard (line 77) no guard (line 102)

ExcludedEncodeContentSC::processTitleString() (line 879) also already handles the null case explicitly, so the class does anticipate non-string input elsewhere — just not at the two filter entry points.

Observed behaviour

With a null filter value, changeContentBeforeEncoderModify() passes it to isShortcodeInsideHtmlAttribute()preg_match_all() at line 521, then to preg_replace_callback() at line 85:

PHP Deprecated: preg_match_all(): Passing null to parameter #2 ($subject) of type string is deprecated
  in .../ContactsEncoder/Shortcodes/ExcludedEncodeContentSC.php:521
PHP Deprecated: preg_replace_callback(): Passing null to parameter #3 ($subject) of type string is deprecated
  in .../ContactsEncoder/Shortcodes/ExcludedEncodeContentSC.php:85

Both fire on every affected page render. On PHP 9 these become a TypeError rather than a deprecation.

Environment: Spam protection, Anti-Spam, FireWall by CleanTalk 6.85, WordPress 7.0.4, PHP 8.4.24, nginx + PHP-FPM. Line numbers above are against current master.

The change

Adds the existing guard to both entry points — 8 lines, no behaviour change for string input:

if ( ! is_string($content) ) {
    return $content;
}

Returning $content unmodified preserves filter semantics for any non-string value rather than coercing it, matching what EncodeContentSC already does.

Verification

Applied to a production site running 6.85 on PHP 8.4.24. Before: the deprecation pair on every render of an affected page. After: no deprecations across repeated requests to the same pages, with page output and rendered content unchanged.

…input

changeContentBeforeEncoderModify() and changeContentAfterEncoderModify() are
registered directly as WordPress filter callbacks by ShortCodesService, so they
receive whatever value the filter carries, which is not always a string.

Both then pass that value to preg_match_all() and preg_replace_callback()
without checking it. On PHP 8.1+ a null value emits "Passing null to parameter
#N ($subject) of type string is deprecated"; on PHP 9 the same call becomes a
TypeError.

EncodeContentSC, the sibling class attached to the same filters by the same
ShortCodesService methods, already guards both of its entry points with exactly
this check. This applies the same guard to ExcludedEncodeContentSC so the two
classes behave consistently on the hooks they share.
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.

1 participant