From c93acdafbd6de650f5afc7e2d33ff4689f7907c4 Mon Sep 17 00:00:00 2001 From: Scott Frederick Date: Tue, 20 Feb 2024 10:26:46 -0600 Subject: [PATCH] Polish "Prevent long parse times for images with illegal char in tag" See gh-39617 --- .../compose/core/ImageReferenceTests.java | 43 ++++++++----------- .../docker/type/ImageReferenceTests.java | 43 ++++++++----------- 2 files changed, 38 insertions(+), 48 deletions(-) diff --git a/spring-boot-project/spring-boot-docker-compose/src/test/java/org/springframework/boot/docker/compose/core/ImageReferenceTests.java b/spring-boot-project/spring-boot-docker-compose/src/test/java/org/springframework/boot/docker/compose/core/ImageReferenceTests.java index 97a479387e..e9eb904581 100644 --- a/spring-boot-project/spring-boot-docker-compose/src/test/java/org/springframework/boot/docker/compose/core/ImageReferenceTests.java +++ b/spring-boot-project/spring-boot-docker-compose/src/test/java/org/springframework/boot/docker/compose/core/ImageReferenceTests.java @@ -22,7 +22,6 @@ import org.junit.jupiter.api.Timeout.ThreadMode; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; -import static org.assertj.core.api.Assertions.fail; /** * Tests for {@link ImageReference}. @@ -42,6 +41,16 @@ class ImageReferenceTests { assertThat(reference).hasToString("docker.io/library/ubuntu"); } + @Test + void ofSimpleNameWithSingleCharacterSuffix() { + ImageReference reference = ImageReference.of("ubuntu-a"); + assertThat(reference.getDomain()).isEqualTo("docker.io"); + assertThat(reference.getName()).isEqualTo("library/ubuntu-a"); + assertThat(reference.getTag()).isNull(); + assertThat(reference.getDigest()).isNull(); + assertThat(reference).hasToString("docker.io/library/ubuntu-a"); + } + @Test void ofLibrarySlashName() { ImageReference reference = ImageReference.of("library/ubuntu"); @@ -152,13 +161,21 @@ class ImageReferenceTests { } @Test - void ofWhenHasIllegalCharacter() { + void ofWhenHasIllegalCharacterThrowsException() { assertThatIllegalArgumentException() .isThrownBy(() -> ImageReference .of("registry.example.com/example/example-app:1.6.0-dev.2.uncommitted+wip.foo.c75795d")) .withMessageContaining("Unable to parse image reference"); } + @Test + @Timeout(value = 1, threadMode = ThreadMode.SEPARATE_THREAD) + void ofWhenImageNameIsVeryLongAndHasIllegalCharacterThrowsException() { + assertThatIllegalArgumentException().isThrownBy(() -> ImageReference + .of("docker.io/library/this-image-has-a-long-name-with-an-invalid-tag-which-is-at-danger-of-catastrophic-backtracking:1.0.0+1234")) + .withMessageContaining("Unable to parse image reference"); + } + @Test void equalsAndHashCode() { ImageReference r1 = ImageReference.of("ubuntu:bionic"); @@ -168,26 +185,4 @@ class ImageReferenceTests { assertThat(r1).isEqualTo(r1).isEqualTo(r2).isNotEqualTo(r3); } - @Test - void ofSimpleNameWithSingleCharacterSuffix() { - ImageReference reference = ImageReference.of("ubuntu-a"); - assertThat(reference.getDomain()).isEqualTo("docker.io"); - assertThat(reference.getName()).isEqualTo("library/ubuntu-a"); - assertThat(reference.getTag()).isNull(); - assertThat(reference.getDigest()).isNull(); - assertThat(reference).hasToString("docker.io/library/ubuntu-a"); - } - - @Test - @Timeout(value = 1, threadMode = ThreadMode.SEPARATE_THREAD) - void ofWhenImageNameIsVeryLongAndHasIllegalCharacter() { - try { - ImageReference - .of("docker.io/library/this-image-has-a-long-name-with-an-invalid-tag-which-is-at-danger-of-catastrophic-backtracking:1.0.0+1234"); - fail("Image Reference contains an illegal character and should have thrown an IllegalArgumentException"); - } - catch (IllegalArgumentException ignored) { - } - } - } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/type/ImageReferenceTests.java b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/type/ImageReferenceTests.java index b5d2abf290..9ec351e503 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/type/ImageReferenceTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/type/ImageReferenceTests.java @@ -25,7 +25,6 @@ import org.junit.jupiter.api.Timeout.ThreadMode; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; -import static org.assertj.core.api.Assertions.fail; /** * Tests for {@link ImageReference}. @@ -46,6 +45,16 @@ class ImageReferenceTests { assertThat(reference).hasToString("docker.io/library/ubuntu"); } + @Test + void ofSimpleNameWithSingleCharacterSuffix() { + ImageReference reference = ImageReference.of("ubuntu-a"); + assertThat(reference.getDomain()).isEqualTo("docker.io"); + assertThat(reference.getName()).isEqualTo("library/ubuntu-a"); + assertThat(reference.getTag()).isNull(); + assertThat(reference.getDigest()).isNull(); + assertThat(reference).hasToString("docker.io/library/ubuntu-a"); + } + @Test void ofLibrarySlashName() { ImageReference reference = ImageReference.of("library/ubuntu"); @@ -176,7 +185,7 @@ class ImageReferenceTests { } @Test - void ofWhenHasIllegalCharacter() { + void ofWhenHasIllegalCharacterThrowsException() { assertThatIllegalArgumentException() .isThrownBy(() -> ImageReference .of("registry.example.com/example/example-app:1.6.0-dev.2.uncommitted+wip.foo.c75795d")) @@ -191,6 +200,14 @@ class ImageReferenceTests { .withMessageContaining("Unable to parse image reference"); } + @Test + @Timeout(value = 1, threadMode = ThreadMode.SEPARATE_THREAD) + void ofWhenIsVeryLongAndHasIllegalCharacter() { + assertThatIllegalArgumentException().isThrownBy(() -> ImageReference + .of("docker.io/library/this-image-has-a-long-name-with-an-invalid-tag-which-is-at-danger-of-catastrophic-backtracking:1.0.0+1234")) + .withMessageContaining("Unable to parse image reference"); + } + @Test void forJarFile() { assertForJarFile("spring-boot.2.0.0.BUILD-SNAPSHOT.jar", "library/spring-boot", "2.0.0.BUILD-SNAPSHOT"); @@ -309,26 +326,4 @@ class ImageReferenceTests { assertThat(updated).hasToString("docker.io/library/ubuntu"); } - @Test - void ofSimpleNameWithSingleCharacterSuffix() { - ImageReference reference = ImageReference.of("ubuntu-a"); - assertThat(reference.getDomain()).isEqualTo("docker.io"); - assertThat(reference.getName()).isEqualTo("library/ubuntu-a"); - assertThat(reference.getTag()).isNull(); - assertThat(reference.getDigest()).isNull(); - assertThat(reference).hasToString("docker.io/library/ubuntu-a"); - } - - @Test - @Timeout(value = 1, threadMode = ThreadMode.SEPARATE_THREAD) - void ofWhenIsVeryLongAndHasIllegalCharacter() { - try { - ImageReference - .of("docker.io/library/this-image-has-a-long-name-with-an-invalid-tag-which-is-at-danger-of-catastrophic-backtracking:1.0.0+1234"); - fail("Contains an illegal character and should have thrown an IllegalArgumentException"); - } - catch (IllegalArgumentException ignored) { - } - } - }