[
https://issues.apache.org/jira/browse/GROOVY-12339?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18111363#comment-18111363
]
ASF GitHub Bot commented on GROOVY-12339:
-----------------------------------------
Copilot commented on code in PR #2864:
URL: https://github.com/apache/groovy/pull/2864#discussion_r3930118212
##########
src/main/java/groovy/lang/Closure.java:
##########
@@ -1361,6 +1362,19 @@ public void setResolveStrategy(int resolveStrategy) {
public int getResolveStrategy() {
return Closure.this.getResolveStrategy();
}
+
+ /**
+ * Rejects a deserialized closure whose owner/delegate/thisObject
references form a cycle, which
+ * would otherwise recurse indefinitely on invocation. See {@link
Closure#checkForReferenceCycle}.
+ * <p>
+ * The enclosing closure this one writes through is its {@code owner},
so the standard walk
+ * already reaches it and no {@code additionalReferences()} override
is needed.
+ */
+ @Serial
+ private Object readResolve() throws ObjectStreamException {
+ Closure.checkForReferenceCycle(this);
+ return this;
+ }
Review Comment:
`WritableClosure` also forwards methods such as `getDelegate`,
`setResolveStrategy`, and `run` through the serialized synthetic outer
reference (`Closure.this`), not through `owner`. A forged stream can therefore
make `this$0` self-referential while leaving `owner` acyclic; this
`readResolve` accepts it, but calling one of those methods still recurses
indefinitely. Include `Closure.this` in `additionalReferences()` (duplicate
normal links are harmless) and cover a forged `this$0` cycle in the regression
test.
> Additional Closure serialization cycle checks
> ---------------------------------------------
>
> Key: GROOVY-12339
> URL: https://issues.apache.org/jira/browse/GROOVY-12339
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Priority: Major
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)