Skip to content

Fix false positive in DockerImageName.isCompatibleWith for library/-prefixed images - #11922

Open
TimurRakhmatullin86 wants to merge 1 commit into
testcontainers:mainfrom
TimurRakhmatullin86:fix/dockerimagename-library-compatibility
Open

Fix false positive in DockerImageName.isCompatibleWith for library/-prefixed images#11922
TimurRakhmatullin86 wants to merge 1 commit into
testcontainers:mainfrom
TimurRakhmatullin86:fix/dockerimagename-library-compatibility

Conversation

@TimurRakhmatullin86

Copy link
Copy Markdown

Broken behaviour

DockerImageName.isCompatibleWith() returns true for any image whose repository starts with library/, regardless of what it is compared against:

DockerImageName.parse("library/mysql:8")
    .isCompatibleWith(DockerImageName.parse("postgres")); // true (!)

The library-prefix handling introduced in #6174 derives imageWithLibraryPrefix from this only and then compares it back against this:

DockerImageName imageWithLibraryPrefix = DockerImageName.parse(finalImageName);
if (other.equals(this) || imageWithLibraryPrefix.equals(this)) {

other never takes part in that comparison, so for any registry-less library/… name the check degenerates to parse(this.repository).equals(this), which is always true (parse drops the tag and AnyVersion equals everything). Since assertCompatibleWith(...) guards the constructors of essentially every module (~96 call sites), it silently accepts wrong images instead of failing fast with the asCompatibleSubstituteFor guidance. The existing test testAssertMethodAcceptsCompatibleLibraryPrefix kept passing for this same wrong reason.

Fix

Normalize both sides to the library/-prefixed form before comparing, and only for names without a registry (Docker Hub official images):

  • library/foo:1.2.3 ~ barfalse (was true)
  • library/foo ~ footrue (as before, now for the right reason)
  • foo ~ library/footrue (previously false; same image, now symmetric)
  • Version semantics are unchanged: other stays on the left-hand side of equals, so a tag present in other must still match exactly and an untagged other still acts as a wildcard (library/foo:1.2.3 ~ foo:4.5.6false).
  • Names with an explicit registry are left untouched (some.registry/foolibrary/foo).

Images named library/… that previously passed the check only because of the false positive now fail with the standard compatibility error — that is the intended behaviour of the guard.

Related to #9958: the library/postgres case reported there is what #6174 targeted; the docker.io/… registry-qualified cases from that issue are out of scope for this PR.

Testing

Added regression tests to DockerImageNameCompatibilityTest covering the false positive, prefix symmetry in both directions (with and without tags), registry exclusion, and asCompatibleSubstituteFor interaction. The new tests fail on main and pass with this change; all pre-existing tests pass, checkstyle and spotless are clean.

@TimurRakhmatullin86
TimurRakhmatullin86 requested a review from a team as a code owner July 10, 2026 05:59
@TimurRakhmatullin86
TimurRakhmatullin86 force-pushed the fix/dockerimagename-library-compatibility branch from aad5fed to 4d2c5e6 Compare July 19, 2026 16:56

@kdelay kdelay left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I went through this locally against upstream/main (2ac3c97) and can confirm the diagnosis. A couple of measured notes below.

What reproduces. The degenerate branch behaves exactly as you describe — parse(this.repository).equals(this) puts AnyVersion on the left-hand side of equals, so other never participates:

library/mysql:8                                      ~ postgres    -> true
library/foo:1.2.3                                    ~ foo:4.5.6   -> true
library/foo@sha256:1234567890abcdef1234567890abcdef  ~ bar         -> true
foo:1.2.3                                            ~ bar         -> false   (control)

So it swallows sha256-pinned names too, not only tags. With the patch all three become false. The three new tests fail on main and pass with the change; DockerImageNameCompatibilityTest, DockerImageNameTest, spotlessJavaCheck and checkstyleMain are all green on the branch.

The claim about testAssertMethodAcceptsCompatibleLibraryPrefix also checks out: if I delete only the new normalization block from your branch and leave everything else in place, that pre-existing test fails. So it does guard the behaviour after this change, whereas on main it was satisfied by the bug.

One thing I would tighten. withLibraryPrefix() prefixes any repository that has no registry, including multi-segment Docker Hub user namespaces. library/ is the single-segment official-image namespace, so library/myuser/myimage is a different repository from myuser/myimage. Measured:

main this PR
foo/bar ~ library/foo/bar false true
myuser/myimage:1 ~ library/myuser/myimage false true
library/foo/bar ~ foo/bar true (via the bug) true

The first two are a new false positive of the same family as the one being removed, and the third is not fixed, just re-derived from the new code path. Restricting the normalization to single-segment names makes all three false:

private static DockerImageName withLibraryPrefix(DockerImageName image) {
    if (!image.registry.isEmpty() || image.repository.startsWith(LIBRARY_PREFIX)) {
        return image;
    }
    if (image.repository.indexOf('/') >= 0) {
        // Not a Docker Hub official image; the library/ namespace does not apply.
        return image;
    }
    return image.withRepository(LIBRARY_PREFIX + image.repository);
}

With that guard your three new tests plus DockerImageNameTest still pass and spotless/checkstyle stay clean.

Two smaller notes.

  • testLibraryPrefixedImageIsNotCompatibleWithDifferentImage and testLibraryPrefixedImageWithClaimedCompatibility assert negatives only, so they also pass if the library handling is removed outright; only testLibraryPrefixIsInterchangeable pins the normalization itself. Together they do cover the intended behaviour — it is just worth knowing which test holds which half.
  • The scope note looks accurate: docker.io/library/foo ~ foo, docker.io/library/foo ~ docker.io/foo and localhost/foo ~ library/foo are all false both before and after, so the registry-qualified half of #9958 is untouched. Since that issue asks for canonicalization to docker.io/library/<name>, the single-segment restriction above is the same rule a full canonicalization would need anyway.

I did not run the full core suite (it needs Docker); the above is the three org.testcontainers.utility test classes plus the checkstyle/spotless tasks. For blast radius, nothing under core/ or modules/ uses a library/-prefixed image name, so tightening the guard has no in-repo fallout — the behaviour change only lands on user-supplied names.

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.

2 participants