From 3502b7ababf18168591899cbe11deb230826629b Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Sat, 12 Jul 2025 18:51:46 +0200 Subject: [PATCH 1/3] Fail on classpath resource names that are blank after removing leading / Fixes #4752. --- .../src/docs/asciidoc/release-notes/release-notes-5.13.4.adoc | 3 ++- .../platform/engine/discovery/ClasspathResourceSelector.java | 3 +++ .../platform/engine/discovery/DiscoverySelectorsTests.java | 3 +++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/documentation/src/docs/asciidoc/release-notes/release-notes-5.13.4.adoc b/documentation/src/docs/asciidoc/release-notes/release-notes-5.13.4.adoc index 25dd429044a1..f098766d77f1 100644 --- a/documentation/src/docs/asciidoc/release-notes/release-notes-5.13.4.adoc +++ b/documentation/src/docs/asciidoc/release-notes/release-notes-5.13.4.adoc @@ -26,7 +26,8 @@ repository on GitHub. [[release-notes-5.13.4-junit-platform-bug-fixes]] ==== Bug Fixes -* ❓ +* `ClasspathResourceSelector` no longer allows to be constructed with a resource name that + is blank after removing the leading slash. [[release-notes-5.13.4-junit-platform-deprecations-and-breaking-changes]] ==== Deprecations and Breaking Changes diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/discovery/ClasspathResourceSelector.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/discovery/ClasspathResourceSelector.java index 5cd34a04a990..87c5e6b6bb9b 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/discovery/ClasspathResourceSelector.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/discovery/ClasspathResourceSelector.java @@ -25,6 +25,7 @@ import org.junit.platform.commons.PreconditionViolationException; import org.junit.platform.commons.function.Try; import org.junit.platform.commons.support.Resource; +import org.junit.platform.commons.util.Preconditions; import org.junit.platform.commons.util.ReflectionUtils; import org.junit.platform.commons.util.StringUtils; import org.junit.platform.commons.util.ToStringBuilder; @@ -64,6 +65,8 @@ public final class ClasspathResourceSelector implements DiscoverySelector { ClasspathResourceSelector(String classpathResourceName, @Nullable FilePosition position) { boolean startsWithSlash = classpathResourceName.startsWith("/"); this.classpathResourceName = (startsWithSlash ? classpathResourceName.substring(1) : classpathResourceName); + Preconditions.notBlank(this.classpathResourceName, + "classpath resource name must not be blank after removing leading slash"); this.position = position; } diff --git a/platform-tests/src/test/java/org/junit/platform/engine/discovery/DiscoverySelectorsTests.java b/platform-tests/src/test/java/org/junit/platform/engine/discovery/DiscoverySelectorsTests.java index 85ca8917a208..1d8c81a4fbd0 100644 --- a/platform-tests/src/test/java/org/junit/platform/engine/discovery/DiscoverySelectorsTests.java +++ b/platform-tests/src/test/java/org/junit/platform/engine/discovery/DiscoverySelectorsTests.java @@ -299,8 +299,11 @@ void parseDirectorySelectorWithAbsolutePath() { void selectClasspathResourcesPreconditions() { assertViolatesPrecondition(() -> selectClasspathResource((String) null)); assertViolatesPrecondition(() -> selectClasspathResource("")); + assertViolatesPrecondition(() -> selectClasspathResource("/")); assertViolatesPrecondition(() -> selectClasspathResource(" ")); + assertViolatesPrecondition(() -> selectClasspathResource("/ ")); assertViolatesPrecondition(() -> selectClasspathResource("\t")); + assertViolatesPrecondition(() -> selectClasspathResource("/\t")); assertViolatesPrecondition(() -> selectClasspathResource((Set) null)); assertViolatesPrecondition(() -> selectClasspathResource(Collections.emptySet())); assertViolatesPrecondition(() -> selectClasspathResource(Collections.singleton(null))); From f59f4db2f71dbbd4b12ffaa449542a6758f1bfc4 Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Sun, 13 Jul 2025 13:39:55 +0200 Subject: [PATCH 2/3] Allow default package for `PackageSource` --- .../asciidoc/release-notes/release-notes-5.13.4.adoc | 2 ++ .../engine/support/descriptor/PackageSource.java | 5 ++++- .../engine/support/descriptor/PackageSourceTests.java | 10 +++++++--- .../launcher/core/DiscoveryIssueCollectorTests.java | 1 + 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/documentation/src/docs/asciidoc/release-notes/release-notes-5.13.4.adoc b/documentation/src/docs/asciidoc/release-notes/release-notes-5.13.4.adoc index f098766d77f1..54b791e7d66f 100644 --- a/documentation/src/docs/asciidoc/release-notes/release-notes-5.13.4.adoc +++ b/documentation/src/docs/asciidoc/release-notes/release-notes-5.13.4.adoc @@ -28,6 +28,8 @@ repository on GitHub. * `ClasspathResourceSelector` no longer allows to be constructed with a resource name that is blank after removing the leading slash. +* `PackageSource.from(String)` now allows to be constructed with an empty string to + indicate the default package. [[release-notes-5.13.4-junit-platform-deprecations-and-breaking-changes]] ==== Deprecations and Breaking Changes diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/descriptor/PackageSource.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/descriptor/PackageSource.java index 94befac98143..c067dcb797df 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/descriptor/PackageSource.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/descriptor/PackageSource.java @@ -60,7 +60,10 @@ private PackageSource(Package javaPackage) { } private PackageSource(String packageName) { - this.packageName = Preconditions.notBlank(packageName, "package name must not be null or blank"); + Preconditions.notNull(packageName, "package name must not be null"); + Preconditions.condition(packageName.isEmpty() || !packageName.isBlank(), + "package name must not contain only whitespace"); + this.packageName = packageName; } /** diff --git a/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/PackageSourceTests.java b/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/PackageSourceTests.java index f964343c4c2d..7b32ab9cd25a 100644 --- a/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/PackageSourceTests.java +++ b/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/PackageSourceTests.java @@ -18,6 +18,8 @@ import java.util.stream.Stream; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.junit.platform.commons.PreconditionViolationException; /** @@ -49,9 +51,11 @@ void packageSourceFromNullPackageReference() { assertThrows(PreconditionViolationException.class, () -> PackageSource.from((Package) null)); } - @Test - void packageSourceFromPackageName() { - var testPackage = getClass().getPackage().getName(); + @ParameterizedTest + @ValueSource(classes = PackageSourceTests.class) + @ValueSource(strings = "DefaultPackageTestCase") + void packageSourceFromPackageName(Class testClass) { + var testPackage = testClass.getPackage().getName(); var source = PackageSource.from(testPackage); assertThat(source.getPackageName()).isEqualTo(testPackage); diff --git a/platform-tests/src/test/java/org/junit/platform/launcher/core/DiscoveryIssueCollectorTests.java b/platform-tests/src/test/java/org/junit/platform/launcher/core/DiscoveryIssueCollectorTests.java index 40994c481f6e..f230fd1dc037 100644 --- a/platform-tests/src/test/java/org/junit/platform/launcher/core/DiscoveryIssueCollectorTests.java +++ b/platform-tests/src/test/java/org/junit/platform/launcher/core/DiscoveryIssueCollectorTests.java @@ -69,6 +69,7 @@ public static Stream pairs() { new Pair(selectClasspathResource("someResource", FilePosition.from(42, 23)), ClasspathResourceSource.from("someResource", org.junit.platform.engine.support.descriptor.FilePosition.from(42, 23))), // + new Pair(selectPackage(""), PackageSource.from("")), // new Pair(selectPackage("some.package"), PackageSource.from("some.package")), // new Pair(selectFile("someFile"), FileSource.from(new File("someFile"))), // new Pair(selectFile("someFile", FilePosition.from(42)), From 40812a2b35e7e3d6cbb1c71ec688ed3dd580da37 Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Sat, 12 Jul 2025 18:56:53 +0200 Subject: [PATCH 3/3] Protect against potential problems when converting file-based selectors --- .../core/DiscoveryIssueCollector.java | 32 ++++++++++++------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/junit-platform-launcher/src/main/java/org/junit/platform/launcher/core/DiscoveryIssueCollector.java b/junit-platform-launcher/src/main/java/org/junit/platform/launcher/core/DiscoveryIssueCollector.java index 9ec2d1f09abb..e6d8a9ce7c43 100644 --- a/junit-platform-launcher/src/main/java/org/junit/platform/launcher/core/DiscoveryIssueCollector.java +++ b/junit-platform-launcher/src/main/java/org/junit/platform/launcher/core/DiscoveryIssueCollector.java @@ -10,6 +10,7 @@ package org.junit.platform.launcher.core; +import static org.junit.platform.commons.util.UnrecoverableExceptions.rethrowIfUnrecoverable; import static org.junit.platform.engine.SelectorResolutionResult.Status.FAILED; import static org.junit.platform.engine.SelectorResolutionResult.Status.UNRESOLVED; @@ -78,7 +79,7 @@ else if (result.getStatus() == UNRESOLVED && selector instanceof UniqueIdSelecto } } - static @Nullable TestSource toSource(DiscoverySelector selector) { + private static @Nullable TestSource toSource(DiscoverySelector selector) { if (selector instanceof ClassSelector classSelector) { return ClassSource.from(classSelector.getClassName()); } @@ -96,17 +97,26 @@ else if (result.getStatus() == UNRESOLVED && selector instanceof UniqueIdSelecto if (selector instanceof PackageSelector packageSelector) { return PackageSource.from(packageSelector.getPackageName()); } - if (selector instanceof FileSelector fileSelector) { - return fileSelector.getPosition() // - .map(DiscoveryIssueCollector::convert) // - .map(position -> FileSource.from(fileSelector.getFile(), position)) // - .orElseGet(() -> FileSource.from(fileSelector.getFile())); - } - if (selector instanceof DirectorySelector directorySelector) { - return DirectorySource.from(directorySelector.getDirectory()); + try { + // Both FileSource and DirectorySource call File.getCanonicalFile() to normalize the reported file which + // can throw an exception for certain file names on certain file systems. UriSource.from(...) is affected + // as well because it may return a FileSource or DirectorySource + if (selector instanceof FileSelector fileSelector) { + return fileSelector.getPosition() // + .map(DiscoveryIssueCollector::convert) // + .map(position -> FileSource.from(fileSelector.getFile(), position)) // + .orElseGet(() -> FileSource.from(fileSelector.getFile())); + } + if (selector instanceof DirectorySelector directorySelector) { + return DirectorySource.from(directorySelector.getDirectory()); + } + if (selector instanceof UriSelector uriSelector) { + return UriSource.from(uriSelector.getUri()); + } } - if (selector instanceof UriSelector uriSelector) { - return UriSource.from(uriSelector.getUri()); + catch (Exception ex) { + rethrowIfUnrecoverable(ex); + logger.warn(ex, () -> "Failed to convert DiscoverySelector [%s] into TestSource".formatted(selector)); } return null; }