Fix a case-sensitive issue with ProcessUtil.pathOfCommand(Path path) on Windows

[Eng-Fouad]

Currently, ProcessUtil.pathOfCommand(Path.of("docker")) will return C:\Program Files\Docker\Docker\resources\bin\docker.EXE instead of C:\Program Files\Docker\Docker\resources\bin\docker.exe.

This PR will fix a recent bug in latest Quarkus with podman detection on Windows:

public final class ContainerRuntimeUtil {

    private static final Pattern PODMAN_PATTERN = Pattern.compile("^podman(?:\\.exe)? version.*", Pattern.DOTALL);

Notice that the podman version output pattern expects that the .exe must be in lower case if exist.

I thought about fixing this bug in Quarkus repo, but I noticed that ProcessUtil.pathOfCommand is used in many places in Quarkus codebase, so fixing it here could potentially fix other bugs.

[gsmet]

Thanks a lot. I added some context and a comment.

Also, I wonder if you would have time to add some tests for this specifically for Windows. We can add a Windows runner to our CI to make sure this is fully sorted out.

Note that I would make it another PR as I think we need this PR to be included ASAP as I would like to include the fix in 3.30.1, that I will ship next Wednesday.

[gsmet]

Yeah so the thing is we have: PATHEXT=.COM;.EXE;.BAT;.CMD;.PS1 . Which means that we actually build a path with .EXE.

I wonder if we should lowercase the extensions in Windows but even so, it's probably safer to use toRealPath anyway, in case someone actually has an executable.EXE, which should work with a case insenstive FS.

@dmlloyd WDYT?

[dmlloyd]

My only concerns would be the cost of toRealPath itself, and the security implications (for example, do we pass NOFOLLOW_LINKS to toRealPath? why or why not? and, what if the real path ends up being in a totally different directory?).

I think that if we do go the toRealPath route, then instead of returning the toRealPath we should just look at its filename part and make sure it matches (e.g. String#equalsIgnoreCase) the original, and if so, take the original path and resolveSibling the filename part of the result of toRealPath. That would probably be safest.

[Eng-Fouad]

I did a little experiment. I changed the file name of podman.exe to podman.EXE, then opened a new cmd and typed podman --version. I get the output: podman version 5.6.2. So I believe we can simulate Windows OS search behavior by just taking the lowercase of the extension, something like: Path execPathExt = execPath.getParent().resolve(execPath.getFileName() + ext.toLowerCase()); without using toRealPath.

[dmlloyd]

I don't think Windows is mapping the case of the name... I think it's because the command was launched just as podman rather than as podman.exe or podman.EXE or anything else. The file name does not get passed to the command AFAIK, just the command name which was used to execute the program.

I'm not sure it's possible to do this from Java though. I recall we had some problems when trying to execute a program without its extension.

Edited to add: Perhaps this is one reason why many launchers try to launch via cmd.exe.

[gsmet]

Also @Eng-Fouad, will it actually cause issues, except for this version detection? What exactly happens in this case, we run podman successfully but the output of the version contains podman.EXE?

To be honest, I think we should be more permissive in the Pattern:

• by making it case insensitive
• by allowing (?:exe|cmd|bat) as we never know what they will do with the Podman distribution

[Eng-Fouad]

Thanks a lot. I added some context and a comment.

Also, I wonder if you would have time to add some tests for this specifically for Windows. We can add a Windows runner to our CI to make sure this is fully sorted out.

Note that I would make it another PR as I think we need this PR to be included ASAP as I would like to include the fix in 3.30.1, that I will ship next Wednesday.

Sure. I will try to add some tests later in a separate PR.

[Eng-Fouad]

Also @Eng-Fouad, will it actually cause issues, except for this version detection? What exactly happens in this case, we run podman successfully but the output of the version contains podman.EXE?

To be honest, I think we should be more permissive in the Pattern:

• by making it case insensitive
• by allowing (?:exe|cmd|bat) as we never know what they will do with the Podman distribution

Usually Windows FS is case-insensitive (i.e. you cannot have podman.exe and podman.EXE in the same folder). I think this is a special case with podman version command:

• podman --version returns podman version 5.6.2.
• "C:\Program Files\RedHat\Podman\podman.exe" --version returns podman version 5.6.2.
• "C:\Program Files\RedHat\Podman\podman.EXE" --version returns podman.EXE version 5.6.2.
• "C:\Program Files\RedHat\Podman\podman.EXe" --version returns podman.EXe version 5.6.2.

While this PR should resolve the issue in Quarkus, I am ok with modifying the pattern to be more permissive. Do you want me to open PR in Quarkus repo?

[dmlloyd]

Usually Windows FS is case-insensitive (i.e. you cannot have podman.exe and podman.EXE in the same folder). I think this is a special case with podman version command:

• podman --version returns podman version 5.6.2.
• "C:\Program Files\RedHat\Podman\podman.exe" --version returns podman version 5.6.2.
• "C:\Program Files\RedHat\Podman\podman.EXE" --version returns podman.EXE version 5.6.2.
• "C:\Program Files\RedHat\Podman\podman.EXe" --version returns podman.EXe version 5.6.2.

This is specifically a bug in podman, IMO (albeit an arguably minor one), so regardless of what we do here, we should report the bug upstream as well.

[dmlloyd]

Currently, ProcessUtil.pathOfCommand(Path.of("docker")) will return C:\Program Files\Docker\Docker\resources\bin\docker.EXE instead of C:\Program Files\Docker\Docker\resources\bin\docker.exe.

This PR will fix a recent bug in latest Quarkus with podman detection on Windows:

public final class ContainerRuntimeUtil {

    private static final Pattern PODMAN_PATTERN = Pattern.compile("^podman(?:\\.exe)? version.*", Pattern.DOTALL);

Notice that the podman version output pattern expects that the .exe must be in lower case if exist.

I thought about fixing this bug in Quarkus repo, but I noticed that ProcessUtil.pathOfCommand is used in many places in Quarkus codebase, so fixing it here could potentially fix other bugs.

I think this should be fixed specifically for this bit of code:

    private static final Pattern PODMAN_PATTERN = Pattern.compile("^podman(?i:\\.exe)? version.*", Pattern.DOTALL);

I.e. make the regex be case-insensitive for the part which detects the file extension. Whether or not we fix pathOfCommand is something worth considering as well, but this fix should definitely be made in Quarkus regardless.

[Eng-Fouad]

• Make podman version detection pattern to be case-insensitive quarkusio/quarkus#51122

@gsmet I did not add cmd|bat, because there are many other executable extensions other than cmd and bat, so I kept it only .exe for now.