smallrye-common icon indicating copy to clipboard operation
smallrye-common copied to clipboard

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

Open Eng-Fouad opened this issue 1 month ago • 6 comments

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.

Eng-Fouad avatar Nov 18 '25 22:11 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

gsmet avatar Nov 19 '25 15:11 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.

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

Eng-Fouad avatar Nov 19 '25 15:11 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?

Eng-Fouad avatar Nov 19 '25 16:11 Eng-Fouad

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 avatar Nov 19 '25 16:11 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.

dmlloyd avatar Nov 19 '25 16:11 dmlloyd

  • https://github.com/quarkusio/quarkus/pull/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.

Eng-Fouad avatar Nov 19 '25 16:11 Eng-Fouad