Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,7 @@ protected Stream<Path> listJdkPaths() throws IOException {
}

protected boolean acceptFolder(@NonNull Path jdkFolder) {
return jdkFolder.startsWith(jdksRoot) && isValidId(jdkFolder.getFileName().toString())
&& JavaUtils.hasJavacCmd(jdkFolder);
return jdkFolder.startsWith(jdksRoot) && JavaUtils.hasJavacCmd(jdkFolder);
}

private final Pattern validId = Pattern.compile("^[a-zA-Z0-9._+-]+$");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ protected boolean acceptFolder(@NonNull Path jdkFolder) {
// We additionally allow folders that are named with a number
// (e.g. "11", "17", etc.) for backwards compatibility with older
// JBang versions
return (super.acceptFolder(jdkFolder) || JavaUtils.parseToInt(jdkFolder.getFileName().toString(), 0) > 0)
return (super.acceptFolder(jdkFolder) && isValidId(jdkFolder.getFileName().toString())
|| JavaUtils.parseToInt(jdkFolder.getFileName().toString(), 0) > 0)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
&& !FileUtils.isLink(jdkFolder);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ public LinkedJdkProvider(Path jdksRoot) {

@Override
protected boolean acceptFolder(@NonNull Path jdkFolder) {
return super.acceptFolder(jdkFolder) && FileUtils.isLink(jdkFolder);
return super.acceptFolder(jdkFolder) && isValidId(jdkFolder.getFileName().toString())
&& FileUtils.isLink(jdkFolder);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,22 @@
* standard location of the users linux distro.
*
* <p>
* For now just using `/usr/lib/devkitman` as apparently fedora, debian, ubuntu
* and centos/rhel use it.
* For now just using `/usr/lib/jvm` as apparently fedora, debian, ubuntu and
* centos/rhel use it.
*
* <p>
* If need different behavior per linux distro its intended this provider will
* adjust based on identified distro.
*/
public class LinuxJdkProvider extends BaseFoldersJdkProvider {
protected static final Path JDKS_ROOT = Paths.get("/usr/lib/devkitman");
private static final Path JDKS_ROOT = Paths.get("/usr/lib/jvm");

public LinuxJdkProvider() {
super(JDKS_ROOT);
super(jdksRoot());
}

public static Path jdksRoot() {
return JDKS_ROOT;
}

@Override
Expand All @@ -35,7 +39,7 @@ public LinuxJdkProvider() {

@Override
protected boolean acceptFolder(@NonNull Path jdkFolder) {
return super.acceptFolder(jdkFolder) && !FileUtils.isSameFolderLink(jdkFolder);
return super.acceptFolder(jdkFolder) && !FileUtils.isLink(jdkFolder);
}

public static class Discovery implements JdkDiscovery {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,14 @@
* (https://mise.jdx.dev/)
*/
public class MiseJdkProvider extends BaseFoldersJdkProvider {
private static final Path JDKS_ROOT = Paths.get(System.getProperty("user.home"))
.resolve(".local/share/mise/installs/java");
private static final Path JDKS_ROOT = Paths.get(".local", "share", "mise", "installs", "java");

public MiseJdkProvider() {
super(JDKS_ROOT);
super(jdksRoot());
}

public static Path jdksRoot() {
return Paths.get(System.getProperty("user.home")).resolve(JDKS_ROOT);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
package dev.jbang.devkitman.jdkproviders;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.stream.Stream;

import org.jspecify.annotations.NonNull;
import org.jspecify.annotations.Nullable;

import dev.jbang.devkitman.Jdk;
import dev.jbang.devkitman.JdkDiscovery;
import dev.jbang.devkitman.JdkProvider;
import dev.jbang.devkitman.util.OsUtils;
Expand All @@ -18,10 +17,14 @@
* package manager. Windows only.
*/
public class ScoopJdkProvider extends BaseFoldersJdkProvider {
private static final Path SCOOP_APPS = Paths.get(System.getProperty("user.home")).resolve("scoop/apps");
private static final Path JDKS_ROOT = Paths.get("scoop", "apps");

public ScoopJdkProvider() {
super(SCOOP_APPS);
super(jdksRoot());
}

public static Path jdksRoot() {
return Paths.get(System.getProperty("user.home")).resolve(JDKS_ROOT);
}

@Override
Expand All @@ -32,23 +35,18 @@ public ScoopJdkProvider() {
@NonNull
@Override
protected Stream<Path> listJdkPaths() throws IOException {
return super.listJdkPaths().map(p -> p.resolve("current"));
if (Files.isDirectory(jdksRoot)) {
return Files.list(jdksRoot)
.filter(p -> p.getFileName().toString().startsWith("openjdk"))
.map(p -> p.resolve("current"))
.filter(this::acceptFolder);
}
return Stream.empty();
}

@Override
protected boolean acceptFolder(@NonNull Path jdkFolder) {
return jdkFolder.getFileName().startsWith("openjdk") && super.acceptFolder(jdkFolder);
}

@Override
protected Jdk.@Nullable InstalledJdk createJdk(@NonNull Path home) {
try {
// Try to resolve any links
home = home.toRealPath();
} catch (IOException e) {
throw new IllegalStateException("Couldn't resolve 'current' link: " + home, e);
}
return super.createJdk(home);
return jdkFolder.getParent().getFileName().toString().startsWith("openjdk") && super.acceptFolder(jdkFolder);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import dev.jbang.devkitman.JdkDiscovery;
import dev.jbang.devkitman.JdkProvider;
import dev.jbang.devkitman.util.FileUtils;
import dev.jbang.devkitman.util.JavaUtils;

/**
* This JDK provider detects any JDKs that have been installed using the SDKMAN
Expand All @@ -18,7 +17,11 @@ public class SdkmanJdkProvider extends BaseFoldersJdkProvider {
private static final Path JDKS_ROOT = Paths.get(".sdkman", "candidates", "java");

public SdkmanJdkProvider() {
super(Paths.get(System.getProperty("user.home")).resolve(JDKS_ROOT));
super(jdksRoot());
}

public static Path jdksRoot() {
return Paths.get(System.getProperty("user.home")).resolve(JDKS_ROOT);
}

@Override
Expand All @@ -28,9 +31,7 @@ public SdkmanJdkProvider() {

@Override
protected boolean acceptFolder(@NonNull Path jdkFolder) {
return jdkFolder.startsWith(jdksRoot)
&& !FileUtils.isSameFolderLink(jdkFolder)
&& JavaUtils.hasJavacCmd(jdkFolder);
return super.acceptFolder(jdkFolder) && !FileUtils.isSameFolderLink(jdkFolder);
}

public static class Discovery implements JdkDiscovery {
Expand Down
159 changes: 159 additions & 0 deletions src/main/java/dev/jbang/devkitman/jdkproviders/WindowsJdkProvider.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
package dev.jbang.devkitman.jdkproviders;

import java.nio.file.Files;
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Stream;

import org.jspecify.annotations.NonNull;

import dev.jbang.devkitman.Jdk;
import dev.jbang.devkitman.JdkDiscovery;
import dev.jbang.devkitman.JdkProvider;
import dev.jbang.devkitman.util.JavaUtils;
import dev.jbang.devkitman.util.OsUtils;

/**
* This JDK provider detects JDKs registered under HKLM\SOFTWARE\JavaSoft.
*/
public class WindowsJdkProvider extends BaseJdkProvider {
public static final String JAVA_SOFT_KEY = "HKLM\\SOFTWARE\\JavaSoft";

private static final Logger LOGGER = Logger.getLogger(WindowsJdkProvider.class.getName());
private static final Pattern KEY_LINE = Pattern.compile("^\\s*HKEY_.*$");
private static final Pattern JAVA_HOME_LINE = Pattern.compile("^\\s*JavaHome\\s+REG_\\w+\\s+(.+?)\\s*$");
private static final Pattern VALID_ID = Pattern.compile("^[a-zA-Z0-9._+-]+-windows$");

private final RegistryReader registryReader;

public WindowsJdkProvider() {
this(new RegCommandRegistryReader());
}

public WindowsJdkProvider(@NonNull RegistryReader registryReader) {
this.registryReader = registryReader;
}
Comment on lines +42 to +44
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Enforce non-null RegistryReader in the constructor.

@NonNull is not a runtime guard. If null is passed, the failure is deferred until dereference in listInstalled() (Line 70), making diagnosis harder.

Suggested fix
 public WindowsJdkProvider(`@NonNull` RegistryReader registryReader) {
-    this.registryReader = registryReader;
+    this.registryReader = Objects.requireNonNull(registryReader, "registryReader");
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/dev/jbang/devkitman/jdkproviders/WindowsJdkProvider.java`
around lines 42 - 44, The WindowsJdkProvider constructor accepts a
RegistryReader annotated `@NonNull` but doesn't guard at runtime; add an explicit
null check in the WindowsJdkProvider(`@NonNull` RegistryReader registryReader)
constructor (e.g., use Objects.requireNonNull or an if-check) to validate
registryReader and throw a clear NPE/IllegalArgumentException so callers fail
fast instead of deferring to listInstalled() when registryReader is used.


@Override
@NonNull
public String name() {
return Discovery.PROVIDER_ID;
}

@Override
public @NonNull String description() {
return "The JDKs registered in the HKLM\\SOFTWARE\\JavaSoft registry key.";
}

@Override
public boolean canUse() {
return OsUtils.isWindows();
}

@Override
public boolean isValidId(@NonNull String id) {
return VALID_ID.matcher(id).matches();
}

@NonNull
@Override
public Stream<Jdk.InstalledJdk> listInstalled() {
return dedupeByJavaHome(registryReader.listJavaHomes(JAVA_SOFT_KEY))
.entrySet()
.stream()
.map(this::createJdk)
.filter(Objects::nonNull);
}

private Map<String, Path> dedupeByJavaHome(Map<String, Path> javaHomes) {
Map<Path, Entry<String, Path>> selectedByHome = new LinkedHashMap<>();
for (Entry<String, Path> entry : javaHomes.entrySet()) {
selectedByHome.merge(entry.getValue(), entry, this::preferLongestKey);
}
Map<String, Path> deduped = new LinkedHashMap<>();
for (Entry<String, Path> entry : selectedByHome.values()) {
deduped.put(entry.getKey(), entry.getValue());
}
return deduped;
}

private Entry<String, Path> preferLongestKey(Entry<String, Path> existing, Entry<String, Path> replacement) {
return replacement.getKey().length() > existing.getKey().length() ? replacement : existing;
}

private Jdk.InstalledJdk createJdk(Map.Entry<String, Path> javaHomeEntry) {
Path javaHome = javaHomeEntry.getValue();
if (!Files.isDirectory(javaHome) || !JavaUtils.hasJavacCmd(javaHome)) {
return null;
}
String id = registryVersionSegment(javaHomeEntry.getKey()) + "-" + name();
return createJdk(id, javaHome);
}

private String registryVersionSegment(String registryKey) {
int idx = registryKey.lastIndexOf('\\');
String segment = idx >= 0 && idx + 1 < registryKey.length() ? registryKey.substring(idx + 1) : registryKey;
return segment.replaceAll("[^a-zA-Z0-9._+-]+", "_");
}

public interface RegistryReader {
@NonNull
Map<String, Path> listJavaHomes(@NonNull String rootKey);
}

static class RegCommandRegistryReader implements RegistryReader {
@Override
public @NonNull Map<String, Path> listJavaHomes(@NonNull String rootKey) {
Map<String, Path> javaHomes = new LinkedHashMap<>();
String output = OsUtils.runCommand("reg", "query", rootKey, "/s", "/v", "JavaHome");
if (output == null || output.trim().isEmpty()) {
return javaHomes;
}
String currentKey = null;
for (String line : output.split("\\r?\\n")) {
Matcher keyMatcher = KEY_LINE.matcher(line);
if (keyMatcher.matches()) {
currentKey = line.trim();
continue;
}
if (currentKey == null) {
continue;
}
Matcher javaHomeMatcher = JAVA_HOME_LINE.matcher(line);
if (javaHomeMatcher.matches()) {
String home = javaHomeMatcher.group(1).trim();
try {
javaHomes.put(currentKey, Paths.get(home));
} catch (InvalidPathException ex) {
LOGGER.log(Level.FINE, "Ignoring invalid registry JavaHome: " + home, ex);
}
}
}
return javaHomes;
}
}

public static class Discovery implements JdkDiscovery {
public static final String PROVIDER_ID = "windows";

@Override
@NonNull
public String name() {
return PROVIDER_ID;
}

@Override
public JdkProvider create(@NonNull Config config) {
return new WindowsJdkProvider();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ dev.jbang.devkitman.jdkproviders.MiseJdkProvider$Discovery
dev.jbang.devkitman.jdkproviders.MultiHomeJdkProvider$Discovery
dev.jbang.devkitman.jdkproviders.ScoopJdkProvider$Discovery
dev.jbang.devkitman.jdkproviders.SdkmanJdkProvider$Discovery
dev.jbang.devkitman.jdkproviders.WindowsJdkProvider$Discovery

6 changes: 4 additions & 2 deletions src/test/java/dev/jbang/devkitman/TestJdkProviders.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ void testAllNames() {
"mise",
"multihome",
"scoop",
"sdkman"));
"sdkman",
"windows"));
}

@Test
Expand Down Expand Up @@ -78,7 +79,8 @@ void testAll() {
instanceOf(MiseJdkProvider.class),
instanceOf(MultiHomeJdkProvider.class),
instanceOf(ScoopJdkProvider.class),
instanceOf(SdkmanJdkProvider.class)));
instanceOf(SdkmanJdkProvider.class),
instanceOf(WindowsJdkProvider.class)));
}

@Test
Expand Down
Loading