FIX - Catastrophic Backtracking for images with illegal char in tag

- Simplify regular expression checking for validity of Image Reference to fix Catastrophic Backtracking for long custom images with illegal characters in tag #39246
spring-projects · Feb 19, 2024 · 195fac7 · 195fac7
1 parent 4fd0e29
commit 195fac7
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 5 deletions.
diff --git a/...boot-docker-compose/src/main/java/org/springframework/boot/docker/compose/core/Regex.java b/...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/...mpose/src/test/java/org/springframework/boot/docker/compose/core/ImageReferenceTests.java b/...mpose/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.
@@ -16,10 +16,14 @@
 
 package org.springframework.boot.docker.compose.core;
 
+import java.time.Duration;
+
 import org.junit.jupiter.api.Test;
+import org.testcontainers.shaded.org.awaitility.Awaitility;
 
 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 +169,30 @@ void equalsAndHashCode() {
 		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
+	void ofWhenImageNameIsVeryLongAndHasIllegalCharacter() {
+		Awaitility.await()
+				.timeout(Duration.ofSeconds(2))
+				.until(() -> {
+					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) {
+					}
+					return true;
+				});
+	}
+
 }
diff --git a/...platform/src/main/java/org/springframework/boot/buildpack/platform/docker/type/Regex.java b/...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/...est/java/org/springframework/boot/buildpack/platform/docker/type/ImageReferenceTests.java b/...est/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.
@@ -17,12 +17,15 @@
 package org.springframework.boot.buildpack.platform.docker.type;
 
 import java.io.File;
+import java.time.Duration;
 
 import org.junit.jupiter.api.Test;
+import org.testcontainers.shaded.org.awaitility.Awaitility;
 
 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,29 @@ void inTaglessForm() {
 		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
+	void ofWhenIsVeryLongAndHasIllegalCharacter() {
+		Awaitility.await()
+				.timeout(Duration.ofSeconds(2))
+				.until(() -> {
+					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) {
+					}
+					return true;
+				});
+	}
+
 }