GROOVY-12109: Harden Closure deserialization against owner/delegate r…

[paulk-asert]

…eference cycle

[Copilot]

Pull request overview

This PR hardens Groovy closure deserialization by adding a Closure.readResolve() validation step that detects owner/delegate/thisObject cycles (which can be forged via a crafted serialized stream) and rejects them, while adding tests to ensure legitimate closure serialization continues to work.

Changes:

• Add a readResolve-based cycle check in groovy.lang.Closure to reject cyclic closure graphs during deserialization.
• Update MethodClosure.readResolve visibility to align with the new Closure#readResolve.
• Add JUnit tests covering self-referential and mutual-cycle closures, plus “legitimate round-trip” cases.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/main/java/groovy/lang/Closure.java	Adds readResolve() cycle detection over owner/delegate/thisObject closure links.
src/main/java/org/codehaus/groovy/runtime/MethodClosure.java	Adjusts readResolve accessibility in response to the new superclass hook.
src/test/groovy/groovy/lang/ClosureSerializationCycleTest.groovy	Adds regression tests for cyclic closure graphs and validates normal serialization round-trips.

Comments suppressed due to low confidence (1)

src/main/java/org/codehaus/groovy/runtime/MethodClosure.java:183

• MethodClosure overrides Closure#readResolve but never invokes the superclass implementation, so the new cycle-validation in Closure.readResolve is bypassed for MethodClosure instances whenever deserialization is allowed. Call super.readResolve() (and propagate ObjectStreamException) before applying the ALLOW_RESOLVE gate so MethodClosure gets the same validation as other closures.

    protected Object readResolve() { // overrides Closure#readResolve (must be at least as accessible)
        if (ALLOW_RESOLVE) {
            return this;
        }
        throw new UnsupportedOperationException();

[codecov-commenter]

Codecov Report

❌ Patch coverage is 88.23529% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.5202%. Comparing base (568bf98) to head (d7d7902).

Files with missing lines	Patch %	Lines
src/main/java/groovy/lang/Closure.java	89.6552%	1 Missing and 2 partials ⚠️
...ava/org/codehaus/groovy/runtime/MethodClosure.java	0.0000%	1 Missing ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##               master      #2634        +/-   ##
==================================================
+ Coverage     68.5180%   68.5202%   +0.0021%     
- Complexity      33597      33605         +8     
==================================================
  Files            1519       1519                
  Lines          127384     127418        +34     
  Branches        23123      23130         +7     
==================================================
+ Hits            87281      87307        +26     
- Misses          32355      32358         +3     
- Partials         7748       7753         +5     

Files with missing lines	Coverage Δ
...a/org/codehaus/groovy/runtime/ComposedClosure.java	76.1905% <100.0000%> (+2.5063%)	⬆️
...va/org/codehaus/groovy/runtime/CurriedClosure.java	91.2088% <100.0000%> (+0.1975%)	⬆️
...ava/org/codehaus/groovy/runtime/MethodClosure.java	79.2453% <0.0000%> (-1.5239%)	⬇️
src/main/java/groovy/lang/Closure.java	79.2857% <89.6552%> (+1.1981%)	⬆️

... and 8 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[testlens-app]

✅ All tests passed ✅

🏷️ Commit: d7d7902
▶️ Tests: 66740 executed
⚪️ Checks: 31/31 completed

---

Learn more about TestLens at testlens.app.

[daniellansun]

Merged. Thanks.