Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions actuator/src/main/java/org/tron/core/vm/VMUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,19 @@ private static void write(InputStream in, OutputStream out, int bufSize) throws

public static byte[] compress(byte[] bytes) throws IOException {
ByteArrayOutputStream baos = new ByteArrayOutputStream();
Deflater deflater = new Deflater();

ByteArrayInputStream in = new ByteArrayInputStream(bytes);
DeflaterOutputStream out = new DeflaterOutputStream(baos, new Deflater(), BUF_SIZE);
try {
ByteArrayInputStream in = new ByteArrayInputStream(bytes);
DeflaterOutputStream out = new DeflaterOutputStream(baos, deflater, BUF_SIZE);

write(in, out, BUF_SIZE);
write(in, out, BUF_SIZE);

return baos.toByteArray();
return baos.toByteArray();
} finally {
// DeflaterOutputStream only ends Deflaters it creates itself.
deflater.end();
}
}

public static byte[] compress(String content) throws IOException {
Expand Down
34 changes: 34 additions & 0 deletions actuator/src/test/java/org/tron/core/vm/VMUtilsTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package org.tron.core.vm;

import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.mockito.Mockito.mockConstruction;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import java.nio.charset.StandardCharsets;
import java.util.zip.Deflater;
import org.junit.Test;
import org.mockito.MockedConstruction;
import org.tron.common.utils.ByteUtil;

public class VMUtilsTest {

@Test
public void compressShouldReleaseDeflater() throws Exception {
try (MockedConstruction<Deflater> deflaters = mockConstruction(Deflater.class,
(deflater, context) -> when(deflater.finished()).thenReturn(true))) {
Comment on lines +19 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: The construction mock forces finished() to return true, so DeflaterOutputStream.close() skips its deflation loop entirely and this test exercises only the explicit end() call. It can therefore pass even if the compression stream's finalization path is broken. Use a real deflater for the lifecycle assertion, or configure the mock to emulate the finish/deflate sequence while still verifying end(). [possible bug]

Severity Level: Major ⚠️
- ⚠️ Lifecycle test can miss compression finalization regressions.
- ⚠️ VM trace compression uses this helper at VMActuator.java:306-307.
- ❌ Future compressed traces could be corrupted despite passing tests.
Steps of Reproduction ✅
1. Run `VMUtilsTest.compressShouldReleaseDeflater()` at
`actuator/src/test/java/org/tron/core/vm/VMUtilsTest.java:17-25`; the test invokes the
public `VMUtils.compress(byte[])` helper directly.

2. `VMUtils.compress()` constructs a `DeflaterOutputStream` at
`actuator/src/main/java/org/tron/core/vm/VMUtils.java:112-127`, and `write()` closes that
stream through its `finally` block at `VMUtils.java:101-104`.

3. The construction mock at `VMUtilsTest.java:19-20` forces `Deflater.finished()` to
return `true`. Consequently, `DeflaterOutputStream.close()` can skip its normal deflation
loop after calling `finish()`, so the test does not exercise actual output finalization or
deflate progress.

4. A regression in the stream's finish/deflate interaction could therefore still satisfy
`verify(...).end()` at `VMUtilsTest.java:23-24`. The separate content test at
`VMUtilsTest.java:28-33` checks successful compression with a real `Deflater`, but it does
not verify that the constructed deflater in the lifecycle test is finalized and released
along the same execution path. Use a real deflater for the lifecycle assertion, or
configure the mock to emulate the finish/deflate sequence while retaining verification of
`end()`.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** actuator/src/test/java/org/tron/core/vm/VMUtilsTest.java
**Line:** 19:20
**Comment:**
	*Possible Bug: The construction mock forces `finished()` to return `true`, so `DeflaterOutputStream.close()` skips its deflation loop entirely and this test exercises only the explicit `end()` call. It can therefore pass even if the compression stream's finalization path is broken. Use a real deflater for the lifecycle assertion, or configure the mock to emulate the finish/deflate sequence while still verifying `end()`.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

VMUtils.compress(new byte[0]);

assertEquals(1, deflaters.constructed().size());
verify(deflaters.constructed().get(0)).end();
}
}

@Test
public void compressShouldPreserveContent() throws Exception {
byte[] content = "VM trace compression".getBytes(StandardCharsets.UTF_8);

assertArrayEquals(content, ByteUtil.decompress(VMUtils.compress(content)));
}
}
Loading