From c892544741bd16df49f9373f43083b0bf3f76661 Mon Sep 17 00:00:00 2001 From: Jakob Wanger Date: Sun, 18 Feb 2024 16:33:50 -0500 Subject: [PATCH 1/2] Prevent long parse times for images with illegal char in tag Update the regular expression used to parse Docker images references to prevent catastrophic backtracking when images names are long and the tag contains an illegal character. See gh-39617 --- .../boot/docker/compose/core/Regex.java | 2 +- .../compose/core/ImageReferenceTests.java | 27 ++++++++++++++++++- .../buildpack/platform/docker/type/Regex.java | 4 +-- .../docker/type/ImageReferenceTests.java | 27 ++++++++++++++++++- 4 files changed, 55 insertions(+), 5 deletions(-) diff --git a/spring-boot-project/spring-boot-docker-compose/src/main/java/org/springframework/boot/docker/compose/core/Regex.java b/spring-boot-project/spring-boot-docker-compose/src/main/java/org/springframework/boot/docker/compose/core/Regex.java index c417ae0d1d..74e8841517 100644 --- a/spring-boot-project/spring-boot-docker-compose/src/main/java/org/springframework/boot/docker/compose/core/Regex.java +++ b/spring-boot-project/spring-boot-docker-compose/src/main/java/org/springframework/boot/docker/compose/core/Regex.java @@ -50,7 +50,7 @@ final class Regex implements CharSequence { private static final Regex PATH_COMPONENT; static { Regex segment = Regex.of("[a-z0-9]+"); - Regex separator = Regex.group("[._]|__|[-]*"); + Regex separator = Regex.group("[._-]{1,2}"); Regex separatedSegment = Regex.group(separator, segment).oneOrMoreTimes(); PATH_COMPONENT = Regex.of(segment, Regex.group(separatedSegment).zeroOrOnce()); } 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 797b82e09d..97a479387e 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 @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,9 +17,12 @@ package org.springframework.boot.docker.compose.core; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; +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}. @@ -165,4 +168,26 @@ 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/main/java/org/springframework/boot/buildpack/platform/docker/type/Regex.java b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/type/Regex.java index c843097d10..648565ee69 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/type/Regex.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/type/Regex.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -50,7 +50,7 @@ final class Regex implements CharSequence { private static final Regex PATH_COMPONENT; static { Regex segment = Regex.of("[a-z0-9]+"); - Regex separator = Regex.group("[._]|__|[-]*"); + Regex separator = Regex.group("[._-]{1,2}"); Regex separatedSegment = Regex.group(separator, segment).oneOrMoreTimes(); PATH_COMPONENT = Regex.of(segment, Regex.group(separatedSegment).zeroOrOnce()); } 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 d33abc7f38..b5d2abf290 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 @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,10 +19,13 @@ package org.springframework.boot.buildpack.platform.docker.type; import java.io.File; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; +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}. @@ -306,4 +309,26 @@ 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) { + } + } + } From c93acdafbd6de650f5afc7e2d33ff4689f7907c4 Mon Sep 17 00:00:00 2001 From: Scott Frederick Date: Tue, 20 Feb 2024 10:26:46 -0600 Subject: [PATCH 2/2] 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) { - } - } - }