Skip to content

fix: enhance ProxySetHandlerTargetCheck to prevent infinite recursion with WeakSet#228

Merged
piquark6046 merged 2 commits intomainfrom
fix/max-call-stack
Mar 23, 2026
Merged

fix: enhance ProxySetHandlerTargetCheck to prevent infinite recursion with WeakSet#228
piquark6046 merged 2 commits intomainfrom
fix/max-call-stack

Conversation

@piquark6046
Copy link
Member

No description provided.

@piquark6046 piquark6046 marked this pull request as ready for review March 23, 2026 06:49
Copilot AI review requested due to automatic review settings March 23, 2026 06:49
@piquark6046 piquark6046 merged commit 0f3ecfd into main Mar 23, 2026
10 checks passed
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the userscript’s Proxy set-handler target inspection logic to avoid infinite recursion when traversing cyclic object graphs (now tracking visited objects via WeakSet).

Changes:

  • Add WeakSet-based cycle detection to ProxySetHandlerTargetCheck to prevent unbounded recursion.
  • Remove the ProxySetHandlerTargetCheckAndReplace helper and the corresponding Proxy set-handler branch that performed in-place string replacement.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +297 to 301
for (const PropertyName of Object.keys(Target)) {
const Value = (Target as Record<string, unknown>)[PropertyName]
const Descriptor = OriginalObjectGetOwnPropertyDescriptor(Target, PropertyName)

if (
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

ProxySetHandlerTargetCheck reads Target[PropertyName] before checking whether the property is an accessor (descriptor.get). If the property has a getter, this access will invoke it (side effects / possible exceptions) even though the later logic tries to skip getters. Consider fetching the descriptor first and, when it has a getter, skipping the property without reading it; for data descriptors, prefer descriptor.value to avoid accidental getter execution.

Copilot uses AI. Check for mistakes.
Comment on lines 418 to 422
console.debug(`[${UserscriptName}]: Proxy set called for PowerLink Skeleton (target check):`, SetArgs)
BrowserWindow.document.dispatchEvent(new CustomEvent('PL2PlaceHolderProxy'))
return
}
else if (ProxySetHandlerTargetCheckAndReplace(SetArgs[0], '')) {
console.debug(`[${UserscriptName}]: Proxy set called for PowerLink Skeleton (target check and replace):`, SetArgs)
}
return OriginalReflectApply(OriginalSet, this, SetArgs)
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

This change removes the previous ProxySetHandlerTargetCheckAndReplace(...) branch (which replaced matched ader.naver.com string values with '' before calling the original setter). That’s a functional behavior change beyond the stated goal of adding cycle detection to ProxySetHandlerTargetCheck. If the replacement is still required for blocking/sanitization, consider reintroducing it with the new Visited guard (or update the PR title/description to reflect the behavior change).

Copilot uses AI. Check for mistakes.
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