Skip to content

fix(images): prevent divide by zero in LoggedPullImageResultCallback (#11416) - #11953

Open
arimu1 wants to merge 1 commit into
testcontainers:mainfrom
arimu1:fix/11416-logged-pull-zero-duration
Open

fix(images): prevent divide by zero in LoggedPullImageResultCallback (#11416)#11953
arimu1 wants to merge 1 commit into
testcontainers:mainfrom
arimu1:fix/11416-logged-pull-zero-duration

Conversation

@arimu1

@arimu1 arimu1 commented Jul 29, 2026

Copy link
Copy Markdown

Fixes #11416

Description

When pulling an image completes in less than 1 second (sub-second duration), Duration.between(start, Instant.now()).getSeconds() returns 0.
Calculating transfer speed as downloadedLayerSize / duration throws java.lang.ArithmeticException: / by zero.

This PR uses Math.max(1, duration) when calculating transfer speed in LoggedPullImageResultCallback#onComplete(), preventing ArithmeticException.

Changes

  • Updated LoggedPullImageResultCallback.java to use Math.max(1, duration) when calculating transfer rate.
  • Added unit test LoggedPullImageResultCallbackTest.java covering zero-duration pull completion.

@arimu1
arimu1 requested a review from a team as a code owner July 29, 2026 02:44
@renechoi

Copy link
Copy Markdown

I ran into the same ArithmeticException, so I checked both open fixes for #11416 locally against 2ac3c9773 on JDK 21.0.9. Here is what I measured.

The fix and the test hold up. With only the new LoggedPullImageResultCallbackTest.java applied and the main source reverted to main, the test fails with exactly the reported error:

java.lang.ArithmeticException: / by zero
    at org.testcontainers.images.LoggedPullImageResultCallback.onComplete(LoggedPullImageResultCallback.java:120)

With the full PR, it passes. So the test pins the change rather than passing either way. Worth noting the reproduction is 0 / 0 (the response item carries no progressDetail, so downloadedLayerSize() is 0), which is the same division as the real sub-second case.

Spotless will fail CI. ./gradlew :testcontainers:spotlessJavaCheck fails on the new test file: line 21 is 129 characters against the repo's printWidth: 120 (gradle/spotless.gradle). spotlessApply rewrites it to:

        PullResponseItem item = mapper.readValue(
            "{\"status\":\"Download complete\",\"id\":\"layer1\"}",
            PullResponseItem.class
        );

The start != null half of the diff is not exercised. I reverted just that hunk and kept Math.max(1, duration), and the new test still passes; Math.max alone is what fixes #11416. Is there a path where onComplete() runs without onStart()? Both call sites (RemoteDockerImage#pullImage and DockerClientFactory) go through pullImageCmd.exec(...) and I could not construct one. If there is none, dropping the ternary keeps this to a one-line change; if there is, it is a different bug from the one in the issue and would be worth a line in the description.

One thing worth flagging before this gets a decision. #11417 fixes the same issue and is approved, but it does not work. It computes Duration.between(start, Instant.now()).get(ChronoUnit.MILLIS) / 1000.0, and Duration.get(TemporalUnit) supports only SECONDS and NANOS. On JDK 21.0.9:

Duration.ofMillis(1500).getSeconds()            -> 1
Duration.ofMillis(1500).get(ChronoUnit.MILLIS)  -> java.time.temporal.UnsupportedTemporalTypeException: Unsupported unit: Millis
Duration.ofMillis(1500).toMillis()              -> 1500

I applied #11417 onto 2ac3c9773 and ran this PR's test against it. It compiles and then fails with UnsupportedTemporalTypeException: Unsupported unit: Millis. Since that accessor throws for any duration, it would throw on every completed pull, not just sub-second ones.

Optional. If the intent behind #11417 (a real rate for sub-second pulls) is wanted, toMillis() is the supported accessor and keeps integer arithmetic:

final long millis = start != null ? Duration.between(start, Instant.now()).toMillis() : 0;
...
FileUtils.byteCountToDisplaySize(downloadedLayerSize * 1000 / Math.max(1, millis))

That also removes the small inconsistency this PR leaves behind, where a sub-second pull logs pulled in 0s (downloaded 50 MB at 50 MB/s) because the duration in the message and the duration used for the rate are different numbers. downloadedLayerSize * 1000 stays well inside long for any realistic image size.

@arimu1
arimu1 force-pushed the fix/11416-logged-pull-zero-duration branch from 59d6fe4 to 67f0d95 Compare July 29, 2026 23:15
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.

[Bug]: LoggedPullImageResultCallback throws java.lang.ArithmeticException: / by zero

2 participants