Fix infinite recursion in option access monitoring#73
Merged
Conversation
Add a recursion guard ($is_processing flag) to both monitor_option_accesses_pre_option and monitor_option_accesses_legacy to prevent re-entrant calls from causing a stack overflow. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Test on Playground |
jdevalk
approved these changes
Mar 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Users report a fatal error during plugin updates:
Root cause
The option access monitoring hooks (
pre_optionfilter andallhook) have no re-entrancy protection. When our filter callback executes and anything in the call stack triggers anotherget_option()call, the filter fires again, creating infinite recursion.The recursion chain:
get_option('some_option')fires thepre_optionfilter (orallhook)add_option_usage()get_option()callThis is a latent bug since v1.0
Checked the code at
v1.0,v1.5.1, andv1.6.0— none of them had recursion protection. Theallhook has always been vulnerable. The bug simply never triggered in test environments because it requires a specific combination of:get_option()during filter dispatchget_option()for connection settingsget_option('WPLANG')Fix
Adds a
$is_processingflag to thePluginclass. Bothmonitor_option_accesses_pre_optionandmonitor_option_accesses_legacycheck this flag and bail out immediately if already processing, preventing re-entrant calls.Why this works
The flag is set before
add_option_usage()and cleared after. Anyget_option()call that occurs during that window will see$is_processing = trueand return immediately without entering the monitoring logic. This breaks the recursion chain while only skipping the nested call — the original monitoring call completes normally.Test plan
🤖 Generated with Claude Code