Repository navigation
Keep full tag when parsing image names with extra colons - #12107
SamuelAdigos wants to merge 1 commit into
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes.
 |
| Layer / File(s) | Summary |
|---|---|
Tag parsing and validation core/src/main/java/org/testcontainers/utility/DockerImageName.java, core/src/test/java/org/testcontainers/utility/DockerImageNameTest.java |
The parser uses the first colon to separate the repository from the tag and retains the remaining text as the tag. It rejects empty tags with an IllegalArgumentException. Tests cover multiple-colon names, canonical names, and the exception message. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Low
Suggested reviewers: eddumelendez
Merge Risk: 
a2589
-
The change preserves malformed image-name input for validation and improves the empty-tag error; no material merge risk was identified.
Pre-merge checks | 
-
❌ Failed checks (1 warning)
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold. ✅ Passed checks (4 passed)
-
Check name Status Explanation Title check The title clearly and concisely describes the main change: preserving the full tag when parsing image names with extra colons. Description check The description explains the bug, the parsing change, the resulting validation behavior, the trailing-colon case, the related issue, and the tests and validation performed. Linked Issues check Issue [ #5190] requires the parser to preserve the supplied image name instead of dropping text after a second colon, and to reject the malformed reference.DockerImageName(String)uses `split(":", 2…Out of Scope Changes check The changes are limited to DockerImageName.javaand its focused test class. The production change implements issue [#5190]. The added tests directly verify the changed parsing and validation behavio…
-
✨ Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
`DockerImageName.parse("neo4j:neo4j:4.4")` silently dropped everything
after the second colon. The tag is now everything after the first colon
of the remote name, consistent with the tag-and-digest branch, so the
malformed name is preserved and rejected by `assertValid()`.
A trailing colon (`myname:`) now fails with a descriptive
`IllegalArgumentException` instead of an `ArrayIndexOutOfBoundsException`.
Fixes testcontainers#5190
a258935 to
397d3c0
Compare
Fixes #5190
DockerImageName.parse("neo4j:neo4j:4.4")silently dropped everything after the second:, yieldingneo4j:neo4j.The malformed name passed
assertValid(), and any later error referred to an image name the user never wrote.The tag is now everything after the first
:of the remote name (split(":", 2)), matching how the tag-and-digest branchalready parses it (see
testMalformedTagWithExtraColonIsRejected). The full name is preserved andassertValid()rejectsthe malformed tag, since
:is not allowed in Docker tags.A trailing colon (
myname:) previously failed with anArrayIndexOutOfBoundsException; it now fails with a descriptiveIllegalArgumentExceptioninstead of producing an empty tag (which would otherwise reach the pull withtag="").Note: a malformed name such as
postgres:16:alpinepreviously resolved silently topostgres:16; it will now fail whenDocker is asked for the invalid tag.
I kept the change minimal rather than aligning the whole parser with the Docker reference grammar, as discussed in the
issue, to avoid rejecting names that are accepted today.
Tests
DockerImageNameTest.InvalidNamestestExtraColonInTagIsNotDroppedandtestEmptyTagIsRejected./gradlew :testcontainers:spotlessApply :testcontainers:checkstyleMain :testcontainers:checkstyleTest :testcontainers:japicmptestMatrixlocally (core, all modules and examples). The only failures were environment-related(arm64 host, Rancher Desktop) and reproduce identically on
mainwithout this change.Summary by CodeRabbit