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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
